[CALCITE-6876] Druid Adapter support more functions #4227
Conversation
| final String sql2 = "SELECT COUNT(*) FROM " + FOODMART_TABLE + " WHERE " | ||
| + "FLOOR(ATAN2(\"store_cost\", \"store_cost\")) = 2 "; | ||
| sql(sql2, FOODMART) | ||
| .returnsOrdered("EXPR$0=0") |
There was a problem hiding this comment.
I assume this result has been validated in some way.
There was a problem hiding this comment.
Yes，I had test the ut with Druid Adapter dataset(https://github.com/zabetak/calcite-druid-dataset) in docker env and result is passed. @mihaibudiu
There was a problem hiding this comment.
I guess the question is about the results, not the test, something like comparing the result here with the output of a few major DBs like postgres or Oracle, could you do that and share @xuzifu666 please?
There was a problem hiding this comment.
@asolimando OK，I had test the result from Oracle，the result like follow which keep the same with Druid:
ORACLE:
select COT(10) from test_db.new_table nt ORDER BY id limit 1;
1.5423510453569202
select ASIN(0.1) from test_db.new_table nt ORDER BY id limit 1;
0.1001674211615598
select ACOS(0.1) from test_db.new_table nt ORDER BY id limit 1;
1.4706289056333368
select ATAN(10) from test_db.new_table nt ORDER BY id limit 1;
1.4711276743037347
select ATAN2(10, 20) from test_db.new_table nt ORDER BY id limit 1;
0.4636476090008061
select DEGREES(10) from test_db.new_table nt ORDER BY id limit 1;
572.9577951308232
Druid：
select COT(10)
1.5423510453569202
select ASIN(0.1)
0.1001674211615598
select ACOS(0.1)
1.4706289056333368
select ATAN(10)
1.4711276743037347
select ATAN2(10, 20)
0.4636476090008061
select DEGREES(10)
572.9577951308232
mihaibudiu
added
the
LGTM-will-merge-soon
| @@ -120,6 +120,12 @@ public class DruidQuery extends AbstractRelNode implements BindableRel { | |||
| .add(new DirectOperatorConversion(SqlStdOperatorTable.SIN, "sin")) | |||
| .add(new DirectOperatorConversion(SqlStdOperatorTable.COS, "cos")) | |||
| .add(new DirectOperatorConversion(SqlStdOperatorTable.TAN, "tan")) | |||
| .add(new DirectOperatorConversion(SqlStdOperatorTable.COT, "cot")) | |||
There was a problem hiding this comment.
Could you please add a link to the druid function documentation for this method, at least to let users know which version it is, thank you
There was a problem hiding this comment.
Done, Thanks~ @caicancai
xuzifu666
force-pushed
the
druid_functions_support
branch
from
2f0b777 to
54f3fc6
Compare
| .add(new DirectOperatorConversion(SqlStdOperatorTable.ACOS, "acos")) | ||
| .add(new DirectOperatorConversion(SqlStdOperatorTable.ATAN, "atan")) | ||
| .add(new DirectOperatorConversion(SqlStdOperatorTable.ATAN2, "atan2")) | ||
| .add(new DirectOperatorConversion(SqlStdOperatorTable.DEGREES, "degrees")) |
There was a problem hiding this comment.
The test cases have not covered the degrees function.
There was a problem hiding this comment.
Thanks for your reminder，had added case for degrees @NobiGo
There was a problem hiding this comment.
degrees function had some problem for druid test, so I remove first. this pr is for COT、ASIN、ACOS、ATAN、ATAN2，and change jira description.
There was a problem hiding this comment.
I'd rather leave a disabled unit test with a comment so people are aware of the problem, it's less probable that they will come check here IMO
xuzifu666
force-pushed
the
druid_functions_support
branch
2 times, most recently
from
e330969 to
eea1b12
Compare
druid/src/main/java/org/apache/calcite/adapter/druid/DruidQuery.java
Outdated
Show resolved
Hide resolved
|
Not blocking as I have only a few minor issues with the PR but please take a look and see if you feel like including them in the final version, thanks for the contribution @xuzifu666 |
@asolimando OK，I accept your suggestion，had included them in the final version，PTAL，thanks~ |
Co-authored-by: Alessandro Solimando <[email protected]>
xuzifu666
force-pushed
the
druid_functions_support
branch
from
19dec2a to
80ea63b
Compare
|
As description in JIRA：https://issues.apache.org/jira/browse/CALCITE-6876
ut had test success in Druid docker test env.