Skip to content

API: Treat default edge algorithm as equal to an omitted one in GeographyType#16766

Closed
huan233usc wants to merge 1 commit into
apache:mainfrom
huan233usc:geography-normalize-default-algorithm
Closed

API: Treat default edge algorithm as equal to an omitted one in GeographyType#16766
huan233usc wants to merge 1 commit into
apache:mainfrom
huan233usc:geography-normalize-default-algorithm

Conversation

@huan233usc

@huan233usc huan233usc commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Make Types.GeographyType treat an explicit default edge-interpolation algorithm as equal to an omitted one, so semantically identical geography types compare equal.

GeographyType uses null internally to mean "default" for both crs and algorithm, but equals / hashCode / toString compared the raw nullable fields. As a result GeographyType.of(crs, EdgeAlgorithm.SPHERICAL) was not equals() to the equivalent GeographyType.of(crs), even though SPHERICAL is the documented default and the algorithm() getter resolves to SPHERICAL for both. This also meant a plain geography did not round-trip through format conversions that omit default values — e.g. Parquet's geography logical type, where an unset algorithm defaults to SPHERICAL (#16765).

Rather than collapse the default in the constructor, this normalizes at the comparison/render boundary: 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 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

  • Extended testGeospatialTypeToString to confirm an explicit default SPHERICAL renders the same compact form as an omitted algorithm (geography, geography(srid:4326)).
  • New testGeospatialTypeDefaultNormalization covers equals() / hashCode() parity for the default-CRS and default-algorithm forms, that algorithm() still reports SPHERICAL, 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 in TestSchemaParser / TestSingleValueParser / TestGeospatialTable / TestSchemaUnionByFieldName or anywhere geography types are serialized.

@github-actions github-actions Bot added the API label Jun 11, 2026
@huan233usc huan233usc marked this pull request as draft June 11, 2026 02:38
…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
@huan233usc huan233usc force-pushed the geography-normalize-default-algorithm branch from da34f57 to 5bc590c Compare June 11, 2026 02:55
@huan233usc huan233usc changed the title API: Normalize default edge algorithm in GeographyType API: Treat default edge algorithm as equal to an omitted one in GeographyType Jun 11, 2026
@huan233usc

Copy link
Copy Markdown
Contributor Author

Consolidated into #16765 — the GeographyType equality change and the Parquet schema mapping it enables are now reviewed together there.

@huan233usc huan233usc closed this Jun 11, 2026
@huan233usc huan233usc deleted the geography-normalize-default-algorithm branch June 11, 2026 05:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant