Re: [PR] [CALCITE-6426] Invalid unparse for INT and BIGINT in StarRocksDialect [calcite]
sonarcloud[bot] commented on PR #3808: URL: https://github.com/apache/calcite/pull/3808#issuecomment-2160653474 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3808) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3808=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3808=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3808=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3808=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3808=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3808) -- 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-6434] Specify identifier quoting for HiveSqlDialect and Spar… [calcite]
sonarcloud[bot] commented on PR #3820: URL: https://github.com/apache/calcite/pull/3820#issuecomment-2160586984 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3820) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3820=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3820=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3820=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3820=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3820=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3820) -- 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-6426] Invalid unparse for INT and BIGINT in StarRocksDialect [calcite]
NobiGo commented on PR #3808: URL: https://github.com/apache/calcite/pull/3808#issuecomment-2160537399 Please squash the commits. -- 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-6434] Specify identifier quoting for HiveSqlDialect and Spar… [calcite]
NobiGo opened a new pull request, #3820: URL: https://github.com/apache/calcite/pull/3820 …kSqlDialect -- 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-6426] Invalid unparse for INT and BIGINT in StarRocksDialect [calcite]
sonarcloud[bot] commented on PR #3808: URL: https://github.com/apache/calcite/pull/3808#issuecomment-2160507355 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3808) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3808=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3808=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3808=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3808=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3808=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3808) -- 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-6426] Invalid unparse for INT and BIGINT in StarRocksDialect [calcite]
fanluoo commented on code in PR #3808:
URL: https://github.com/apache/calcite/pull/3808#discussion_r1634664990
##
core/src/main/java/org/apache/calcite/sql/dialect/StarRocksSqlDialect.java:
##
@@ -109,6 +109,20 @@ public StarRocksSqlDialect(Context context) {
@Override public @Nullable SqlNode getCastSpec(RelDataType type) {
switch (type.getSqlTypeName()) {
+case INTEGER:
+ return new SqlDataTypeSpec(
+ new SqlAlienSystemTypeNameSpec(
+ "INT",
+ type.getSqlTypeName(),
+ SqlParserPos.ZERO),
+ SqlParserPos.ZERO);
+case BIGINT:
+ return new SqlDataTypeSpec(
+ new SqlAlienSystemTypeNameSpec(
+ "BIGINT",
Review Comment:
> Hi @fanluoo , AFAIK, If a datatype exists in SqlTypeName, Then we use the
SqlBasicTypeNameSpec, meas this datatype is a basic type in Calcite.
SqlAlienSystemTypeNameSpec represents a type name for an alien system. For
example, UNSIGNED is a built-in type in MySQL which is synonym of INTEGER. and
STRING is synonym of VARCHAR in Spark. UNSIGNED and STRING should used
SqlAlienSystemTypeNameSpec. So BIGINT should used the SqlBasicTypeNameSpec
@NobiGo The modifications have been made
--
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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
caicancai commented on PR #3789: URL: https://github.com/apache/calcite/pull/3789#issuecomment-2159857773 @YiwenWu @NobiGo If you have time, could you help me take a look at this PR? Thank you. -- 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-6352] The map_contains_key function may return true when the key and mapkeytype types are different [calcite]
caicancai commented on code in PR #3786:
URL: https://github.com/apache/calcite/pull/3786#discussion_r1634237742
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -5529,9 +5529,23 @@ public static List mapValues(Map map) {
/** Support the MAP_CONTAINS_KEY function. */
public static Boolean mapContainsKey(Map map, Object key) {
+for (Object mapKey : map.keySet()) {
+ if (isNumericEqual(mapKey, key)) {
+return true;
+ }
+}
return map.containsKey(key);
}
+ private static boolean isNumericEqual(Object obj1, Object obj2) {
+if (obj1 instanceof Number && obj2 instanceof Number) {
+ double num1 = ((Number) obj1).doubleValue();
Review Comment:
OK, I will convert the numeric type to double by default for comparison
--
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-6431] Implement the SINGLE_VALUE aggregation in HiveSqlDiale… [calcite]
mihaibudiu commented on PR #3817: URL: https://github.com/apache/calcite/pull/3817#issuecomment-2159568382 Can you make a separate issue about quoting then? Too many unrelated changes in this PR. -- 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-6431] Implement the SINGLE_VALUE aggregation in HiveSqlDiale… [calcite]
NobiGo commented on PR #3817: URL: https://github.com/apache/calcite/pull/3817#issuecomment-2159567167 > There are lots of changes in the quoting of identifiers. Is this a bug that is being fixed? If so, can it be a separate PR? Yes,If we don't add the quating, Spark will parse failed the alias `$f0`, So we need add this. And Others database dialect almost have added this. Of course, We can separate this PR. -- 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-6008] ARRAY_AGG should returns ARRAY NULL when there's no input rows [calcite]
mihaibudiu commented on PR #3430: URL: https://github.com/apache/calcite/pull/3430#issuecomment-2159337297 @tanclary you asked for changes, are you fine with this PR? -- 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-6392] Support all PostgreSQL 14 date/time patterns for to_da… [calcite]
normanj-bitquill commented on code in PR #3805:
URL: https://github.com/apache/calcite/pull/3805#discussion_r1633827873
##
core/src/main/java/org/apache/calcite/util/format/postgresql/DateStringFormatPattern.java:
##
@@ -0,0 +1,130 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.util.format.postgresql;
+
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.time.DayOfWeek;
+import java.time.Month;
+import java.time.ZonedDateTime;
+import java.time.format.TextStyle;
+import java.util.Locale;
+
+/**
+ * Converts a non numeric value from a string to a datetime component and can
generate
+ * a string representation of of a datetime component from a datetime. An
example is
+ * converting to and from month names.
+ *
+ * @param a type used by java.time to represent a datetime
component
+ * that has a string representation
+ */
+public class DateStringFormatPattern extends StringFormatPattern {
+ /**
+ * Provides an abstraction over datetime components that have string
representations.
+ *
+ * @param a type used by java.time to represent a datetime
component
+ * that has a string representation
+ */
+ private interface DateStringConverter {
+T getValueFromDateTime(ZonedDateTime dateTime);
+
+String getDisplayName(T value, TextStyle textStyle, boolean haveFillMode,
Locale locale);
+ }
+
+ /**
+ * Can convert between a day of week name and the corresponding datetime
component value.
+ */
+ private static class DayOfWeekConverter implements
DateStringConverter {
+@Override public DayOfWeek getValueFromDateTime(ZonedDateTime dateTime) {
+ return dateTime.getDayOfWeek();
+}
+
+@Override public String getDisplayName(DayOfWeek value, TextStyle
textStyle,
+boolean haveFillMode, Locale locale) {
+ final String formattedValue = value.getDisplayName(textStyle, locale);
+
+ if (!haveFillMode && textStyle == TextStyle.FULL) {
+return String.format(locale, "%-9s", formattedValue);
Review Comment:
@mihaibudiu Added comments to explain this in the code.
--
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-6369] Expanding "star" gives ArrayIndexOutOfBoundsException … [calcite]
sonarcloud[bot] commented on PR #3802: URL: https://github.com/apache/calcite/pull/3802#issuecomment-2159274607 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3802) **Quality Gate passed** Issues  [7 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3802=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3802=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3802=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3802=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3802=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3802) -- 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-6369] Expanding "star" gives ArrayIndexOutOfBoundsException … [calcite]
mihaibudiu commented on code in PR #3802:
URL: https://github.com/apache/calcite/pull/3802#discussion_r1633816119
##
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorImpl.java:
##
@@ -736,7 +737,7 @@ private boolean expandStar(List selectItems,
Set aliases,
if (!hasDynamicStruct || Bug.CALCITE_2400_FIXED) {
// If some fields before star identifier,
// we should move offset.
-int offset = calculatePermuteOffset(selectItems);
+int offset = Math.min(calculatePermuteOffset(selectItems),
originalSize);
Review Comment:
I see this is gated on CALCITE_2400.
What is the relationship between this bug and CALCITE-2400?
--
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-6369] Expanding "star" gives ArrayIndexOutOfBoundsException … [calcite]
normanj-bitquill commented on code in PR #3802: URL: https://github.com/apache/calcite/pull/3802#discussion_r1633810394 ## core/src/main/java/org/apache/calcite/sql/validate/DelegatingScope.java: ## @@ -297,8 +298,17 @@ protected void addColumnNames( namespace = map.get(tableName).namespace; break; default: -throw validator.newValidationError(identifier, -RESOURCE.columnAmbiguous(columnName)); +// Allow if all namespaces are the same and the tables are not null Review Comment: @YiwenWu Rebased this change off of the current main branch. As a result, this change is not needed and has been removed. -- 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-2067] RexLiteral cannot represent accurately floating point values, including NaN, Infinity [calcite]
sonarcloud[bot] commented on PR #3663: URL: https://github.com/apache/calcite/pull/3663#issuecomment-2159084319 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3663) **Quality Gate passed** Issues  [13 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3663=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3663=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3663=false=true)  [70.4% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3663=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3663=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3663) -- 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-6352] The map_contains_key function may return true when the key and mapkeytype types are different [calcite]
mihaibudiu commented on code in PR #3786:
URL: https://github.com/apache/calcite/pull/3786#discussion_r1633663931
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -5529,9 +5529,23 @@ public static List mapValues(Map map) {
/** Support the MAP_CONTAINS_KEY function. */
public static Boolean mapContainsKey(Map map, Object key) {
+for (Object mapKey : map.keySet()) {
+ if (isNumericEqual(mapKey, key)) {
+return true;
+ }
+}
return map.containsKey(key);
}
+ private static boolean isNumericEqual(Object obj1, Object obj2) {
+if (obj1 instanceof Number && obj2 instanceof Number) {
+ double num1 = ((Number) obj1).doubleValue();
Review Comment:
As I said, I don't think you should convert the values to double and then
compare them.
--
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-6431] Implement the SINGLE_VALUE aggregation in HiveSqlDiale… [calcite]
mihaibudiu commented on PR #3817: URL: https://github.com/apache/calcite/pull/3817#issuecomment-2158989260 There are lots of changes in the quoting of identifiers. Is this a bug that is being fixed? If so, can it be a separate PR? -- 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-5855] Implement frame exclusion in window functions [calcite]
mihaibudiu commented on PR #3811: URL: https://github.com/apache/calcite/pull/3811#issuecomment-2158912898 @NobiGo if you are satisfied we can merge this -- 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-6352] The map_contains_key function may return true when the key and mapkeytype types are different [calcite]
sonarcloud[bot] commented on PR #3786: URL: https://github.com/apache/calcite/pull/3786#issuecomment-2156686939 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3786) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3786=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3786=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3786=false=true)  [95.8% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3786=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3786=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3786) -- 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-6352] The map_contains_key function may return true when the key and mapkeytype types are different [calcite]
caicancai commented on code in PR #3786:
URL: https://github.com/apache/calcite/pull/3786#discussion_r1590005412
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -5529,9 +5529,23 @@ public static List mapValues(Map map) {
/** Support the MAP_CONTAINS_KEY function. */
public static Boolean mapContainsKey(Map map, Object key) {
+for (Object mapKey : map.keySet()) {
+ if (isNumericEqual(mapKey, key)) {
+return true;
+ }
+}
return map.containsKey(key);
}
+ private static boolean isNumericEqual(Object obj1, Object obj2) {
+if (obj1 instanceof Number && obj2 instanceof Number) {
+ double num1 = ((Number) obj1).doubleValue();
Review Comment:
Or convert BigDecimal for comparison, what do you think
##
core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java:
##
@@ -5529,9 +5529,23 @@ public static List mapValues(Map map) {
/** Support the MAP_CONTAINS_KEY function. */
public static Boolean mapContainsKey(Map map, Object key) {
+for (Object mapKey : map.keySet()) {
+ if (isNumericEqual(mapKey, key)) {
+return true;
+ }
+}
return map.containsKey(key);
}
+ private static boolean isNumericEqual(Object obj1, Object obj2) {
+if (obj1 instanceof Number && obj2 instanceof Number) {
+ double num1 = ((Number) obj1).doubleValue();
Review Comment:
@mihaibudiu Should I add a layer of explicit conversion in case to convert
the types in map and key to the biggest type
--
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-6352] The map_contains_key function may return true when the key and mapkeytype types are different [calcite]
caicancai commented on PR #3786: URL: https://github.com/apache/calcite/pull/3786#issuecomment-2156658739  The number comparison of spark map_contains_key function seems to only support double, not bigdecimal. Calcite's map_contains_key is also only adapted to spark. I think my change is the minimum change. Is it acceptable? What do you think? -- 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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
sonarcloud[bot] commented on PR #3789: URL: https://github.com/apache/calcite/pull/3789#issuecomment-2156655687 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3789) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3789=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3789=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3789=false=true)  [88.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3789=new_coverage=list)  [19.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3789=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3789) -- 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-6380] Casts from INTERVAL and STRING to DECIMAL are incorrect [calcite]
caicancai commented on PR #3790: URL: https://github.com/apache/calcite/pull/3790#issuecomment-2156638333 But there seems to be confusion in the JIRA case. -- 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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
sonarcloud[bot] commented on PR #3789: URL: https://github.com/apache/calcite/pull/3789#issuecomment-2156630504 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3789) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3789=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3789=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3789=false=true)  [88.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3789=new_coverage=list)  [19.6% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3789=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3789) -- 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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
caicancai commented on code in PR #3789:
URL: https://github.com/apache/calcite/pull/3789#discussion_r1632309384
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -645,12 +646,14 @@ Builder populate() {
defineMethod(EXP, BuiltInMethod.EXP.method, NullPolicy.STRICT);
defineMethod(POWER, BuiltInMethod.POWER.method, NullPolicy.STRICT);
defineMethod(ABS, BuiltInMethod.ABS.method, NullPolicy.STRICT);
- defineMethod(LOG2, BuiltInMethod.LOG2.method, NullPolicy.STRICT);
map.put(LN, new LogImplementor());
map.put(LOG, new LogImplementor());
map.put(LOG10, new LogImplementor());
+ map.put(LOG_MYSQLSPARK, new LogMysqlSparkImplementor());
Review Comment:
@normanj-bitquill I'm very sorry for the late reply. I've been very busy
recently. You are correct, but I have a question, why the test can pass
Thanks for reminding me
--
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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
caicancai commented on code in PR #3789:
URL: https://github.com/apache/calcite/pull/3789#discussion_r1632228446
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -645,12 +646,14 @@ Builder populate() {
defineMethod(EXP, BuiltInMethod.EXP.method, NullPolicy.STRICT);
defineMethod(POWER, BuiltInMethod.POWER.method, NullPolicy.STRICT);
defineMethod(ABS, BuiltInMethod.ABS.method, NullPolicy.STRICT);
- defineMethod(LOG2, BuiltInMethod.LOG2.method, NullPolicy.STRICT);
map.put(LN, new LogImplementor());
map.put(LOG, new LogImplementor());
map.put(LOG10, new LogImplementor());
+ map.put(LOG_MYSQLSPARK, new LogMysqlSparkImplementor());
Review Comment:
@normanj-bitquill I'm very sorry for the late reply, I've been very busy
recently. You are correct, but I have a question, but will this have any
impact, because the test can pass.
Please forgive me for ignoring this question before.
--
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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
caicancai commented on code in PR #3789:
URL: https://github.com/apache/calcite/pull/3789#discussion_r1632308915
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -645,12 +646,14 @@ Builder populate() {
defineMethod(EXP, BuiltInMethod.EXP.method, NullPolicy.STRICT);
defineMethod(POWER, BuiltInMethod.POWER.method, NullPolicy.STRICT);
defineMethod(ABS, BuiltInMethod.ABS.method, NullPolicy.STRICT);
- defineMethod(LOG2, BuiltInMethod.LOG2.method, NullPolicy.STRICT);
map.put(LN, new LogImplementor());
map.put(LOG, new LogImplementor());
map.put(LOG10, new LogImplementor());
+ map.put(LOG_MYSQLSPARK, new LogMysqlSparkImplementor());
Review Comment:
fix,
```
public static final SqlFunction LOG_MYSQL =
SqlBasicFunction.create(SqlKind.LOG,
ReturnTypes.DOUBLE_FORCE_NULLABLE,
OperandTypes.NUMERIC_OPTIONAL_NUMERIC);
```
Changing the parameter list can fix this situation
--
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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
caicancai commented on code in PR #3789: URL: https://github.com/apache/calcite/pull/3789#discussion_r1632307334 ## core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java: ## @@ -211,6 +211,7 @@ import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOG2; import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOGICAL_AND; import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOGICAL_OR; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOG_MYSQLSPARK; Review Comment: Bigquery's log function and MySQL's log function are inconsistent in log(0) -- 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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
sonarcloud[bot] commented on PR #3789: URL: https://github.com/apache/calcite/pull/3789#issuecomment-2156605959 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3789) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3789=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3789=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3789=false=true)  [87.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3789=new_coverage=list)  [20.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3789=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3789) -- 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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
caicancai commented on code in PR #3789:
URL: https://github.com/apache/calcite/pull/3789#discussion_r1632228446
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -645,12 +646,14 @@ Builder populate() {
defineMethod(EXP, BuiltInMethod.EXP.method, NullPolicy.STRICT);
defineMethod(POWER, BuiltInMethod.POWER.method, NullPolicy.STRICT);
defineMethod(ABS, BuiltInMethod.ABS.method, NullPolicy.STRICT);
- defineMethod(LOG2, BuiltInMethod.LOG2.method, NullPolicy.STRICT);
map.put(LN, new LogImplementor());
map.put(LOG, new LogImplementor());
map.put(LOG10, new LogImplementor());
+ map.put(LOG_MYSQLSPARK, new LogMysqlSparkImplementor());
Review Comment:
@normanj-bitquill I'm very sorry for the late reply, I've been very busy
recently. You are correct, but I have a question, but will this have any
impact, because the test can pass.
Please forgive me for ignoring this question before.
--
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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
caicancai commented on code in PR #3789: URL: https://github.com/apache/calcite/pull/3789#discussion_r1632287708 ## site/_docs/reference.md: ## @@ -2789,7 +2789,8 @@ In the following: | f s | LEN(string) | Equivalent to `CHAR_LENGTH(string)` | b f s | LENGTH(string) | Equivalent to `CHAR_LENGTH(string)` | h s | LEVENSHTEIN(string1, string2)| Returns the Levenshtein distance between *string1* and *string2* -| b | LOG(numeric1 [, numeric2 ])| Returns the logarithm of *numeric1* to base *numeric2*, or base e if *numeric2* is not present +| b p | LOG(numeric1 [, numeric2 ]) | Returns the logarithm of *numeric1* to base *numeric2*, or base e if *numeric2* is not present +| m s | LOG(numeric1 [, numeric2 ]) | Returns the logarithm of *numeric1* to base *numeric2*, or base e if *numeric2* is not present Review Comment: done ## core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java: ## @@ -2214,6 +,7 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding OperandTypes.NUMERIC, SqlFunctionCategory.NUMERIC); + Review Comment: done -- 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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
caicancai commented on code in PR #3789: URL: https://github.com/apache/calcite/pull/3789#discussion_r1632230470 ## core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java: ## @@ -211,6 +211,7 @@ import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOG2; import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOGICAL_AND; import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOGICAL_OR; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOG_MYSQLSPARK; Review Comment: Is LOG_MYSQL OK? -- 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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
caicancai commented on code in PR #3789:
URL: https://github.com/apache/calcite/pull/3789#discussion_r1632228446
##
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##
@@ -645,12 +646,14 @@ Builder populate() {
defineMethod(EXP, BuiltInMethod.EXP.method, NullPolicy.STRICT);
defineMethod(POWER, BuiltInMethod.POWER.method, NullPolicy.STRICT);
defineMethod(ABS, BuiltInMethod.ABS.method, NullPolicy.STRICT);
- defineMethod(LOG2, BuiltInMethod.LOG2.method, NullPolicy.STRICT);
map.put(LN, new LogImplementor());
map.put(LOG, new LogImplementor());
map.put(LOG10, new LogImplementor());
+ map.put(LOG_MYSQLSPARK, new LogMysqlSparkImplementor());
Review Comment:
@normanj-bitquill I'm very sorry for the late reply, I've been very busy
recently. You are correct, but I have a question, but will this have any
impact, because the test can pass.
Please forgive me for ignoring this question before.
--
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-5855] Implement frame exclusion in window functions [calcite]
sonarcloud[bot] commented on PR #3811: URL: https://github.com/apache/calcite/pull/3811#issuecomment-2156431316 Please retry analysis of this Pull-Request directly on SonarCloud -- 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-5855] Implement frame exclusion in window functions [calcite]
sonarcloud[bot] commented on PR #3811: URL: https://github.com/apache/calcite/pull/3811#issuecomment-2156430167 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3811) **Quality Gate passed** Issues  [13 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3811=false=true)  [74.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3811) -- 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-5855] Implement frame exclusion in window functions [calcite]
sonarcloud[bot] commented on PR #3811: URL: https://github.com/apache/calcite/pull/3811#issuecomment-2156425650 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3811) **Quality Gate passed** Issues  [13 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3811=false=true)  [74.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3811) -- 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-5855] Implement frame exclusion in window functions [calcite]
itiels commented on PR #3811: URL: https://github.com/apache/calcite/pull/3811#issuecomment-2156422082 Thanks for the review @NobiGo , I've fixes the issues. @asolimando I've squashed the commits. Thank you. -- 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-6432] Infinite loop for JoinPushTransitivePredicatesRule [calcite]
sonarcloud[bot] commented on PR #3818: URL: https://github.com/apache/calcite/pull/3818#issuecomment-2156300352 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3818) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3818=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3818=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3818=false=true)  [92.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3818=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3818=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3818) -- 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-6432] Infinite loop for JoinPushTransitivePredicatesRule [calcite]
asdfgh19 opened a new pull request, #3819: URL: https://github.com/apache/calcite/pull/3819 jira: [CALCITE-6432](https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6432) Infinite loop for JoinPushTransitivePredicatesRule when there are multiple project expressions reference the same input field. -- 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-6432] Infinite loop for JoinPushTransitivePredicatesRule [calcite]
asdfgh19 closed pull request #3818: [CALCITE-6432] Infinite loop for JoinPushTransitivePredicatesRule URL: https://github.com/apache/calcite/pull/3818 -- 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-6432] Infinite loop for JoinPushTransitivePredicatesRule [calcite]
asdfgh19 opened a new pull request, #3818: URL: https://github.com/apache/calcite/pull/3818 jira: [CALCITE-6432](https://issues.apache.org/jira/projects/CALCITE/issues/CALCITE-6432) Infinite loop for JoinPushTransitivePredicatesRule when there are multiple project expressions reference the same input field. -- 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-6427] Use a higher precision for DECIMAL intermediate results for some aggregate functions like STDDEV [calcite]
mihaibudiu commented on PR #3809: URL: https://github.com/apache/calcite/pull/3809#issuecomment-2156227991 > > I will try this commit in combination with #3733 to see whether it handles STDDEV. If it does, I will merge this one first, then amend #3733, and then hopefully merge that one too. > > This PR works good with #3733 ? The current version of #3733 includes this pr. This shows that this pr solves the problem for stddev but not for other aggregates like variance. I still want to merge this first. -- 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-6427] Use a higher precision for DECIMAL intermediate results for some aggregate functions like STDDEV [calcite]
mihaibudiu commented on PR #3809: URL: https://github.com/apache/calcite/pull/3809#issuecomment-2156227166 > Use a higher precision for DECIMAL by precision_2 and scala_2, Do we refer to any database or SQL standard？I just want to confirm. This is independent on the database. It is three enumerable implementation. Arguably, some of these choices should be part of the type system. -- 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-6431] Implement the SINGLE_VALUE aggregation in HiveSqlDiale… [calcite]
sonarcloud[bot] commented on PR #3817: URL: https://github.com/apache/calcite/pull/3817#issuecomment-2156227173 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3817) **Quality Gate passed** Issues  [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3817=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3817=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3817=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3817=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3817=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3817) -- 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-6427] Use a higher precision for DECIMAL intermediate results for some aggregate functions like STDDEV [calcite]
mihaibudiu commented on PR #3809: URL: https://github.com/apache/calcite/pull/3809#issuecomment-2156226844 > Strangely, we don't need to change any unit test. That is because casts to decimal have no effect -- 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-6325] Add LOG function (enabled in Mysql and Spark library) [calcite]
NobiGo commented on code in PR #3789: URL: https://github.com/apache/calcite/pull/3789#discussion_r1632131142 ## core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java: ## @@ -211,6 +211,7 @@ import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOG2; import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOGICAL_AND; import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOGICAL_OR; +import static org.apache.calcite.sql.fun.SqlLibraryOperators.LOG_MYSQLSPARK; Review Comment: It is strange we use LOG_MYSQLSPARK. May be LOG_SPARK or LOG_MYSQL. we will add the `libraries` label in SqlLibraryOperators to show the function can work in which database. ## core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java: ## @@ -2214,6 +,7 @@ private static RelDataType deriveTypeMapFromEntries(SqlOperatorBinding opBinding OperandTypes.NUMERIC, SqlFunctionCategory.NUMERIC); + Review Comment: Delete the blank line. ## site/_docs/reference.md: ## @@ -2789,7 +2789,8 @@ In the following: | f s | LEN(string) | Equivalent to `CHAR_LENGTH(string)` | b f s | LENGTH(string) | Equivalent to `CHAR_LENGTH(string)` | h s | LEVENSHTEIN(string1, string2)| Returns the Levenshtein distance between *string1* and *string2* -| b | LOG(numeric1 [, numeric2 ])| Returns the logarithm of *numeric1* to base *numeric2*, or base e if *numeric2* is not present +| b p | LOG(numeric1 [, numeric2 ]) | Returns the logarithm of *numeric1* to base *numeric2*, or base e if *numeric2* is not present +| m s | LOG(numeric1 [, numeric2 ]) | Returns the logarithm of *numeric1* to base *numeric2*, or base e if *numeric2* is not present Review Comment: Need to add a description to illustrate the difference before LOG. -- 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-6427] Use a higher precision for DECIMAL intermediate results for some aggregate functions like STDDEV [calcite]
NobiGo commented on PR #3809: URL: https://github.com/apache/calcite/pull/3809#issuecomment-2156221250 Use a higher precision for DECIMAL by precision*2 and scala*2, Do we refer to any database or SQL standard？I just want to confirm. -- 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-6427] Use a higher precision for DECIMAL intermediate results for some aggregate functions like STDDEV [calcite]
NobiGo commented on PR #3809: URL: https://github.com/apache/calcite/pull/3809#issuecomment-2156220331 > I will try this commit in combination with #3733 to see whether it handles STDDEV. If it does, I will merge this one first, then amend #3733, and then hopefully merge that one too. This PR works good with #3733 ? -- 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-6427] Use a higher precision for DECIMAL intermediate results for some aggregate functions like STDDEV [calcite]
NobiGo commented on PR #3809: URL: https://github.com/apache/calcite/pull/3809#issuecomment-2156219969 Strangely, we don't need to change any unit test. -- 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-5855] Implement frame exclusion in window functions [calcite]
NobiGo commented on code in PR #3811:
URL: https://github.com/apache/calcite/pull/3811#discussion_r1632126797
##
core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java:
##
@@ -66,10 +67,10 @@ public final class LogicalWindow extends Window {
*
* Use {@link #create} unless you know what you're doing.
*
- * @param cluster Cluster
+ * @param cluster Cluster
Review Comment:
Small corrections:There is an extra space here
##
core/src/test/resources/sql/winagg.iq:
##
@@ -839,5 +839,68 @@ EnumerableCalc(expr#0..3=[{inputs}], GENDER=[$t2],
$1=[$t3])
+++
(6 rows)
+!ok
+
+# Test excluding. Verified against Postgres
Review Comment:
Adding the Jira summary is better.
##
site/_docs/history.md:
##
@@ -49,6 +49,8 @@ Guava versions 21.0 to 32.1.3-jre;
other software versions as specified in gradle.properties.
New features
+* [https://issues.apache.org/jira/browse/CALCITE-5855;>CALCITE-5855]
Review Comment:
This file is a template file, we don't need to change this. The release
manager will modify it.
##
core/src/test/resources/sql/winagg.iq:
##
@@ -839,5 +839,68 @@ EnumerableCalc(expr#0..3=[{inputs}], GENDER=[$t2],
$1=[$t3])
+++
(6 rows)
+!ok
+
+# Test excluding. Verified against Postgres
Review Comment:
PostgreSQL may be better.
##
site/_docs/algebra.md:
##
@@ -530,8 +530,6 @@ call and then call its `over()` method, for instance
`count().over()`.
To further modify the `OverCall`, call its methods:
-| Method | Description
Review Comment:
This change is necessary?
--
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-6414] Snowflake JDBC adapter should generate BOOLOR_AGG, BOOLAND_AGG for MAX, MIN on BOOLEAN values [calcite]
NobiGo commented on code in PR #3812:
URL: https://github.com/apache/calcite/pull/3812#discussion_r1632125069
##
core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java:
##
@@ -73,6 +80,30 @@ public SnowflakeSqlDialect(Context context) {
}
}
+ @Override public SqlNode rewriteMaxMinExpr(SqlNode aggCall, RelDataType
relDataType) {
+return rewriteMaxMin(aggCall, relDataType);
+ }
+
+ /**
+ * Helper for rewrites of MAX/MIN.
+ * Snowflake, rewrite as
+ * BOOLOR_AGG/BOOLAND_AGG if the return type is BOOLEAN.
+ */
+ public static SqlNode rewriteMaxMin(SqlNode aggCall, RelDataType
relDataType) {
Review Comment:
Consider adding your description, Add the specific SQL after the rewrite.
--
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-6430] SINGLE_VALUE rewrite to wrong sql when the sub-query r… [calcite]
NobiGo merged PR #3815: URL: https://github.com/apache/calcite/pull/3815 -- 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-6414] Snowflake JDBC adapter should generate BOOLOR_AGG, BOOLAND_AGG for MAX, MIN on BOOLEAN values [calcite]
sonarcloud[bot] commented on PR #3812: URL: https://github.com/apache/calcite/pull/3812#issuecomment-2155997670 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3812) **Quality Gate passed** Issues  [3 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3812=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3812=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3812=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3812=new_coverage=list)  [43.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3812=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3812) -- 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-6429] Arrow adapter should default to the Enumerable convent… [calcite]
asolimando merged PR #3814: URL: https://github.com/apache/calcite/pull/3814 -- 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] Remove conditional stream.iq execution [calcite]
mihaibudiu merged PR #3816: URL: https://github.com/apache/calcite/pull/3816 -- 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-6431] Implement the SINGLE_VALUE aggregation in HiveSqlDiale… [calcite]
sonarcloud[bot] commented on PR #3817: URL: https://github.com/apache/calcite/pull/3817#issuecomment-2155790916 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3817) **Quality Gate passed** Issues  [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3817=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3817=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3817=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3817=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3817=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3817) -- 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-6427] Use a higher precision for DECIMAL intermediate results for some aggregate functions like STDDEV [calcite]
NobiGo commented on PR #3809: URL: https://github.com/apache/calcite/pull/3809#issuecomment-2155784294 > Does anyone want to review this PR? This is blocking some other more important PRs, and it's not too complicated. I will review in the next week. -- 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-6431] Implement the SINGLE_VALUE aggregation in HiveSqlDiale… [calcite]
NobiGo opened a new pull request, #3817: URL: https://github.com/apache/calcite/pull/3817 …ct And SparkSQLDialect -- 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] Remove conditional stream.iq execution [calcite]
sonarcloud[bot] commented on PR #3816: URL: https://github.com/apache/calcite/pull/3816#issuecomment-2155671115 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3816) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3816=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3816=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3816=false=true)  No data about Coverage  No data about Duplication [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3816) -- 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-6430] SINGLE_VALUE rewrite to wrong sql when the sub-query r… [calcite]
sonarcloud[bot] commented on PR #3815: URL: https://github.com/apache/calcite/pull/3815#issuecomment-2155667617 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3815) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3815=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3815=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3815=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3815=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3815=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3815) -- 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] Remove conditional stream.iq execution [calcite]
mihaibudiu opened a new pull request, #3816: URL: https://github.com/apache/calcite/pull/3816 (no comment) -- 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-6427] Use a higher precision for DECIMAL intermediate results for some aggregate functions like STDDEV [calcite]
mihaibudiu commented on PR #3809: URL: https://github.com/apache/calcite/pull/3809#issuecomment-2155589337 Does anyone want to review this PR? This is blocking some other more important PRs, and it's not too complicated. -- 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-6370] AS operator problems with USING clause [calcite]
mihaibudiu merged PR #3797: URL: https://github.com/apache/calcite/pull/3797 -- 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-6311] Support PostgreSQL DATE_PART [calcite]
mihaibudiu merged PR #3794: URL: https://github.com/apache/calcite/pull/3794 -- 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-6430] SINGLE_VALUE rewrite to wrong sql when the sub-query r… [calcite]
sonarcloud[bot] commented on PR #3815: URL: https://github.com/apache/calcite/pull/3815#issuecomment-2154868982 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3815) **Quality Gate passed** Issues  [0 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3815=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3815=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3815=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3815=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3815=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3815) -- 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-6414] Snowflake JDBC adapter should generate BOOLOR_AGG, BOOLAND_AGG for MAX, MIN on BOOLEAN values [calcite]
sonarcloud[bot] commented on PR #3812: URL: https://github.com/apache/calcite/pull/3812#issuecomment-2154389374 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3812) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3812=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3812=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3812=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3812=new_coverage=list)  [44.2% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3812=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3812) -- 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-5156] Support implicit type cast for IN Sub-query [calcite]
NobiGo commented on code in PR #2811:
URL: https://github.com/apache/calcite/pull/2811#discussion_r1218959711
##
core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java:
##
@@ -1704,6 +1704,23 @@ void checkCorrelatedMapSubQuery(boolean expand) {
sql(sql).withExpand(false).ok();
}
+ /** Test case for
+ * https://issues.apache.org/jira/browse/CALCITE-5156;>[CALCITE-5156]
+ * Support implicit type cast for IN Sub-query. */
+ @Test void testInSubQueryWithTypeCast() {
Review Comment:
Ok.I will handle 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-5855] Implement frame exclusion in window functions [calcite]
sonarcloud[bot] commented on PR #3811: URL: https://github.com/apache/calcite/pull/3811#issuecomment-2151426357 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3811) **Quality Gate passed** Issues  [18 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3811=false=true)  [74.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3811) -- 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] Snowflake JDBC adapter should generate BOOLOR_AGG, BOOLAND_AGG for MAX, MIN on BOOLEAN values [calcite]
YiwenWu commented on code in PR #3812:
URL: https://github.com/apache/calcite/pull/3812#discussion_r1628635561
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -14575,6 +14611,42 @@ private static void checkBoolOrFunc(SqlOperatorFixture
f) {
f.checkAgg("bool_or(x)", values4, isNullValue());
}
+ @Test void testBoolOrAggfunc() {
Review Comment:
same as above
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -14527,6 +14527,42 @@ private static void
checkBoolAndFunc(SqlOperatorFixture f) {
f.checkAgg("bool_and(x)", values4, isNullValue());
}
+ @Test void testBoolAndAggFunc() {
Review Comment:
suggestion: can be merged with `SqlOperatorTest#testBoolAndFunc` to verify
the same underlying Function implementation.
Please refer to `SqlOperatorTest#testRlikeOperator`
--
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-5855] Implement frame exclusion in window functions [calcite]
sonarcloud[bot] commented on PR #3811: URL: https://github.com/apache/calcite/pull/3811#issuecomment-2150851782 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3811) **Quality Gate passed** Issues  [18 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3811=false=true)  [74.9% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3811) -- 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-6311] Support PostgreSQL DATE_PART [calcite]
normanj-bitquill commented on PR #3794: URL: https://github.com/apache/calcite/pull/3794#issuecomment-2150804239 @mihaibudiu The commits have been squashed. The CI failure is from running out of heap space in the sonar task. -- 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-5855] Implement frame exclusion in window functions [calcite]
itiels commented on PR #3811: URL: https://github.com/apache/calcite/pull/3811#issuecomment-2150803443 Thanks again @asolimando! I've fixed according to your requests, and fixed some of the sonar cloud issues (some were indeed out of scope). -- 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-5855] Implement frame exclusion in window functions [calcite]
itiels commented on code in PR #3811:
URL: https://github.com/apache/calcite/pull/3811#discussion_r1628116428
##
core/src/main/java/org/apache/calcite/rex/RexWindowExclusion.java:
##
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rex;
+
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlWindow;
+
+/**
+ * Representation of different kinds of exclude clause in window functions.
+ */
+public enum RexWindowExclusion {
+ EXCLUDE_NO_OTHER("EXCLUDE NO OTHER"),
+ EXCLUDE_CURRENT_ROW("EXCLUDE CURRENT ROW"),
+ EXCLUDE_TIES("EXCLUDE TIES"),
+ EXCLUDE_GROUP("EXCLUDE GROUP");
+
+ private final String sql;
+
+ RexWindowExclusion(String sql) {
+this.sql = sql;
+ }
+
+ @Override public String toString() {
+return sql;
+ }
+
+ /**
+ * Creates a window exclusion from a {@link SqlNode}.
+ *
+ * @param node SqlLiteral of the exclusion clause
+ * @return RexWindowExclusion
+ */
+ public static RexWindowExclusion create(SqlNode node) {
+SqlLiteral exclude = (SqlLiteral) node;
+if (SqlWindow.isExcludeCurrentRow(exclude)) {
Review Comment:
Maybe I'm missing something, but I think I can't do that here because I have
`SqlLiteral` and not an Enum.
--
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-5855] Implement frame exclusion in window functions [calcite]
itiels commented on code in PR #3811:
URL: https://github.com/apache/calcite/pull/3811#discussion_r1628113810
##
core/src/main/java/org/apache/calcite/rel/rel2sql/SqlImplementor.java:
##
@@ -958,6 +959,16 @@ public SqlNode toSql(RexWindowBound rexWindowBound) {
}
}
+public SqlLiteral toSql(RexWindowExclusion exclude) {
+ switch (exclude) {
+ case EXCLUDE_CURRENT_ROW: return SqlWindow.createExcludeCurrentRow(POS);
+ case EXCLUDE_GROUP: return SqlWindow.createExcludeGroup(POS);
+ case EXCLUDE_TIES: return SqlWindow.createExcludeTies(POS);
+ }
+ assert exclude == RexWindowExclusion.EXCLUDE_NO_OTHER;
Review Comment:
I've replace this assert with exception.
Regarding generated SQL, we do ignore the exclude in case of `Exclude no
others` in `SqlWindow.unparse`.
--
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-6429] Arrow adapter should default to the Enumerable convent… [calcite]
asolimando opened a new pull request, #3814: URL: https://github.com/apache/calcite/pull/3814 …ion for unsupported filters -- 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-5855] Implement frame exclusion in window functions [calcite]
mihaibudiu commented on code in PR #3811:
URL: https://github.com/apache/calcite/pull/3811#discussion_r1628088990
##
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##
@@ -5780,6 +5780,22 @@ private static Matcher isCharLiteral(String s) {
+ "FROM `BIDS`");
}
+ @Test void testWindowSpecExclusion() {
+// parses correctly
+sql("select sum(x) over (order by x rows between 2 preceding and 2
following exclude group) "
++ "from emp")
+.ok("SELECT (SUM(`X`) OVER (ORDER BY `X` ROWS BETWEEN 2 PRECEDING AND
2 "
++ "FOLLOWING EXCLUDE GROUP))\n"
++ "FROM `EMP`");
+// doesn't parse without frame definition
+sql("select sum(x) over (order by x ^exclude^ current row) from bids")
Review Comment:
@asolimando this is a convention used to indicate the error position
reported by the parser in the statement
--
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-5855] Implement frame exclusion in window functions [calcite]
asolimando commented on code in PR #3811:
URL: https://github.com/apache/calcite/pull/3811#discussion_r1628075025
##
core/src/test/resources/sql/winagg.iq:
##
@@ -839,5 +839,66 @@ EnumerableCalc(expr#0..3=[{inputs}], GENDER=[$t2],
$1=[$t3])
+++
(6 rows)
+!ok
+
+# Test excluding. Verified against Postgres
+!use scott
+
+select empno,
+ avg(comm) over (order by empno rows 2 preceding exclude current row) as e1,
+ avg(comm) over (order by empno rows 2 preceding exclude group) as e2,
+ avg(comm) over (order by empno rows 2 preceding exclude ties) as e3,
+ avg(comm) over (order by empno rows 2 preceding exclude no others) as e4
Review Comment:
Can you also add the same line but without "exclude no others" to verify
they results in the same resultset? (here and in similar added tests)
##
core/src/main/java/org/apache/calcite/rel/logical/LogicalWindow.java:
##
@@ -66,13 +67,13 @@ public final class LogicalWindow extends Window {
*
* Use {@link #create} unless you know what you're doing.
*
- * @param cluster Cluster
- * @param traitSet Trait set
- * @param hints Hints for this node
- * @param input Input relational expression
+ * @param cluster Cluster
Review Comment:
In the codebase we generally don't format javadoc as you do here, can you
please revert these changes?
##
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##
@@ -5780,6 +5780,22 @@ private static Matcher isCharLiteral(String s) {
+ "FROM `BIDS`");
}
+ @Test void testWindowSpecExclusion() {
+// parses correctly
+sql("select sum(x) over (order by x rows between 2 preceding and 2
following exclude group) "
++ "from emp")
+.ok("SELECT (SUM(`X`) OVER (ORDER BY `X` ROWS BETWEEN 2 PRECEDING AND
2 "
++ "FOLLOWING EXCLUDE GROUP))\n"
++ "FROM `EMP`");
+// doesn't parse without frame definition
+sql("select sum(x) over (order by x ^exclude^ current row) from bids")
Review Comment:
Why do we need the `'^'` here?
##
testkit/src/main/java/org/apache/calcite/sql/parser/SqlParserTest.java:
##
@@ -5780,6 +5780,22 @@ private static Matcher isCharLiteral(String s) {
+ "FROM `BIDS`");
}
+ @Test void testWindowSpecExclusion() {
+// parses correctly
+sql("select sum(x) over (order by x rows between 2 preceding and 2
following exclude group) "
++ "from emp")
+.ok("SELECT (SUM(`X`) OVER (ORDER BY `X` ROWS BETWEEN 2 PRECEDING AND
2 "
++ "FOLLOWING EXCLUDE GROUP))\n"
++ "FROM `EMP`");
+// doesn't parse without frame definition
+sql("select sum(x) over (order by x ^exclude^ current row) from bids")
+.fails("(?s).*Encountered \"exclude\".*");
+// EXCLUDE NO OTHERS is the default behavior, and ommited from UNPARSE
+sql("select sum(x) over (order by x rows between 2 preceding and 2
following exclude no others)"
++ " from emp")
+.ok("SELECT (SUM(`X`) OVER (ORDER BY `X` ROWS BETWEEN 2 PRECEDING AND
2 FOLLOWING))\n"
++ "FROM `EMP`");
+ }
Review Comment:
Please add a newline here
##
core/src/main/java/org/apache/calcite/rex/RexWindowExclusion.java:
##
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.calcite.rex;
+
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlWindow;
+
+/**
+ * Representation of different kinds of exclude clause in window functions.
+ */
+public enum RexWindowExclusion {
+ EXCLUDE_NO_OTHER("EXCLUDE NO OTHER"),
+ EXCLUDE_CURRENT_ROW("EXCLUDE CURRENT ROW"),
+ EXCLUDE_TIES("EXCLUDE TIES"),
+ EXCLUDE_GROUP("EXCLUDE GROUP");
+
+ private final String sql;
+
+ RexWindowExclusion(String sql) {
+this.sql = sql;
+ }
+
+ @Override public String toString() {
+return sql;
+ }
+
+ /**
+ * Creates a window exclusion from a {@link SqlNode}.
+ *
+ * @param node SqlLiteral of the exclusion clause
+ * @return RexWindowExclusion
+ */
+ public static RexWindowExclusion create(SqlNode node) {
+SqlLiteral exclude =
Re: [PR] [CALCITE-6370] AS operator problems with USING clause [calcite]
normanj-bitquill commented on PR #3797: URL: https://github.com/apache/calcite/pull/3797#issuecomment-2150473450 @mihaibudiu The changes have been squashed. -- 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-5855] Implement frame exclusion in window functions [calcite]
sonarcloud[bot] commented on PR #3811: URL: https://github.com/apache/calcite/pull/3811#issuecomment-2150463801 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3811) **Quality Gate passed** Issues  [26 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3811=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3811=false=true)  [75.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3811=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3811) -- 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-5855] Implement frame exclusion in window functions [calcite]
itiels commented on code in PR #3811: URL: https://github.com/apache/calcite/pull/3811#discussion_r1628030077 ## core/src/main/java/org/apache/calcite/rex/RexBuilder.java: ## @@ -389,6 +389,7 @@ public RexNode makeOver( ImmutableList orderKeys, RexWindowBound lowerBound, RexWindowBound upperBound, + RexWindowExclusion exclude, Review Comment: Ok. I've pushed the changes, please let me know if that is what you had in mind. -- 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] Snowflake JDBC adapter should generate BOOLOR_AGG, BOOLAND_AGG for MAX, MIN on BOOLEAN values [calcite]
sonarcloud[bot] commented on PR #3812: URL: https://github.com/apache/calcite/pull/3812#issuecomment-2149943059 Please retry analysis of this Pull-Request directly on SonarCloud -- 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] Snowflake JDBC adapter should generate BOOLOR_AGG, BOOLAND_AGG for MAX, MIN on BOOLEAN values [calcite]
im-nitish commented on PR #3812: URL: https://github.com/apache/calcite/pull/3812#issuecomment-2149934669 > @im-nitish Thank you for your contribution. Please fix the error in cli Done. -- 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] Snowflake JDBC adapter should generate BOOLOR_AGG, BOOLAND_AGG for MAX, MIN on BOOLEAN values [calcite]
sonarcloud[bot] commented on PR #3812: URL: https://github.com/apache/calcite/pull/3812#issuecomment-2149920602 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3812) **Quality Gate passed** Issues  [4 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3812=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3812=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3812=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3812=new_coverage=list)  [40.7% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3812=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3812) -- 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-5156] Support implicit type cast for IN Sub-query [calcite]
sonarcloud[bot] commented on PR #2811: URL: https://github.com/apache/calcite/pull/2811#issuecomment-2149740116 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=2811) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=2811=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=2811=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=2811=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=2811=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=2811=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=2811) -- 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-5156] Support implicit type cast for IN Sub-query [calcite]
sonarcloud[bot] commented on PR #2811: URL: https://github.com/apache/calcite/pull/2811#issuecomment-2149685713 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=2811) **Quality Gate passed** Issues  [2 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=2811=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=2811=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=2811=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=2811=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=2811=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=2811) -- 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-5855] Implement frame exclusion in window functions [calcite]
asolimando commented on code in PR #3811: URL: https://github.com/apache/calcite/pull/3811#discussion_r1627279119 ## core/src/main/java/org/apache/calcite/rex/RexBuilder.java: ## @@ -389,6 +389,7 @@ public RexNode makeOver( ImmutableList orderKeys, RexWindowBound lowerBound, RexWindowBound upperBound, + RexWindowExclusion exclude, Review Comment: From a cursory look, the SQL parser still accepts the same syntax (without `exclude` statement) after your modifications. If that's confirmed, then I think there is no need to deprecate the old APIs, as they would basically mimic the same (legal) old behaviour. In a nut-shell, you are adding an optional functionality, what was working before should still be working as-is, it's not a change in behaviour. -- 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-5855] Implement frame exclusion in window functions [calcite]
itiels commented on code in PR #3811:
URL: https://github.com/apache/calcite/pull/3811#discussion_r1627184125
##
core/src/main/java/org/apache/calcite/rel/core/Window.java:
##
@@ -358,7 +365,8 @@ public RelCollation collation() {
public boolean isAlwaysNonEmpty() {
int lowerKey = lowerBound.getOrderKey();
int upperKey = upperBound.getOrderKey();
- return lowerKey > -1 && lowerKey <= upperKey;
+ return lowerKey > -1 && lowerKey <= upperKey
Review Comment:
You are correct, great catch.
--
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-5855] Implement frame exclusion in window functions [calcite]
itiels commented on code in PR #3811: URL: https://github.com/apache/calcite/pull/3811#discussion_r1627181513 ## core/src/main/java/org/apache/calcite/rex/RexBuilder.java: ## @@ -389,6 +389,7 @@ public RexNode makeOver( ImmutableList orderKeys, RexWindowBound lowerBound, RexWindowBound upperBound, + RexWindowExclusion exclude, Review Comment: Thanks for reviewing @mihaibudiu and @asolimando . So should I add overload option for each public method that I've changed? And how do I decide if the old versions should be deprecated or not? -- 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-5855] Implement frame exclusion in window functions [calcite]
asolimando commented on code in PR #3811: URL: https://github.com/apache/calcite/pull/3811#discussion_r1627054403 ## core/src/main/java/org/apache/calcite/rex/RexBuilder.java: ## @@ -389,6 +389,7 @@ public RexNode makeOver( ImmutableList orderKeys, RexWindowBound lowerBound, RexWindowBound upperBound, + RexWindowExclusion exclude, Review Comment: The current policy is to leave the old API with a default value for the new parameter as you suggest. At that point the number of files changed for this PR will significantly drop. If the old API is deprecated after the current changes, you can mark it as such with the `@Deprecated` annotation (see [SqlDrop.java#L37](https://github.com/apache/calcite/blob/3c4d54a82e21ec6926e6aa724ce94ead63d19083/core/src/main/java/org/apache/calcite/sql/SqlDrop.java#L37) for instance). -- 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-5855] Implement frame exclusion in window functions [calcite]
asolimando commented on code in PR #3811: URL: https://github.com/apache/calcite/pull/3811#discussion_r1627060170 ## core/src/main/java/org/apache/calcite/rex/RexBuilder.java: ## @@ -389,6 +389,7 @@ public RexNode makeOver( ImmutableList orderKeys, RexWindowBound lowerBound, RexWindowBound upperBound, + RexWindowExclusion exclude, Review Comment: The current policy is exactly what @mihaibudiu suggests, you should leave the old API around with a default value for the new parameter (exclude no others?), at that point no need to document in `history.md` IMO. By doing this, the number of files to be updated in the scope of this PR will drop. If you want to deprecate the old API, you can do so with the `@Deprecated` annotation (see [SqlDrop.java#L37](https://github.com/apache/calcite/blob/3c4d54a82e21ec6926e6aa724ce94ead63d19083/core/src/main/java/org/apache/calcite/sql/SqlDrop.java#L37) for an example), but I don't think it's necessary here. -- 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-5855] Implement frame exclusion in window functions [calcite]
asolimando commented on code in PR #3811: URL: https://github.com/apache/calcite/pull/3811#discussion_r1627054403 ## core/src/main/java/org/apache/calcite/rex/RexBuilder.java: ## @@ -389,6 +389,7 @@ public RexNode makeOver( ImmutableList orderKeys, RexWindowBound lowerBound, RexWindowBound upperBound, + RexWindowExclusion exclude, Review Comment: The current policy is to leave the old API with a default value for the new parameter as you suggest. At that point the number of files changed for this PR will significantly drop. If the old API is deprecated after the current changes, you can mark it as such with the `@Deprecated` annotation (see [SqlDrop.java#L37](https://github.com/apache/calcite/blob/3c4d54a82e21ec6926e6aa724ce94ead63d19083/core/src/main/java/org/apache/calcite/sql/SqlDrop.java#L37) for instance). -- 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] Fixing [CALCITE-6414], adding support for BOOLOR_AND and BOOLAND_AGG. [calcite]
caicancai commented on PR #3812: URL: https://github.com/apache/calcite/pull/3812#issuecomment-2148726706 @im-nitish Please fix the error in cli -- 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] Fixing [CALCITE-6414], adding support for BOOLOR_AND and BOOLAND_AGG. [calcite]
caicancai commented on code in PR #3812:
URL: https://github.com/apache/calcite/pull/3812#discussion_r1626829656
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -635,6 +635,18 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding
operatorBinding,
public static final SqlAggFunction BOOL_OR =
new SqlMinMaxAggFunction("BOOL_OR", SqlKind.MAX, OperandTypes.BOOLEAN);
+ /** The "BOOLAND_AGG(condition)" aggregate function, Snowflake's
+ * equivalent to {@link SqlStdOperatorTable#EVERY}. */
+ @LibraryOperator(libraries = {SNOWFLAKE})
+ public static final SqlAggFunction BOOLAND_AGG =
Review Comment:
If it is SqlLibraryOperators, I have the same opinion as yiwenwu, can it be
added to SqlOperatorTest
##
core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java:
##
@@ -73,6 +80,30 @@ public SnowflakeSqlDialect(Context context) {
}
}
+ @Override public SqlNode rewriteMaxMinExpr(SqlNode aggCall, RelDataType
relDataType) {
+return rewriteMaxMin(aggCall, relDataType);
+ }
+
+ /**
+ * Helper for rewrites of MAX/MIN.
+ * Snowflake, rewrite as
+ * BOOLOR_AGG/BOOLAND_AGG if the return type is BOOLEAN.
+ */
+ public static SqlNode rewriteMaxMin(SqlNode aggCall, RelDataType
relDataType) {
Review Comment:
You should add your jira link or add description where you tested
--
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] Fixing [CALCITE-6414], adding support for BOOLOR_AND and BOOLAND_AGG. [calcite]
YiwenWu commented on code in PR #3812:
URL: https://github.com/apache/calcite/pull/3812#discussion_r1626821681
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -635,6 +635,18 @@ static RelDataType deriveTypeSplit(SqlOperatorBinding
operatorBinding,
public static final SqlAggFunction BOOL_OR =
new SqlMinMaxAggFunction("BOOL_OR", SqlKind.MAX, OperandTypes.BOOLEAN);
+ /** The "BOOLAND_AGG(condition)" aggregate function, Snowflake's
+ * equivalent to {@link SqlStdOperatorTable#EVERY}. */
+ @LibraryOperator(libraries = {SNOWFLAKE})
+ public static final SqlAggFunction BOOLAND_AGG =
Review Comment:
A question: Is the implementation of RexImpTable missing? and add
corresponding unit tests in `SqlOperatorTest`
--
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-6369] Expanding "star" gives ArrayIndexOutOfBoundsException … [calcite]
YiwenWu commented on code in PR #3802: URL: https://github.com/apache/calcite/pull/3802#discussion_r1626817381 ## core/src/main/java/org/apache/calcite/sql/validate/DelegatingScope.java: ## @@ -297,8 +298,17 @@ protected void addColumnNames( namespace = map.get(tableName).namespace; break; default: -throw validator.newValidationError(identifier, -RESOURCE.columnAmbiguous(columnName)); +// Allow if all namespaces are the same and the tables are not null Review Comment: Without this modification, verified that the two examples of unit tests can also be executed successfully. -- 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-6370] AS operator problems with USING clause [calcite]
sonarcloud[bot] commented on PR #3797: URL: https://github.com/apache/calcite/pull/3797#issuecomment-2148533182 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3797) **Quality Gate passed** Issues  [6 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3797=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3797=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3797=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3797=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3797=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3797) -- 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-6322] Casts to DECIMAL types are ignored [calcite]
sonarcloud[bot] commented on PR #3733: URL: https://github.com/apache/calcite/pull/3733#issuecomment-2148501385 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3733) **Quality Gate passed** Issues  [11 New issues](https://sonarcloud.io/project/issues?id=apache_calcite=3733=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3733=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3733=false=true)  [94.2% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3733=new_coverage=list)  [12.3% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3733=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3733) -- 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-6369] Expanding "star" gives ArrayIndexOutOfBoundsException … [calcite]
normanj-bitquill commented on code in PR #3802: URL: https://github.com/apache/calcite/pull/3802#discussion_r1626659020 ## core/src/main/java/org/apache/calcite/sql/validate/DelegatingScope.java: ## @@ -297,8 +298,17 @@ protected void addColumnNames( namespace = map.get(tableName).namespace; break; default: -throw validator.newValidationError(identifier, -RESOURCE.columnAmbiguous(columnName)); +// Allow if all namespaces are the same and the tables are not null Review Comment: @YiwenWu I'm not sure. If we have a query such as: ``` select x, * from foo f join bar b using (x) ``` Does it matter if we choose `foo.x` or `bar.x` in this context? Due to the join, they should have the same value. -- 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-6427] Use a higher precision for DECIMAL intermediate results for some aggregate functions like STDDEV [calcite]
sonarcloud[bot] commented on PR #3809: URL: https://github.com/apache/calcite/pull/3809#issuecomment-2148490133 ## [](https://sonarcloud.io/dashboard?id=apache_calcite=3809) **Quality Gate passed** Issues  [1 New issue](https://sonarcloud.io/project/issues?id=apache_calcite=3809=false=true)  [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_calcite=3809=WONTFIX) Measures  [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_calcite=3809=false=true)  [100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3809=new_coverage=list)  [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_calcite=3809=new_duplicated_lines_density=list) [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_calcite=3809) -- 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
