[CALCITE-6150] Unparse for certain EXTRACT Unit with ClickHouseSqlDialect #3557
Conversation
Chen768959
changed the title
| @@ -7453,6 +7453,23 @@ private void checkLiteral2(String expression, String expected) { | |||
| sql(query).withSpark().withLibrary(SqlLibrary.SPARK).ok(expectedSql); | |||
| } | |||
|
|
|||
| /** Test case for | |||
| * <a href="https://issues.apache.org/jira/browse/CALCITE-6150">[CALCITE-6150] | |||
There was a problem hiding this comment.
Convert this to testExtract, move it to the right place in the file (at the end will collide with other changes), and test at least one other dialect.
There was a problem hiding this comment.
@julianhyde Thank you for your review. It seems that there were no tests for the EXTRACT operator previously, so I have changed the test case to testExtract and added tests for the EXTRACT operator in various dialects. If it's convenient for you, I look forward to your further review. Thank you.
There was a problem hiding this comment.
This is good. Can you break up the strings to make the lines shorter (just one call per line). Put + at the start of the line (per our style checker).
Move the method after testBigQueryDatetimeFormatFunctions, which is similar. (Anywhere but the end of the file.)
There was a problem hiding this comment.
This is good. Can you break up the strings to make the lines shorter (just one call per line). Put
+at the start of the line (per our style checker).Move the method after
testBigQueryDatetimeFormatFunctions, which is similar. (Anywhere but the end of the file.)
I have fixed the code style and moved testExtract after testBigQuery. I look forward to your further review. Thank you.
There was a problem hiding this comment.
There are many existing extract tests.
I tried to fix many more cases for extract here: #3435
But this failed the Druid tests. (Not sure it's the same problem with the current Druid failures. Didn't have time to investigate.)
|
@mihaibudiu, By 'no tests for EXTRACT previously' I believe @Chen768959 is referring specifically to |
Yes |
| @@ -2264,6 +2264,74 @@ private SqlDialect nonOrdinalDialect() { | |||
| sql(query).withLibrary(SqlLibrary.BIG_QUERY).ok(expected); | |||
| } | |||
|
|
|||
| /** Test case for | |||
| * <a href="https://issues.apache.org/jira/browse/CALCITE-6150">[CALCITE-6150] | |||
| * Generate dialect-specific SQL for EXTRACT operator.</a>. */ | |||
There was a problem hiding this comment.
Maybe you can remove . before a tag.
There was a problem hiding this comment.
Maybe you can remove
.before a tag.
Okay, it seems that other tags also don't have a . at the end, I will remove it.
…r certain units in the EXTRACT function Close apache#3557
…r certain units in the EXTRACT function Close apache#3557
Clickhouse does not support the following EXTRACT Unit syntax, and ClickHouseSqlDialect may need some adaptation.
SELECT EXTRACT(DOW FROM Date '2023-12-01')SELECT EXTRACT(DOY FROM Date '2023-12-01')SELECT EXTRACT(WEEK FROM Date '2023-12-01')