Skip to content

[GH-3124] Preserve S2 semantics in geography distance joins - #3527

Draft
jiayuasu wants to merge 1 commit into
apache:masterfrom
jiayuasu:codex/oss-geography-distance-join
Draft

jiayuasu wants to merge 1 commit into
apache:masterfrom
jiayuasu:codex/oss-geography-distance-join

Conversation

@jiayuasu

@jiayuasu jiayuasu commented Nov 6, 2026

Copy link
Copy Markdown
Member

Did you read the Contributor Guide?

Yes, I have read the contributor rules and development guide.

Is this PR related to a ticket?

Related to #3124. This is an independent correctness fix identified while checking the existing S2 geography boundary for the native Spark port. It does not enable native types.

What changes were proposed in this PR?

ST_Distance(geography, geography) < radius and <= radius were detected as planar geometry joins. Route supported comparisons through the existing spherical geography path instead.

For a finite, nonnegative constant radius, a broadcast join uses the existing S2 candidate bounds and refinement. When there is no broadcast S2 path, or the bound is row-dependent, nonfinite, null, or negative, retain Spark's exact scalar join evaluation. This matters for correctness: Spark orders NaN above finite doubles, whereas S2's dWithin refinement does not have those comparison semantics. Row-dependent bounds can contain such values.

Geometry joins, existing ST_DWithin handling, S2 calculations, and RDD/index serialization are unchanged. The patch changes one production file and adds one SQL regression suite. This is a correctness change; it does not claim a new partitioned geography join or a performance improvement for fallback cases.

How was this patch tested?

  • Reproduced the wrong geography planning with four failing SQL checks on unchanged OSS code.
  • Independently reproduced antimeridian/polar false negatives from planar candidate envelopes, and the NaN difference with actual Catalyst comparisons and the existing S2 refiner.
  • The expanded SQL checks failed before adding the bound guard.
  • Clean Spark 3.5.8 / Scala 2.12 and Spark 4.1.1 / Scala 2.13: 38 tests passed on each runtime (six new SQL tests plus all 32 existing geography broadcast-join tests).
  • New SQL tests compare optimization disabled/enabled for both < and <=, broadcast/nonbroadcast plans, SRIDs 4326/4269, antimeridian and polar matches, null geography, and constant/stream-side/build-side bounds including NaN and infinities.
  • Spotless, Scalastyle, and git diff --check passed. Tests use the existing Sedona session/Kryo configuration; no serializer modification was necessary.

Did this PR include necessary documentation updates?

The planner comment explains why unsupported geography comparison bounds retain Spark's exact evaluation. No public function signature changes.

This branch has not been deployed

No deployments
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