Skip to content

Conversation

@asuriy
Copy link
Contributor

@asuriy asuriy commented Nov 10, 2025

  • Added is_true, is_false, is_not_true, is_not_false and is_distinct_from functions in FunctionMappings.java
  • Added tests into new file: ComparisonFunctionsTest.java

@asuriy asuriy marked this pull request as draft November 10, 2025 12:44
@asuriy asuriy marked this pull request as ready for review November 10, 2025 13:06
@andrew-coleman andrew-coleman merged commit f59ecc5 into substrait-io:main Nov 10, 2025
12 checks passed
@CsvSource({"int_a, int_b", "int_b, int_a", "double_a, double_b", "double_b, double_a"})
void is_distinct_from(String left, String right) throws Exception {
String query = String.format("SELECT (%s IS DISTINCT FROM %s) FROM numbers", left, right);
assertSqlSubstraitRelRoundTrip(query, CREATES);
Copy link
Member

Choose a reason for hiding this comment

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

Minor comment looking at this from the future. This doesn't actually test the IS DISTINCT FROM mapping because a query like

SELECT (int_a IS DISTINCT FROM int_b) FROM numbers

gets processed into the following Calcite representation

LogicalProject(EXPR$0=[AND(OR(IS NOT NULL($0), IS NOT NULL($1)), IS NOT TRUE(=($0, $1)))])
  LogicalTableScan(table=[[NUMBERS]])

I only noticed this because I was poking around updating some tests. The same thing happens with IS NOT DISTINCT FROM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for bringing this to my attention @vbarua! I'll take a look and rectify this tomorrow morning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vbarua I believe I've addressed the logical rewrite in this pull request: #628 - let me know if this works

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.

3 participants