[GitHub] [calcite] libenchao commented on pull request #2868: [CALCITE-5230] Fix PERCENTILE_DISC return type derivation
libenchao commented on PR #2868: URL: https://github.com/apache/calcite/pull/2868#issuecomment-1312638552 > As for PERCENTILE_CONT, based on conversation with Julian in the Jira ticket, there is nothing to be done. I'm not sure about this conclusion, I've checked the SQL standard (2011), 10.9: > If PERCENTILE_CONT is specified, then: > 1) Let ROW0 be the greatest exact numeric value with scale 0 (zero) that is less than or equal > to NVE*(N–1). Let ROWLIT0 be a representing ROW0. > 2) Let ROW1 be the least exact numeric value with scale 0 (zero) that is greater than or equal to NVE*(N–1). Let ROWLIT1 be a representing ROW1. > 3) Let FACTOR be an representing NVE*(N–1)–ROW0. > 4) The result is the result of the > ( WITH TEMPTABLE(X, Y) AS >( SELECT ROW_NUMBER() > OVER (ORDER BY WSP) - 1, TXCOLNAME > FROM TXANAME ) > SELECT CAST ( T0.Y + FACTOR * (T1.Y - T0.Y) AS DT ) FROM TEMPTABLE T0, TEMPTABLE T1 > WHERE T0.ROWNUMBER = ROWLIT0 > AND T1.ROWNUMBER = ROWLIT1 ) We can see that the standard is using `CAST ( T0.Y + FACTOR * (T1.Y - T0.Y) AS DT )` as the result for `PERCENTILE_COUT`, whose type is the same as the sort key. And also I've checked some databases, and found: [oracle](https://docs.oracle.com/cd/B19306_01/server.102/b14200/functions110.htm) and [postgrelsql](https://www.postgresql.org/docs/9.4/functions-aggregate.html) are consistent with the standard. There are some databases, such as [server sql](https://learn.microsoft.com/en-us/sql/t-sql/functions/percentile-cont-transact-sql?view=sql-server-ver16), are declaring the return type which is not consistent with the standard. Hence, IMO, we should make it consistent with the standard by default. Of course if could be extensible for some dialects, if we want to achieve that, maybe the proper place to add the extension is in `RelDataTypeSystem` -- 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
[GitHub] [calcite] libenchao commented on a diff in pull request #2929: [CALCITE-5310] JSON_OBJECT in scalar sub-query throws AssertionError
libenchao commented on code in PR #2929:
URL: https://github.com/apache/calcite/pull/2929#discussion_r1020832236
##
core/src/main/java/org/apache/calcite/sql2rel/RelDecorrelator.java:
##
@@ -1790,10 +1790,11 @@ private RexNode createCaseExpression(
@Override public RexNode visitLiteral(RexLiteral literal) {
// Use nullIndicator to decide whether to project null.
- // Do nothing if the literal is null.
+ // Do nothing if the literal is null or symbol.
if (!RexUtil.isNull(literal)
Review Comment:
We are using `LEFT` join type for scalar sub-query decorrelating, see here:
https://github.com/apache/calcite/blob/7277e53adea98e7dd8477d5a47e728aca6d8f680/core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java#L136.
Hence, in this place, we'll wrap all expressions into case expression.
--
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
[GitHub] [calcite] julianhyde commented on a diff in pull request #2965: [CALCITE-5105] Add MEASURE type and AGGREGATE aggregate function
julianhyde commented on code in PR #2965:
URL: https://github.com/apache/calcite/pull/2965#discussion_r1020820936
##
core/src/main/java/org/apache/calcite/sql/validate/SqlValidatorUtil.java:
##
@@ -1287,6 +1287,20 @@ private static FlatAggregate flattenRecurse(@Nullable
SqlCall filterCall,
}
}
+ /** Returns whether a select item is a measure. */
+ public static boolean isMeasure(SqlNode selectItem) {
+return getMeasure(selectItem) != null;
+ }
+
+ /** Returns the measure expression if a select item is a measure, null
+ * otherwise.
+ *
+ * For a measure, {@code selectItem} will have the form
+ * {@code AS(MEASURE(exp), alias)} and this method returns {@code exp}. */
+ public static @Nullable SqlNode getMeasure(SqlNode selectItem) {
+return null;
Review Comment:
Yes, it will be extended in
[CALCITE-4496](https://issues.apache.org/jira/browse/CALCITE-4496). I added a
note.
--
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
[GitHub] [calcite] julianhyde commented on a diff in pull request #2965: [CALCITE-5105] Add MEASURE type and AGGREGATE aggregate function
julianhyde commented on code in PR #2965:
URL: https://github.com/apache/calcite/pull/2965#discussion_r1020820690
##
core/src/main/java/org/apache/calcite/sql/type/ApplySqlType.java:
##
@@ -0,0 +1,56 @@
+/*
+ * 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.sql.type;
+
+import org.apache.calcite.linq4j.Ord;
+import org.apache.calcite.rel.type.RelDataType;
+
+import com.google.common.collect.ImmutableList;
+
+/** Type that applies generic type to type parameters. */
+abstract class ApplySqlType extends AbstractSqlType {
+ protected final ImmutableList types;
+
+ ApplySqlType(SqlTypeName typeName, boolean isNullable,
+ Iterable types) {
+super(typeName, isNullable, null);
+this.types = ImmutableList.copyOf(types);
Review Comment:
It's possible that 0 type arguments makes sense. So I don't think we should
add the check.
--
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
[GitHub] [calcite] julianhyde commented on a diff in pull request #2965: [CALCITE-5105] Add MEASURE type and AGGREGATE aggregate function
julianhyde commented on code in PR #2965: URL: https://github.com/apache/calcite/pull/2965#discussion_r1020820454 ## site/_docs/reference.md: ## @@ -49,6 +49,13 @@ here to appease testAllFunctionsAreDocumented: | SUCCEEDS | Documented as a period operator | TABLE | Documented as part of FROM syntax | VARIANCE() | In SqlStdOperatorTable, but not fully implemented + +Dialect-specific: + +| C | Function | Reason not documented +|:--|:-- |:- +| c | AGGREGATE(m) | TODO: document; also AS MEASURE Review Comment: Good point. I've added the code 'c' now, but not much explanation of measures at this point. -- 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
[GitHub] [calcite-avatica] snuyanzin opened a new pull request, #196: [CALCITE-5379] Upgrade protobuf to 3.21.9
snuyanzin opened a new pull request, #196: URL: https://github.com/apache/calcite-avatica/pull/196 The PR upgrades protobuf version to avoid CVE-2022-3171 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@calcite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[calcite-avatica] branch main updated (8d5eb143e -> 00c60533d)
This is an automated email from the ASF dual-hosted git repository. snuyanzin pushed a change to branch main in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git from 8d5eb143e [CALCITE-5353] Document new procedure for requesting JIRA accounts and becoming a contributor add 00c60533d [CALCITE-5373] Upgrade bouncycastle to 1.70 No new revisions were added by this update. Summary of changes: gradle.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
[GitHub] [calcite-avatica] snuyanzin merged pull request #194: [CALCITE-5373] Upgrade bouncycastle to 1.70
snuyanzin merged PR #194: URL: https://github.com/apache/calcite-avatica/pull/194 -- 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
[GitHub] [calcite] snuyanzin opened a new pull request, #2966: [CALCITE-5374] Upgrade jackson to 2.14.0
snuyanzin opened a new pull request, #2966: URL: https://github.com/apache/calcite/pull/2966 The PR upgrading jackson to 2.14.0 in 2.14.0 `com.fasterxml.jackson.databind.node.ObjectNode#with` became deprecated after https://github.com/FasterXML/jackson-databind/issues/3568 Since currently such warnings are considered as build error's in current gradle configuration, the PR replaces deprecated methods as well -- 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
[GitHub] [calcite-avatica] snuyanzin commented on pull request #195: [CALCITE-5374] Upgrade jackson to 2.14.0
snuyanzin commented on PR #195: URL: https://github.com/apache/calcite-avatica/pull/195#issuecomment-1312563753 It seems the reason is that in 2.14.0 `com.fasterxml.jackson.databind.node.ObjectNode#with` became deprecated https://github.com/FasterXML/jackson-databind/issues/3568 So first need to update it in Calcite and replace deprecated methods and then could be updated for Avatica. -- 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
