[Tests] Remove npgsqldbtype passing and rename related flag #6305
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the start of a restructuring of the type mapping asserts. Recently we ran into them being hard to grasp in the Cube PR, and in #6267 I had to wrangle them into a passable shape again as well for those changes to pass.
The first change I've pulled out removes
npgsqlDbTypeas a parameter on any of the AssertType methods. In the second commit all the callers are changed. With this change we're always deriving the expected NpgsqlDbType from the given pgTypeName. The asserts still check whether the NpgsqlDbType is returned back to us correctly on the db parameter. We're using this same static map in the actual parameter implementation, so all tests pass without further changes.There is just no good reason to keep this duplication around. In discussions around #6267 we have also concluded again that we don't want
NpgsqlDbType <-> DataTypeNamemappings to become overridable in any way so this seems like a good moment to do it.The last commit renames
isNpgsqlDbTypeInferredFromClrTypetoisDataTypeInferredFromValue, which should have been renamed sooner for clarity. Starting from the 8.0 converter refactor pgTypeName and npgsqlDbType are both affected by this flag already, as we use the same source for these. Also note that the name moves away from 'ClrType' to 'Value'. This aligns the name with thevalueparameter, and captures that the db parameter might do value-dependent inference as well.My next PR should replace isDefault (isDefaultForWriting/isDefaultForReading) with parameters that more closely match what observations we do. It's too easy today to pass
isNpgsqlDbTypeInferredFromClrType: falseorisDefault: falsewhile in fact the test can (and should) run with more inference checks. Replacing the wider flags (so far, with a similar number of new ones) will allow us to do more inference checks, which have locally already caught a good number of those liberally defined tests.