API: Treat default edge algorithm as equal to an omitted one in GeographyType#16766
Closed
huan233usc wants to merge 1 commit into
Closed
API: Treat default edge algorithm as equal to an omitted one in GeographyType#16766huan233usc wants to merge 1 commit into
huan233usc wants to merge 1 commit into
Conversation
…aphyType GeographyType used null to mean "default" for both crs and algorithm, but equals/hashCode/toString compared the raw nullable fields, so GeographyType.of(crs, EdgeAlgorithm.SPHERICAL) was not equal to the equivalent GeographyType.of(crs) even though SPHERICAL is the documented default. A plain geography therefore did not round-trip through format conversions that omit default values (e.g. Parquet, where an unset algorithm defaults to SPHERICAL). Normalize at the comparison/render boundary instead of in the constructor: equals, hashCode, and toString now use the resolved getters crs() / algorithm() (which already apply the defaults), so an explicit default and an omitted one compare equal and render the same. The constructor is unchanged. CRS already behaved this way; this brings the algorithm in line. Co-authored-by: Isaac
da34f57 to
5bc590c
Compare
Contributor
Author
|
Consolidated into #16765 — the GeographyType equality change and the Parquet schema mapping it enables are now reviewed together there. |
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.
Summary
Make
Types.GeographyTypetreat an explicit default edge-interpolation algorithm as equal to an omitted one, so semantically identical geography types compare equal.GeographyTypeusesnullinternally to mean "default" for bothcrsandalgorithm, butequals/hashCode/toStringcompared the raw nullable fields. As a resultGeographyType.of(crs, EdgeAlgorithm.SPHERICAL)was notequals()to the equivalentGeographyType.of(crs), even thoughSPHERICALis the documented default and thealgorithm()getter resolves toSPHERICALfor both. This also meant a plaingeographydid not round-trip through format conversions that omit default values — e.g. Parquet's geography logical type, where an unsetalgorithmdefaults toSPHERICAL(#16765).Rather than collapse the default in the constructor, this normalizes at the comparison/render boundary:
equals,hashCode, andtoStringnow use the resolved getterscrs()/algorithm(), which already apply the defaults. So an explicit default and an omitted one compare equal and render the same compact form. The constructor is left untouched, and CRS — which already resolved this way through the getter — is brought in line with the algorithm. No public signature changes.Test plan
testGeospatialTypeToStringto confirm an explicit defaultSPHERICALrenders the same compact form as an omitted algorithm (geography,geography(srid:4326)).testGeospatialTypeDefaultNormalizationcoversequals()/hashCode()parity for the default-CRS and default-algorithm forms, thatalgorithm()still reportsSPHERICAL, and that a non-default algorithm (KARNEY) stays distinct../gradlew :iceberg-api:check— clean (tests, checkstyle, revapi, spotless)../gradlew :iceberg-core:test— full core suite green; no regressions inTestSchemaParser/TestSingleValueParser/TestGeospatialTable/TestSchemaUnionByFieldNameor anywhere geography types are serialized.