Skip to content

Conversation

@NinoFloris
Copy link
Member

@NinoFloris NinoFloris commented Nov 11, 2025

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 npgsqlDbType as 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 <-> DataTypeName mappings to become overridable in any way so this seems like a good moment to do it.

The last commit renames isNpgsqlDbTypeInferredFromClrType to isDataTypeInferredFromValue, 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 the value parameter, 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: false or isDefault: false while 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.

@NinoFloris NinoFloris changed the title Remove npgsqldbtype passing and rename related flag [Tests] Remove npgsqldbtype passing and rename related flag Dec 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant