Skip to content

[CALCITE-6688] Allow operators of SqlKind.SYMMETRICAL to be reversed#4062

Merged
NobiGo merged 1 commit intoapache:mainfrom
tjbanghart:calcite-6688
Dec 5, 2024
Merged

[CALCITE-6688] Allow operators of SqlKind.SYMMETRICAL to be reversed#4062
NobiGo merged 1 commit intoapache:mainfrom
tjbanghart:calcite-6688

Conversation

@tjbanghart
Copy link
Contributor

SqlBasicFunction did not override SqlOperator#reverse() so functions belonging to the SqlKind.SYMMETRICAL set threw a NPE during normalization.

This PR adds a overridden reverse() method to SqlBasicFunction that returns the operator if isSymmetrical() is true.

@tjbanghart tjbanghart changed the title [CALCITE-6688] Allow basic symmetric operators to be reversed. [CALCITE-6688] Allow symmetric basic functions to be reversed. Nov 22, 2024
@mihaibudiu
Copy link
Contributor

You need some unit tests.
You should reconcile the JIRA title with this one.

@tjbanghart
Copy link
Contributor Author

Could you suggest another good test class to add to? There are some in core/src/test/java/org/apache/calcite/rex/RexNormalizeTest.java

@tjbanghart tjbanghart changed the title [CALCITE-6688] Allow symmetric basic functions to be reversed. [CALCITE-6688] Allow symmetrical SqlBasicFunctions functions to be reversed. Nov 22, 2024
@mihaibudiu
Copy link
Contributor

It would be great to have a test that reproduces the NPE that is mentioned in the JIRA issue.

@tjbanghart tjbanghart changed the title [CALCITE-6688] Allow symmetrical SqlBasicFunctions functions to be reversed. [CALCITE-6688] Allow operators of SqlKind.SYMMETRICAL to be reversed. Nov 22, 2024
@tjbanghart tjbanghart force-pushed the calcite-6688 branch 2 times, most recently from e16feb9 to 0f20939 Compare December 2, 2024 21:07
@tjbanghart tjbanghart requested a review from mihaibudiu December 2, 2024 22:56
@mihaibudiu mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Dec 2, 2024
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 4, 2024

@mihaibudiu
Copy link
Contributor

@NobiGo I will let you approve and merge this when you think you are happy with the requested changes.

@NobiGo NobiGo changed the title [CALCITE-6688] Allow operators of SqlKind.SYMMETRICAL to be reversed. [CALCITE-6688] Allow operators of SqlKind.SYMMETRICAL to be reversed Dec 5, 2024
@NobiGo NobiGo merged commit 0eb83b1 into apache:main Dec 5, 2024
@tjbanghart
Copy link
Contributor Author

Thank you both for the review @NobiGo, @mihaibudiu!

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

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants