Re: [PR] [CALCITE-5955] BigQuery PERCENTILE functions are unparsed incorrectly [calcite]
caicancai commented on PR #3438: URL: https://github.com/apache/calcite/pull/3438#issuecomment-1867349263 > > @tanclary Hello, should we add corresponding tests to SqlOperatorTest > > @caicancai Could you clarify what you mean? I believe there are already tests for these functions in `SqlOperatorTest`. Also, these functions are not fully implemented so we're a bit restricted in what kind of tests we can write. Thank you for your reply for answering my questions -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6165] Add DATE_ADD test and DATE_DIFF test on SqlOperatorTest [calcite]
caicancai commented on code in PR #3586:
URL: https://github.com/apache/calcite/pull/3586#discussion_r1434803576
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -11948,6 +11984,25 @@ void testTimestampDiff(boolean coercionEnabled) {
f.checkFails("datediff(^\"MONTH\"^, '2019-09-14', '2019-09-15')",
"(?s)Column 'MONTH' not found in any table",
false);
+final SqlOperatorFixture f0 = f.withLibrary(SqlLibrary.BIG_QUERY);
+f0.checkScalar("date_diff(DATE '2010-07-07', DATE '2008-12-25', DAY)",
+"559",
+"INTEGER NOT NULL");
+f0.checkScalar("date_diff(DATE '2010-07-14', DATE '2010-07-07', WEEK)",
+"1",
+"INTEGER NOT NULL");
+f0.checkScalar("date_diff(DATE '2011-12-14', DATE '2011-07-14', MONTH)",
+"5",
+"INTEGER NOT NULL");
+f0.checkScalar("date_diff(DATE '2011-10-14', DATE '2011-07-14', QUARTER)",
+"1",
+"INTEGER NOT NULL");
+f0.checkScalar("date_diff(DATE '2021-07-14', DATE '2011-07-14', YEAR)",
+"10",
+"INTEGER NOT NULL");
+f0.checkNull("date_diff(CAST(NULL AS DATE), CAST(NULL AS DATE), DAY)");
+f0.checkNull("date_diff(CAST(NULL AS DATE), CAST(NULL AS DATE), DAY)");
Review Comment:
outdated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6165] Add DATE_ADD test and DATE_DIFF test on SqlOperatorTest [calcite]
caicancai commented on code in PR #3586:
URL: https://github.com/apache/calcite/pull/3586#discussion_r1434801168
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -11859,6 +11859,42 @@ void testTimestampDiff(boolean coercionEnabled) {
f.checkNull("time_sub(CAST(NULL AS TIME), interval 5 minute)");
}
+ @Test void testDateAdd() {
+final SqlOperatorFixture f0 = fixture()
+.setFor(SqlLibraryOperators.DATE_ADD);
+f0.checkFails("^date_add(date '2008-12-25', "
++ "interval 5 day)^",
+"No match found for function signature "
++ "DATE_ADD\\(, \\)", false);
+
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalar("date_add(date '2016-02-22', interval 2 day)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2016-02-17', interval 1 week)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2016-02-10', interval 2 weeks)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2020-10-17', interval 0 week)",
+"2020-10-17",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2016-11-24', interval 3 month)",
+"2017-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2015-11-24', interval 1 quarter)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2015-08-24', interval 2 quarters)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2011-02-24', interval 5 year)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkNull("date_add(CAST(NULL AS DATE), interval 5 day)");
Review Comment:
Next pr fix
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Control simplification in RelBuilder#filter with `config.simplify()' [calcite]
TusharSthul closed pull request #3592: Control simplification in RelBuilder#filter with `config.simplify()' URL: https://github.com/apache/calcite/pull/3592 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6165] Add DATE_SUB test and DATE_DIFF test on SqlOperatorTest [calcite]
chucheng92 commented on PR #3586: URL: https://github.com/apache/calcite/pull/3586#issuecomment-1867156919 The commit name "Add DATE_SUB test and DATE_DIFF" but you actually add DATE_ADD and DATE_DIFF. could you recheck it? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6165] Add DATE_SUB test and DATE_DIFF test on SqlOperatorTest [calcite]
chucheng92 commented on code in PR #3586:
URL: https://github.com/apache/calcite/pull/3586#discussion_r1433384145
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -11859,6 +11859,42 @@ void testTimestampDiff(boolean coercionEnabled) {
f.checkNull("time_sub(CAST(NULL AS TIME), interval 5 minute)");
}
+ @Test void testDateAdd() {
+final SqlOperatorFixture f0 = fixture()
+.setFor(SqlLibraryOperators.DATE_ADD);
+f0.checkFails("^date_add(date '2008-12-25', "
++ "interval 5 day)^",
+"No match found for function signature "
++ "DATE_ADD\\(, \\)", false);
+
+final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.BIG_QUERY);
+f.checkScalar("date_add(date '2016-02-22', interval 2 day)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2016-02-17', interval 1 week)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2016-02-10', interval 2 weeks)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2020-10-17', interval 0 week)",
+"2020-10-17",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2016-11-24', interval 3 month)",
+"2017-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2015-11-24', interval 1 quarter)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2015-08-24', interval 2 quarters)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkScalar("date_add(date '2011-02-24', interval 5 year)",
+"2016-02-24",
+"DATE NOT NULL");
+f.checkNull("date_add(CAST(NULL AS DATE), interval 5 day)");
Review Comment:
could u add a case that next arg is NULL?
##
core/src/main/java/org/apache/calcite/sql/SqlKind.java:
##
@@ -443,6 +443,9 @@ public enum SqlKind {
LEAST,
/** {@code DATE_DIFF} function (BigQuery Semantics). */
+ DATE_DIFF,
Review Comment:
why add a new SqlKind but never use?
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -11948,6 +11984,25 @@ void testTimestampDiff(boolean coercionEnabled) {
f.checkFails("datediff(^\"MONTH\"^, '2019-09-14', '2019-09-15')",
"(?s)Column 'MONTH' not found in any table",
false);
+final SqlOperatorFixture f0 = f.withLibrary(SqlLibrary.BIG_QUERY);
+f0.checkScalar("date_diff(DATE '2010-07-07', DATE '2008-12-25', DAY)",
+"559",
+"INTEGER NOT NULL");
+f0.checkScalar("date_diff(DATE '2010-07-14', DATE '2010-07-07', WEEK)",
+"1",
+"INTEGER NOT NULL");
+f0.checkScalar("date_diff(DATE '2011-12-14', DATE '2011-07-14', MONTH)",
+"5",
+"INTEGER NOT NULL");
+f0.checkScalar("date_diff(DATE '2011-10-14', DATE '2011-07-14', QUARTER)",
+"1",
+"INTEGER NOT NULL");
+f0.checkScalar("date_diff(DATE '2021-07-14', DATE '2011-07-14', YEAR)",
+"10",
+"INTEGER NOT NULL");
+f0.checkNull("date_diff(CAST(NULL AS DATE), CAST(NULL AS DATE), DAY)");
+f0.checkNull("date_diff(CAST(NULL AS DATE), CAST(NULL AS DATE), DAY)");
Review Comment:
duplicated lines
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
mihaibudiu commented on code in PR #3590:
URL: https://github.com/apache/calcite/pull/3590#discussion_r1434660313
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -8897,56 +8898,44 @@ private void testCurrentDateFunc(Pair pair) {
f0.forEachLibrary(list(SqlLibrary.BIG_QUERY, SqlLibrary.ORACLE), consumer);
}
- @Test void testStartsWithFunction() {
+ /** Tests the {@code STARTS_WITH} and {@code STARTSWITH} operator. */
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("startsWithAliases")
+ void testStartsWithFunction2(FunctionAlias functionAlias) {
final SqlOperatorFixture f0 = fixture();
-f0.setFor(SqlLibraryOperators.STARTS_WITH);
+final SqlFunction function = functionAlias.function;
+f0.setFor(function);
+final String alias = functionAlias.alias;
final Consumer consumer = f -> {
- f.checkBoolean("starts_with('12345', '123')", true);
- f.checkBoolean("starts_with('12345', '1243')", false);
- f.checkBoolean("starts_with(x'11', x'11')", true);
- f.checkBoolean("starts_with(x'112211', x'33')", false);
- f.checkFails("^starts_with('aabbcc', x'aa')^",
- "Cannot apply 'STARTS_WITH' to arguments of type "
- + "'STARTS_WITH\\(, \\)'\\.
Supported "
- + "form\\(s\\): 'STARTS_WITH\\(, \\)'",
+ f.checkBoolean(alias + "('12345', '123')", true);
Review Comment:
Your proposal works for standard function syntax, but sql is a weird
language. But your proposal may be good enough for most use cases.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
snuyanzin commented on code in PR #3590:
URL: https://github.com/apache/calcite/pull/3590#discussion_r1434637134
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -8897,56 +8898,44 @@ private void testCurrentDateFunc(Pair pair) {
f0.forEachLibrary(list(SqlLibrary.BIG_QUERY, SqlLibrary.ORACLE), consumer);
}
- @Test void testStartsWithFunction() {
+ /** Tests the {@code STARTS_WITH} and {@code STARTSWITH} operator. */
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("startsWithAliases")
+ void testStartsWithFunction2(FunctionAlias functionAlias) {
final SqlOperatorFixture f0 = fixture();
-f0.setFor(SqlLibraryOperators.STARTS_WITH);
+final SqlFunction function = functionAlias.function;
+f0.setFor(function);
+final String alias = functionAlias.alias;
final Consumer consumer = f -> {
- f.checkBoolean("starts_with('12345', '123')", true);
- f.checkBoolean("starts_with('12345', '1243')", false);
- f.checkBoolean("starts_with(x'11', x'11')", true);
- f.checkBoolean("starts_with(x'112211', x'33')", false);
- f.checkFails("^starts_with('aabbcc', x'aa')^",
- "Cannot apply 'STARTS_WITH' to arguments of type "
- + "'STARTS_WITH\\(, \\)'\\.
Supported "
- + "form\\(s\\): 'STARTS_WITH\\(, \\)'",
+ f.checkBoolean(alias + "('12345', '123')", true);
Review Comment:
Do we need then `"%function%`?
May be just name this method `for` or `applyFor` and use it like
```java
f.checkBoolean(alias.applyFor("('12345', '1234')"));
```
?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
sonarcloud[bot] commented on PR #3590: URL: https://github.com/apache/calcite/pull/3590#issuecomment-1867101890 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3590) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3590=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3590=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3590=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3590=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3590) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6174] Upgrade gradle from 7.6.1 to 8.5 [calcite]
snuyanzin commented on code in PR #3591: URL: https://github.com/apache/calcite/pull/3591#discussion_r1434619460 ## gradle.properties: ## @@ -174,3 +174,5 @@ xercesImpl.version=2.9.1 systemProp.sonar.organization=apache systemProp.sonar.projectKey=apache_calcite systemProp.sonar.host.url=https://sonarcloud.io + +kotlin.jvm.target.validation.mode = IGNORE Review Comment: This is a result of a weird Kotlin bug https://youtrack.jetbrains.com/issue/KT-48745 which is unlikely to be fixed -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6138] Parser does not accept TIMESTAMP WITH TIME ZONE as a data type [calcite]
mihaibudiu commented on PR #3569: URL: https://github.com/apache/calcite/pull/3569#issuecomment-1867088799 I have simplified the naming of the types, as suggested by @julianhyde in a JIRA discussion for the uber-issue 208 that this PR is a part of: https://issues.apache.org/jira/browse/CALCITE-208 This is ready for review. If you were waiting for time with timezones, this is your chance to get it! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
mihaibudiu commented on code in PR #3590:
URL: https://github.com/apache/calcite/pull/3590#discussion_r1434613012
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -8897,56 +8898,44 @@ private void testCurrentDateFunc(Pair pair) {
f0.forEachLibrary(list(SqlLibrary.BIG_QUERY, SqlLibrary.ORACLE), consumer);
}
- @Test void testStartsWithFunction() {
+ /** Tests the {@code STARTS_WITH} and {@code STARTSWITH} operator. */
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("startsWithAliases")
+ void testStartsWithFunction2(FunctionAlias functionAlias) {
final SqlOperatorFixture f0 = fixture();
-f0.setFor(SqlLibraryOperators.STARTS_WITH);
+final SqlFunction function = functionAlias.function;
+f0.setFor(function);
+final String alias = functionAlias.alias;
final Consumer consumer = f -> {
- f.checkBoolean("starts_with('12345', '123')", true);
- f.checkBoolean("starts_with('12345', '1243')", false);
- f.checkBoolean("starts_with(x'11', x'11')", true);
- f.checkBoolean("starts_with(x'112211', x'33')", false);
- f.checkFails("^starts_with('aabbcc', x'aa')^",
- "Cannot apply 'STARTS_WITH' to arguments of type "
- + "'STARTS_WITH\\(, \\)'\\.
Supported "
- + "form\\(s\\): 'STARTS_WITH\\(, \\)'",
+ f.checkBoolean(alias + "('12345', '123')", true);
Review Comment:
Another possible suggestion is to have the alias implement a "rewrite"
method which rewrites a specific template. Then you can write tests like
`f.checkBoolean(alias.rewrite("%function%('12345', '1234')"))`. But I don't
know if the readability of the test improves much.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
julianhyde commented on code in PR #3590:
URL: https://github.com/apache/calcite/pull/3590#discussion_r1434597746
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -13686,6 +13673,31 @@ void testCastTruncates(CastType castType,
SqlOperatorFixture f) {
}
}
+ static class AliasInfo {
+private final SqlFunction function;
+private final String alias;
+private final SqlLibrary[] libraries;
+
+private AliasInfo(SqlFunction function, String alias, SqlLibrary[]
libraries) {
Review Comment:
Use a better name than `AliasInfo`. Info means data, after all. Maybe
`FunctionAlias`. Also, move it to the end of the file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
julianhyde commented on PR #3590: URL: https://github.com/apache/calcite/pull/3590#issuecomment-1867061770 @mihaibudiu The danger with your approach is that, by working on the AST, you automatically fix errors. E.g. you might accidentally change STARTSWITH to STARTS_WITH and not notice that STARTSWITH is a reserved word. Or the unparser might convert things to upper-case. For this, I favor a brute force approach that generates strings. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
julianhyde commented on code in PR #3590:
URL: https://github.com/apache/calcite/pull/3590#discussion_r1434594909
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -8897,56 +8898,42 @@ private void testCurrentDateFunc(Pair pair) {
f0.forEachLibrary(list(SqlLibrary.BIG_QUERY, SqlLibrary.ORACLE), consumer);
}
- @Test void testStartsWithFunction() {
Review Comment:
Need a comment saying that it tests STARTS_WITH and STARTSWITH.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
julianhyde commented on code in PR #3590:
URL: https://github.com/apache/calcite/pull/3590#discussion_r1434594471
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -13686,6 +13673,31 @@ void testCastTruncates(CastType castType,
SqlOperatorFixture f) {
}
}
+ static class AliasInfo {
+private final SqlFunction function;
+private final String alias;
+private final SqlLibrary[] libraries;
Review Comment:
Constructor parameter should be varargs, not array.
`libraries` field should be immutable list, not array.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
mihaibudiu commented on PR #3590: URL: https://github.com/apache/calcite/pull/3590#issuecomment-1867056620 You leave the queries unchanged, and then you use a shuttle to replace the use of `starts_with` with `startswith`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6172] PoC for parameterized tests [calcite]
snuyanzin commented on PR #3590: URL: https://github.com/apache/calcite/pull/3590#issuecomment-1867055416 I didn't get how shuttle could help with different `SQLLibraries` per alias... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[PR] [CALCITE-3522] Sql validator limits decimal literals to 64 bits and [CALCITE-6169] EnumUtils.convert does not implement the correct SQL cast semantics [calcite]
mihaibudiu opened a new pull request, #3589: URL: https://github.com/apache/calcite/pull/3589 I initially started with the goal of fixing just [CALCITE-3522], https://issues.apache.org/jira/browse/CALCITE-3522, which is limiting the size of decimal literals to 64 bits. The fix is to actually check the precision of the type system when rejecting a decimal literal. That's easy. I am not checking the scale, under the assumption that rounding is OK for literals but losing precision is not. However, fixing this problem unmasked a bunch of other bugs in handling large values, so in order to make the tests pass I had to also fix [CALCITE-6169], which is about incorrect casts between numeric values. There is still one case which is not handled, which is casts between decimal values that change the scale of the result, but I plan to do that in a separate PR (because I also have to think about it more.) You will notice that this fix re-enables some tests that were disabled a long time ago. I think that disabling failing tests is not a good practice, and should be avoided. There are still some disabled tests in these categories, but strictly fewer than before. I couldn't make this into 2 separate PRs that both keep all tests running. I am not 100% happy with the code, in particular with the implementation of the new Expression `ConvertChecked`. The implementation (which is in the unparse method...) in fact just calls the existing code in `Primitive` which handles numeric casts. I think that's better than duplicating the code. There is already a very large number of places where these conversions are handled, which is shown by the many files that I had to patch. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6156] Add ENDSWITH, STARTSWITH functions (enabled in Postgres, Snowflake libraries) [calcite]
tanclary merged PR #3565: URL: https://github.com/apache/calcite/pull/3565 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(calcite) branch main updated: [CALCITE-6156] Add ENDSWITH, STARTSWITH functions (enabled in Postgres, Snowflake libraries)
This is an automated email from the ASF dual-hosted git repository.
tanner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new e74ff3eaa0 [CALCITE-6156] Add ENDSWITH, STARTSWITH functions (enabled
in Postgres, Snowflake libraries)
e74ff3eaa0 is described below
commit e74ff3eaa0a339ad3d099739780ce05a40efbe87
Author: Tanner Clary
AuthorDate: Mon Dec 4 17:00:41 2023 -0800
[CALCITE-6156] Add ENDSWITH, STARTSWITH functions (enabled in Postgres,
Snowflake libraries)
---
.../main/java/org/apache/calcite/sql/SqlKind.java | 6 +
.../calcite/sql/dialect/SnowflakeSqlDialect.java | 22
.../org/apache/calcite/sql/fun/SqlLibrary.java | 4 +-
.../calcite/sql/fun/SqlLibraryOperators.java | 29 +++--
.../calcite/sql2rel/StandardConvertletTable.java | 2 +
.../calcite/rel/rel2sql/RelToSqlConverterTest.java | 34 +
.../java/org/apache/calcite/util/UtilTest.java | 8 +-
site/_docs/reference.md| 7 +-
.../org/apache/calcite/test/SqlOperatorTest.java | 139 ++---
9 files changed, 192 insertions(+), 59 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/sql/SqlKind.java
b/core/src/main/java/org/apache/calcite/sql/SqlKind.java
index 35034ca910..3b11ac5cec 100644
--- a/core/src/main/java/org/apache/calcite/sql/SqlKind.java
+++ b/core/src/main/java/org/apache/calcite/sql/SqlKind.java
@@ -806,6 +806,12 @@ public enum SqlKind {
/** {@code SUBSTR} function (PostgreSQL semantics). */
SUBSTR_POSTGRESQL,
+ /** {@code ENDS_WITH} function. */
+ ENDS_WITH,
+
+ /** {@code STARTS_WITH} function. */
+ STARTS_WITH,
+
/** Call to a function using JDBC function syntax. */
JDBC_FN,
diff --git
a/core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java
b/core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java
index b12e647488..32c2874298 100644
--- a/core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java
+++ b/core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java
@@ -17,7 +17,11 @@
package org.apache.calcite.sql.dialect;
import org.apache.calcite.avatica.util.Casing;
+import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlDialect;
+import org.apache.calcite.sql.SqlWriter;
+import org.apache.calcite.sql.fun.SqlLibraryOperators;
+import org.apache.calcite.sql.parser.SqlParserPos;
/**
* A SqlDialect implementation for the Snowflake database.
@@ -36,6 +40,24 @@ public class SnowflakeSqlDialect extends SqlDialect {
super(context);
}
+ @Override public void unparseCall(final SqlWriter writer, final SqlCall
call, final int leftPrec,
+ final int rightPrec) {
+switch (call.getKind()) {
+case ENDS_WITH:
+ SqlCall endsWithCall = SqlLibraryOperators.ENDSWITH
+ .createCall(SqlParserPos.ZERO, call.getOperandList());
+ super.unparseCall(writer, endsWithCall, leftPrec, rightPrec);
+ break;
+case STARTS_WITH:
+ SqlCall startsWithCall = SqlLibraryOperators.STARTSWITH
+ .createCall(SqlParserPos.ZERO, call.getOperandList());
+ super.unparseCall(writer, startsWithCall, leftPrec, rightPrec);
+ break;
+default:
+ super.unparseCall(writer, call, leftPrec, rightPrec);
+}
+ }
+
@Override public boolean supportsApproxCountDistinct() {
return true;
}
diff --git a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java
index 3b75d1a763..3f95150608 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java
@@ -71,6 +71,8 @@ public enum SqlLibrary {
/** A collection of operators that are in PostgreSQL but not in standard
* SQL. */
POSTGRESQL("p", "postgresql"),
+ /** A collection of operators that are in Snowflake but not in standard SQL.
*/
+ SNOWFLAKE("f", "snowflake"),
/** A collection of operators that are in Apache Spark but not in standard
* SQL. */
SPARK("s", "spark");
@@ -97,7 +99,7 @@ public enum SqlLibrary {
switch (this) {
case ALL:
return ImmutableList.of(BIG_QUERY, CALCITE, HIVE, MSSQL, MYSQL, ORACLE,
- POSTGRESQL, SPARK);
+ POSTGRESQL, SNOWFLAKE, SPARK);
default:
return ImmutableList.of();
}
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
index 0abce560e5..7f113cc96d 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java
@@ -67,6 +67,7 @@ import static org.apache.calcite.sql.fun.SqlLibrary.MSSQL;
import static org.apache.calcite.sql.fun.SqlLibrary.MYSQL;
import static
Re: [PR] [CALCITE-6156] Add ENDSWITH, STARTSWITH functions (enabled in Postgres, Snowflake libraries) [calcite]
sonarcloud[bot] commented on PR #3565: URL: https://github.com/apache/calcite/pull/3565#issuecomment-1866979251 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3565) **Quality Gate passed** Kudos, no new issues were introduced! [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3565=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3565=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3565=new_coverage=list) [45.5% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3565=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3565) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6156] Add ENDSWITH, STARTSWITH functions (enabled in Postgres, Snowflake libraries) [calcite]
tanclary commented on code in PR #3565:
URL: https://github.com/apache/calcite/pull/3565#discussion_r1434526658
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -8878,51 +8878,106 @@ private void testCurrentDateFunc(Pair pair) {
}
@Test void testStartsWithFunction() {
-final SqlOperatorFixture f = fixture().withLibrary(SqlLibrary.BIG_QUERY);
-f.setFor(SqlLibraryOperators.STARTS_WITH);
-f.checkBoolean("starts_with('12345', '123')", true);
-f.checkBoolean("starts_with('12345', '1243')", false);
-f.checkBoolean("starts_with(x'11', x'11')", true);
-f.checkBoolean("starts_with(x'112211', x'33')", false);
-f.checkFails("^starts_with('aabbcc', x'aa')^",
-"Cannot apply 'STARTS_WITH' to arguments of type "
-+ "'STARTS_WITH\\(, \\)'\\. Supported "
-+ "form\\(s\\): 'STARTS_WITH\\(, \\)'",
-false);
-f.checkNull("starts_with(null, null)");
-f.checkNull("starts_with('12345', null)");
-f.checkNull("starts_with(null, '123')");
-f.checkBoolean("starts_with('', '123')", false);
-f.checkBoolean("starts_with('', '')", true);
-f.checkNull("starts_with(x'aa', null)");
-f.checkNull("starts_with(null, x'aa')");
-f.checkBoolean("starts_with(x'1234', x'')", true);
-f.checkBoolean("starts_with(x'', x'123456')", false);
-f.checkBoolean("starts_with(x'', x'')", true);
+final SqlOperatorFixture f0 = fixture();
Review Comment:
Yeah I was thinking this too.
I filed https://issues.apache.org/jira/browse/CALCITE-6172 in the meantime
while I think about it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6156] Add ENDSWITH, STARTSWITH functions (enabled in Postgres, Snowflake libraries) [calcite]
julianhyde commented on code in PR #3565:
URL: https://github.com/apache/calcite/pull/3565#discussion_r1434504495
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -8878,51 +8878,106 @@ private void testCurrentDateFunc(Pair pair) {
}
@Test void testStartsWithFunction() {
-final SqlOperatorFixture f = fixture().withLibrary(SqlLibrary.BIG_QUERY);
-f.setFor(SqlLibraryOperators.STARTS_WITH);
-f.checkBoolean("starts_with('12345', '123')", true);
-f.checkBoolean("starts_with('12345', '1243')", false);
-f.checkBoolean("starts_with(x'11', x'11')", true);
-f.checkBoolean("starts_with(x'112211', x'33')", false);
-f.checkFails("^starts_with('aabbcc', x'aa')^",
-"Cannot apply 'STARTS_WITH' to arguments of type "
-+ "'STARTS_WITH\\(, \\)'\\. Supported "
-+ "form\\(s\\): 'STARTS_WITH\\(, \\)'",
-false);
-f.checkNull("starts_with(null, null)");
-f.checkNull("starts_with('12345', null)");
-f.checkNull("starts_with(null, '123')");
-f.checkBoolean("starts_with('', '123')", false);
-f.checkBoolean("starts_with('', '')", true);
-f.checkNull("starts_with(x'aa', null)");
-f.checkNull("starts_with(null, x'aa')");
-f.checkBoolean("starts_with(x'1234', x'')", true);
-f.checkBoolean("starts_with(x'', x'123456')", false);
-f.checkBoolean("starts_with(x'', x'')", true);
+final SqlOperatorFixture f0 = fixture();
Review Comment:
I wish there was a way to create tests for functions that are aliases (e.g.
starts_with and startswith, length and char_length) that did not use
copy-paste. Any ideas?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] [CALCITE-6165] Add DATE_SUB test and DATE_DIFF test on SqlOperatorTest [calcite]
sonarcloud[bot] commented on PR #3586: URL: https://github.com/apache/calcite/pull/3586#issuecomment-1866441880 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3586) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3586=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3586=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3586=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3586=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3586) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6156] Add ENDSWITH, STARTSWITH functions (enabled in Postgres, Snowflake libraries) [calcite]
sonarcloud[bot] commented on PR #3565: URL: https://github.com/apache/calcite/pull/3565#issuecomment-1866428817 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3565) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3565=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3565=false=true) [98.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3565=new_coverage=list) [39.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3565=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3565) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6165] Add DATE_SUB test and DATE_DIFF test on SqlOperatorTest [calcite]
sonarcloud[bot] commented on PR #3586: URL: https://github.com/apache/calcite/pull/3586#issuecomment-1866332917 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3586) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3586=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3586=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3586=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3586=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3586) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6165] Add DATE_SUB test and DATE_DIFF test on SqlOperatorTest [calcite]
sonarcloud[bot] commented on PR #3586: URL: https://github.com/apache/calcite/pull/3586#issuecomment-1866326301 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3586) **Quality Gate passed** The SonarCloud Quality Gate passed, but some issues were introduced. [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3586=false=true) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3586=false=true) [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3586=new_coverage=list) [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3586=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3586) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] [CALCITE-6165] Add DATE_SUB test and DATE_DIFF test on SqlOperatorTest [calcite]
caicancai commented on PR #3586: URL: https://github.com/apache/calcite/pull/3586#issuecomment-1866257065 @chucheng92 Regarding the failure information related to date_add(date '2011-02-24', CAST(NULL AS interval)), I will add it in the next pr. Because date_sub does not have a corresponding checkfail, I will check where SqlOperatorTest can add related tests. If If there are more, I will open a jira for discussion. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
(calcite-site) branch main updated: Website deployed from calcite@a816267808f2aea93ef5831783d9d7f0f3dd7d3f
This is an automated email from the ASF dual-hosted git repository. asf-ci-deploy pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite-site.git The following commit(s) were added to refs/heads/main by this push: new 121010a0c Website deployed from calcite@a816267808f2aea93ef5831783d9d7f0f3dd7d3f 121010a0c is described below commit 121010a0c2e28fbb0e597c97a69b46c34095b7d1 Author: zabetak AuthorDate: Thu Dec 21 09:23:21 2023 + Website deployed from calcite@a816267808f2aea93ef5831783d9d7f0f3dd7d3f --- community/index.html | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/community/index.html b/community/index.html index 7d5478b82..05e4a5622 100644 --- a/community/index.html +++ b/community/index.html @@ -136,7 +136,7 @@ Benchao Li (https://people.apache.org/phonebook.html?uid=libenchao;>libenchao) he/him https://github.com/libenchao;>https://github.com/libenchao.png;> ByteDance - PMC + PMC Chair Bertil Chapuis (https://people.apache.org/phonebook.html?uid=bchapuis;>bchapuis) @@ -435,7 +435,7 @@ https://people.apache.org/~zabetak/;>Stamatis Zampetakis (https://people.apache.org/phonebook.html?uid=zabetak;>zabetak) he/him https://github.com/zabetak;>https://github.com/zabetak.png;> Cloudera - PMC Chair + PMC Steven Noels (https://people.apache.org/phonebook.html?uid=stevenn;>stevenn)
(calcite) branch site updated: Site: Switch PMC Chair to Benchao Li
This is an automated email from the ASF dual-hosted git repository. github-bot pushed a commit to branch site in repository https://gitbox.apache.org/repos/asf/calcite.git The following commit(s) were added to refs/heads/site by this push: new 9d75a8faf8 Site: Switch PMC Chair to Benchao Li 9d75a8faf8 is described below commit 9d75a8faf83718ba58ce7568edd1feeabae5bbe6 Author: Stamatis Zampetakis AuthorDate: Thu Dec 21 10:21:00 2023 +0100 Site: Switch PMC Chair to Benchao Li --- site/_data/contributors.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/_data/contributors.yml b/site/_data/contributors.yml index ba0d0961bb..afcc4c519c 100644 --- a/site/_data/contributors.yml +++ b/site/_data/contributors.yml @@ -50,7 +50,7 @@ githubId: libenchao pronouns: he/him org: ByteDance - role: PMC + role: PMC Chair - name: Bertil Chapuis apacheId: bchapuis githubId: bchapuis @@ -308,7 +308,7 @@ githubId: zabetak pronouns: he/him org: Cloudera - role: PMC Chair + role: PMC homepage: https://people.apache.org/~zabetak/ - name: Steven Noels apacheId: stevenn
(calcite) branch main updated: Site: Switch PMC Chair to Benchao Li
This is an automated email from the ASF dual-hosted git repository. zabetak pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git The following commit(s) were added to refs/heads/main by this push: new a816267808 Site: Switch PMC Chair to Benchao Li a816267808 is described below commit a816267808f2aea93ef5831783d9d7f0f3dd7d3f Author: Stamatis Zampetakis AuthorDate: Thu Dec 21 10:21:00 2023 +0100 Site: Switch PMC Chair to Benchao Li --- site/_data/contributors.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/_data/contributors.yml b/site/_data/contributors.yml index 9f650f48e9..528602b4c2 100644 --- a/site/_data/contributors.yml +++ b/site/_data/contributors.yml @@ -50,7 +50,7 @@ githubId: libenchao pronouns: he/him org: ByteDance - role: PMC + role: PMC Chair - name: Bertil Chapuis apacheId: bchapuis githubId: bchapuis @@ -313,7 +313,7 @@ githubId: zabetak pronouns: he/him org: Cloudera - role: PMC Chair + role: PMC homepage: https://people.apache.org/~zabetak/ - name: Steven Noels apacheId: stevenn
