Skip to content

feat: support SQL IS [NOT] DISTINCT FROM as null-safe comparison - #9331

Open
jonasdedden wants to merge 2 commits into
lance-format:mainfrom
jonasdedden:sql-nullsafe-distinct-from
Open

jonasdedden wants to merge 2 commits into
lance-format:mainfrom
jonasdedden:sql-nullsafe-distinct-from

Conversation

@jonasdedden

@jonasdedden jonasdedden commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Support SQL IS [NOT] DISTINCT FROM in Lance filters, which previously failed with "not supported SQL".

The planner now maps both spellings to DataFusion's IsDistinctFrom / IsNotDistinctFrom, as DataFusion's own SQL frontend does. Literal coercion, the signed-zero rewrite (x IS NOT DISTINCT FROM 0.0 matches both zero encodings) and null-safe evaluation already handle these operators. Also fixes the signed_zero.rs doc table, which described an outdated rewrite shape.

Column-vs-column signed zeros and NaN still follow Arrow's total order (#9316, fixed by #9323; #9315).

@github-actions github-actions Bot added the enhancement New feature or request label Sep 17, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 17, 2026
Drop the test asserting the scalar index is not used and the asserts
pinning known float total-order behavior (lance-format#9332, lance-format#9315); cover integers,
strings and both zero spellings in the existing query tests instead.
@lance-gatekeeper lance-gatekeeper Bot removed K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 17, 2026
@jonasdedden

Copy link
Copy Markdown
Contributor Author

@lance-gatekeeper: Agreed. The risk isn't specific to this spelling: = and != between float columns already follow Arrow's total order. This PR only adds the SQL syntax and doesn't change comparison semantics. The PR description now points to #9316 (signed-zero column comparisons, fixed by #9323) and #9315 (NaN ordering). I also removed the test asserts that treated the current -0.0/NaN results as correct, so those fixes won't break this PR's tests when they land.

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Gate recommendation: approve with a non-blocking risk.

The author clarification confirms that this PR only exposes the existing comparison semantics and now avoids tests that pin the signed-zero or NaN total-order behavior. The direct parser lowering still matches DataFusion and preserves null-safe evaluation plus the existing signed-zero literal rewrite, so no change is requested here. The accepted residual risk is that float column-to-column and NaN distinctness can still follow Arrow total order; #9323 is the active signed-zero repair and #9315 tracks NaN ordering.

@lance-gatekeeper lance-gatekeeper Bot added K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk. labels Sep 17, 2026
@jonasdedden

Copy link
Copy Markdown
Contributor Author

@Xuanwo any review?:)

@jonasdedden

Copy link
Copy Markdown
Contributor Author

@wjones127 @LuciferYang could you take a look? Small planner change mapping IS [NOT] DISTINCT FROM to DataFusion's operators; the signed-zero rewrite already handles them (cc #6236).

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

enhancement New feature or request K-approved Latest Gatekeeper recommendation permits acceptance. K-risk Latest Gatekeeper recommendation includes a non-blocking risk.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant