Skip to content

Conversation

@hqbhoho
Copy link
Contributor

@hqbhoho hqbhoho commented Nov 5, 2025

Description

if (functionName.equals(IDENTICAL_OPERATOR_FUNCTION_NAME)) {
 return Optional.of(Domain.create(ValueSet.ofRanges(Range.range(type, startOfDate, true, startOfNextDate, false)), true));
}

Since IDENTICAL_OPERATOR_FUNCTION_NAME resolves to a range query (>=startOfDate AND <startOfNextDate), which is designed to match a specific date interval, the nullAllowed parameter would be set to false.

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
(X) Release notes are required, with the following suggested text:

## Iceberg/Delta Lake/MySQL/PostgreSQL
* Fix incorrect results due to incorrect pushdown of `IS NOT DISTINCT FROM`. ({issue}`27213`)

@cla-bot cla-bot bot added the cla-signed label Nov 5, 2025
@hqbhoho hqbhoho requested a review from martint November 5, 2025 10:56
}
if (functionName.equals(IDENTICAL_OPERATOR_FUNCTION_NAME)) {
return Optional.of(Domain.create(ValueSet.ofRanges(Range.range(type, startOfDate, true, startOfNextDate, false)), true));
return Optional.of(Domain.create(ValueSet.ofRanges(Range.range(type, startOfDate, true, startOfNextDate, false)), false));
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to add a jdoc to IDENTICAL_OPERATOR_FUNCTION_NAME documenting semantics (like SQL's NOT DISTINCT FROM, i assume)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

it would be great to document it where io.trino.spi.expression.StandardFunctions#IDENTICAL_OPERATOR_FUNCTION_NAME is defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a jdoc like:
$identical function is equivalent to the SQL operator "IS NOT DISTINCT FROM".

@findepi findepi merged commit 7f380c0 into trinodb:master Nov 6, 2025
100 checks passed
@findepi
Copy link
Member

findepi commented Nov 6, 2025

Merged, thanks!

@github-actions github-actions bot added this to the 479 milestone Nov 6, 2025
@chenjian2664
Copy link
Contributor

@findepi @hqbhoho I think this PR probably deserves a release note, could you check if we should add one?

@hqbhoho
Copy link
Contributor Author

hqbhoho commented Nov 7, 2025

@findepi @hqbhoho I think this PR probably deserves a release note, could you check if we should add one?

@chenjian2664 @findepi PTAL, I have added a release note.

@findepi
Copy link
Member

findepi commented Nov 7, 2025

I don't think it should cause incorrect query results.
I thought it is a small perf improvement in predicate pushdown, allowing us to filter better.

Please clarify.

Also, if this did indeed cause incorrect query results, would you mind adding a regression test that would show that correctness issue in plain sight (if applied before the fix was applied)?

@hqbhoho
Copy link
Contributor Author

hqbhoho commented Nov 7, 2025

@findepi I have added a test to show incorrect query results, #27242

@findepi
Copy link
Member

findepi commented Nov 7, 2025

@chenjian2664 i suggest following RN

* Fix incorrect results due to incorrect pushdown of `IS [NOT] DISTINCT FROM`.  

this applies to Iceberg, Delta, PostgreSQL, MySQL

@hqbhoho
Copy link
Contributor Author

hqbhoho commented Nov 8, 2025

@chenjian2664 i suggest following RN

* Fix incorrect results due to incorrect pushdown of `IS [NOT] DISTINCT FROM`.  

this applies to Iceberg, Delta, PostgreSQL, MySQL

I think maybe only IS NOT DISTINCT FROM will query incorrect results,IS DISTINCT FROM is equal to $not($identical()) will not pushdown in UtcConstraintExtractor.

@chenjian2664
Copy link
Contributor

Thanks both!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants