-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CALCITE-6312] Add LOG function (enabled in PostgreSQL library) #3839
Conversation
In order to prevent LogImplementor from making the code redundant, I added SqlLibrary for identification. @normanj-bitquill If the log function in Redshift is the same as postgres please let me know so I can modify it |
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
Outdated
Show resolved
Hide resolved
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
Outdated
Show resolved
Hide resolved
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.POSTGRESQL); | ||
f.checkScalar("log(10, 10)", 1.0, | ||
"DOUBLE NOT NULL"); | ||
f.checkScalar("log(64, 8)", 2.0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LOG(base, value), the result of log(64, 8) is not 2.0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohhh, seems to be a bug in sqlfunctions, I open a new jira case, bigquery is log(64, 8) == 2?
I overlooked the problem, and I'm sorry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fix
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
Outdated
Show resolved
Hide resolved
fa068f0
to
97d0d99
Compare
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
Outdated
Show resolved
Hide resolved
@YiwenWu @XuQianJin-Stars @NobiGo PATL, thanks |
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
Outdated
Show resolved
Hide resolved
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
Outdated
Show resolved
Hide resolved
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
Outdated
Show resolved
Hide resolved
69e9e93
to
e44286e
Compare
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
@mihaibudiu @YiwenWu @XuQianJin-Stars Thank you for your review |
https://issues.apache.org/jira/browse/CALCITE-6312
https://issues.apache.org/jira/browse/CALCITE-6456