fix(postgresql): stop declaring IPNetwork on both the cidr and inet mappings (weasel#405) - #408
Merged
Conversation
…appings (weasel#405)
GetTypeMapping resolves a CLR type with
NpgsqlTypeMapper.Mappings.LastOrDefault(m => m.ClrTypes.Contains(type))
and LastOrDefault only means something over an ordered sequence. Mappings is
a JasperFx.Core.Cache backed by an ImHashMap, so it enumerates in hash-tree
order, not insertion order:
enumeration : <custom>, <custom>, Bigint, Boolean, Box, Bytea, Circle, Char
declared : Smallint, Integer, Bigint, Real, Double, Numeric, Money, Text
IPNetwork was the one CLR type claimed by two mappings, cidr and inet. cidr
is correct and `ipnetwork_resolves_to_cidr` passes -- but only incidentally:
the source declares Cidr *before* Inet, so under real insertion order
LastOrDefault would return "inet" and that test would fail. It passed
because the hash layout happens to put Cidr last, which is a function of the
current key set rather than of anything declared. Consuming code is
explicitly invited to add mappings.
Drop the stray typeof(IPNetwork) from the inet mapping. IPAddress,
NpgsqlInet and the (IPAddress, int) tuple stay. Resolution is now
unambiguous and order-independent.
Two guards added: one pinning IPNetwork to exactly one mapping, and a
general one asserting no CLR type is claimed twice, so the ambiguity cannot
come back silently. Both fail if the duplicate is restored.
Postgres suite green on all four CI matrix legs, 788 passed each.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #405.
PostgresqlProvider.GetTypeMappingbreaks a CLR-type tie withLastOrDefault:LastOrDefaultonly means something over an ordered sequence.Mappingsis aJasperFx.Core.Cachebacked by anImHashMap, so it enumerates in hash-tree order:IPNetworkwas the one CLR type claimed by two mappings —Cidr(line 85) andInet(line 86):cidris the right answer andipnetwork_resolves_to_cidrpasses — but only by accident. The source declaresCidrbeforeInet, so under genuine insertion orderLastOrDefaultwould returninetand that test would fail today. It passes because the hash layout happens to enumerateCidrlast, which is a function of the current key set — and consuming code is explicitly invited to add mappings.Change
Drop the stray
typeof(IPNetwork)from theInetmapping.IPAddress,NpgsqlInetand the(IPAddress, int)tuple stay onInet. Resolution becomes unambiguous and order-independent.Two guards so this cannot silently come back:
ipnetwork_is_claimed_by_exactly_one_mappingno_clr_type_is_claimed_by_more_than_one_mapping— general, reports offenders asType <- Mapping, MappingCounterfactual — restoring the duplicate fails both:
Verification
All four CI matrix legs (net9.0/net10.0 × case-sensitive true/false): 788 passed, 0 failed, 3 skipped.
One unrelated intermittent failure surfaced during validation —
DatabaseWithTablesTests.detect_and_apply_schema_changes, which races other collections over the sharedpublicschema. Confirmed pre-existing on cleanmaster(reproduced in 3 runs there) and filed separately as #407. It is not affected by this change.🤖 Generated with Claude Code