You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "snuyanzin (via GitHub)" <gi...@apache.org> on 2025/05/12 06:49:24 UTC
[PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
snuyanzin opened a new pull request, #26547:
URL: https://github.com/apache/flink/pull/26547
## What is the purpose of the change
Upgrade Calcite to 1.35.0
## Brief change log
TBD
## Verifying this change
existing tests
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): (yes )
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: ( no)
- The serializers: ( no)
- The runtime per-record code paths (performance sensitive): (no)
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: ( no)
- The S3 file system connector: ( no)
## Documentation
- Does this pull request introduce a new feature? ( no)
- If yes, how is the feature documented? (not applicable)
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2114886304
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
Review Comment:
looks like with current framework it is not possible to do it...
At least this test requires it JoinWithoutKeyITCase#testNonKeySemi
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #26547:
URL: https://github.com/apache/flink/pull/26547#issuecomment-2871098626
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "8bb4c36c24faee6947a844c5449fbd80d7e9e164",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "8bb4c36c24faee6947a844c5449fbd80d7e9e164",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 8bb4c36c24faee6947a844c5449fbd80d7e9e164 UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2114863909
##########
flink-table/flink-sql-parser/src/main/codegen/templates/Parser.jj:
##########
@@ -8001,6 +8003,8 @@ SqlPostfixOperator PostfixRowOperator() :
| < DATETIME_INTERVAL_PRECISION: "DATETIME_INTERVAL_PRECISION" >
| < DATETIME_TRUNC: "DATETIME_TRUNC" >
| < DAY: "DAY" >
+| < DAYOFWEEK: "DAYOFWEEK" >
Review Comment:
it's already covered in doc
https://github.com/apache/flink/blob/a4e3084ab9220b02bc3eefc70ea372251aec7f07/docs/data/sql_functions.yml#L628-L633
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2083974743
##########
flink-table/flink-sql-parser/pom.xml:
##########
@@ -68,12 +68,12 @@ under the License.
<version>${calcite.version}</version>
<exclusions>
<!--
- "mvn dependency:tree" as of Calcite 1.34.0:
+ "mvn dependency:tree" as of Calcite 1.35.0:
- [INFO] +- org.apache.calcite:calcite-core:jar:1.34.0:compile
+ [INFO] +- org.apache.calcite:calcite-core:jar:1.35.0:compile
[INFO] | +- org.apache.calcite.avatica:avatica-core:jar:1.23.0:compile
[INFO] | +- org.apiguardian:apiguardian-api:jar:1.1.2:compile
- [INFO] | +- org.checkerframework:checker-qual:jar:3.12.0:compile
+ [INFO] | +- org.checkerframework:checker-qual:jar:3.10.0:compile
Review Comment:
this is not related to this commit, however seems at some point the version was downgraded in Flink.
There is no critical changes, so should be 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2114863719
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql/type/FlinkCalciteTableMappingRule.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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 com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+/** Rules that determine whether a type is castable from another type. */
+public class FlinkCalciteTableMappingRule implements SqlTypeMappingRule {
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2114867387
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -265,6 +277,19 @@ private StandardConvertletTable() {
call.operand(0),
SqlLiteral.createExactNumeric("0.5", SqlParserPos.ZERO))));
+ // "STRPOS(string, substring) is equivalent to
+ // "POSITION(substring IN string)"
+ registerOp(
+ SqlLibraryOperators.STRPOS,
+ (cx, call) ->
+ cx.convertExpression(
+ SqlStdOperatorTable.POSITION.createCall(
+ SqlParserPos.ZERO, call.operand(1), call.operand(0))));
+
+ // "INSTR(string, substring, position, occurrence) is equivalent to
Review Comment:
don't think so
existing `position`, `instr` are covered in doc
with other args just fail with validation
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2201708714
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
+ }
+
+ @Override
+ public DataType[] getAggBufferTypes() {
+ return new DataType[] {getResultType()};
+ }
+
+ @Override
+ public Expression[] initialValuesExpressions() {
+ return new Expression[] {nullOf(getResultType())};
+ }
+
+ @Override
+ public Expression[] accumulateExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] retractExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] mergeExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression getValueExpression() {
+ return literal(rexLiteral.getValue(), getResultType());
+ }
+
+ /** Built-in Boolean Literal aggregate function. */
+ public static class BooleanLiteralAggFunction extends LiteralAggFunction {
+
+ public BooleanLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.BOOLEAN();
+ }
+ }
+
+ /** Built-in Byte Literal aggregate function. */
+ public static class ByteLiteralAggFunction extends LiteralAggFunction {
+
+ public ByteLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.TINYINT();
+ }
+ }
+
+ /** Built-in Short Literal aggregate function. */
+ public static class ShortLiteralAggFunction extends LiteralAggFunction {
+
+ public ShortLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.SMALLINT();
+ }
+ }
+
+ /** Built-in Long Literal aggregate function. */
+ public static class LongLiteralAggFunction extends LiteralAggFunction {
Review Comment:
applied this way: turned off `LITERAL_AGG` in this PR by rewriting `SubQueryRemoveRule `
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "twalthr (via GitHub)" <gi...@apache.org>.
twalthr commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2111591075
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java:
##########
@@ -265,6 +277,19 @@ private StandardConvertletTable() {
call.operand(0),
SqlLiteral.createExactNumeric("0.5", SqlParserPos.ZERO))));
+ // "STRPOS(string, substring) is equivalent to
+ // "POSITION(substring IN string)"
+ registerOp(
+ SqlLibraryOperators.STRPOS,
+ (cx, call) ->
+ cx.convertExpression(
+ SqlStdOperatorTable.POSITION.createCall(
+ SqlParserPos.ZERO, call.operand(1), call.operand(0))));
+
+ // "INSTR(string, substring, position, occurrence) is equivalent to
Review Comment:
do we need to document those function synonyms?
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql2rel/SqlToRelConverter.java:
##########
@@ -240,19 +242,18 @@
* <p>FLINK modifications are at lines
*
* <ol>
- * <li>Added in FLINK-29081, FLINK-28682, FLINK-33395: Lines 670 ~ 687
- * <li>Added in Flink-24024: Lines 1463 ~ 1469
- * <li>Added in Flink-24024: Lines 1483 ~ 1522
- * <li>Added in Flink-37269: Lines 2239 ~ 2261
- * <li>Added in FLINK-28682: Lines 2372 ~ 2389
- * <li>Added in FLINK-28682: Lines 2426 ~ 2454
- * <li>Added in FLINK-32474: Lines 2507 ~ 2509
- * <li>Added in FLINK-32474: Lines 2513 ~ 2515
- * <li>Added in FLINK-32474: Lines 2526 ~ 2528
- * <li>Added in FLINK-32474: Lines 2934 ~ 2945
- * <li>Added in FLINK-32474: Lines 3046 ~ 3080
- * <li>Added in FLINK-34312: Lines 5827 ~ 5838
- * <li>Added in FLINK-34057, FLINK-34058, FLINK-34312: Lines 6285 ~ 6303
+ * <li>Added in FLINK-29081, FLINK-28682, FLINK-33395: Lines 673 ~ 690
Review Comment:
Thank you for updating all these lines. Very helpful, but I'm sure also a lot of work!
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql/type/FlinkCalciteTableMappingRule.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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 com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+/** Rules that determine whether a type is castable from another type. */
+public class FlinkCalciteTableMappingRule implements SqlTypeMappingRule {
Review Comment:
`FlinkSqlTypeMappingRule `
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
Review Comment:
Do we actually need to use any state? Can't this be empty array?
##########
flink-table/flink-sql-parser/src/main/codegen/templates/Parser.jj:
##########
@@ -8001,6 +8003,8 @@ SqlPostfixOperator PostfixRowOperator() :
| < DATETIME_INTERVAL_PRECISION: "DATETIME_INTERVAL_PRECISION" >
| < DATETIME_TRUNC: "DATETIME_TRUNC" >
| < DAY: "DAY" >
+| < DAYOFWEEK: "DAYOFWEEK" >
Review Comment:
update documentation, do we also support those in runtime code? or at least throw a proper error message?
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
+ }
+
+ @Override
+ public DataType[] getAggBufferTypes() {
+ return new DataType[] {getResultType()};
+ }
+
+ @Override
+ public Expression[] initialValuesExpressions() {
+ return new Expression[] {nullOf(getResultType())};
+ }
+
+ @Override
+ public Expression[] accumulateExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] retractExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] mergeExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression getValueExpression() {
+ return literal(rexLiteral.getValue(), getResultType());
+ }
+
+ /** Built-in Boolean Literal aggregate function. */
+ public static class BooleanLiteralAggFunction extends LiteralAggFunction {
+
+ public BooleanLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.BOOLEAN();
+ }
+ }
+
+ /** Built-in Byte Literal aggregate function. */
+ public static class ByteLiteralAggFunction extends LiteralAggFunction {
+
+ public ByteLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.TINYINT();
+ }
+ }
+
+ /** Built-in Short Literal aggregate function. */
+ public static class ShortLiteralAggFunction extends LiteralAggFunction {
+
+ public ShortLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.SMALLINT();
+ }
+ }
+
+ /** Built-in Long Literal aggregate function. */
+ public static class LongLiteralAggFunction extends LiteralAggFunction {
Review Comment:
No INT? Are byte and short ever called?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2110243058
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql/type/FlinkCalciteTableMappingRule.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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 com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+/** TODO. */
+public class FlinkCalciteTableMappingRule implements SqlTypeMappingRule {
+ private static final FlinkCalciteTableMappingRule INSTANCE;
Review Comment:
The behavior of `SqlTypeCoercionRule` was changed (https://issues.apache.org/jira/browse/CALCITE-5662) and starting leading to a number of failures in `CastFunctionITCase` .
New mapping rule makes it working in same way as 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "twalthr (via GitHub)" <gi...@apache.org>.
twalthr commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2219250227
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/plan/utils/AggFunctionFactory.scala:
##########
@@ -31,7 +31,10 @@ import org.apache.flink.table.runtime.functions.aggregate.PercentileAggFunction.
import org.apache.flink.table.types.logical._
import org.apache.flink.table.types.logical.LogicalTypeRoot._
+import org.apache.calcite.rel.`type`.RelDataType
import org.apache.calcite.rel.core.AggregateCall
+import org.apache.calcite.rex.{RexLiteral, RexNode}
Review Comment:
undo
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/agg/batch/HashAggCodeGenHelper.scala:
##########
@@ -203,7 +203,7 @@ object HashAggCodeGenHelper {
val initAggCallBufferExprs = aggInfos
.map(_.function.asInstanceOf[DeclarativeAggregateFunction])
- .flatMap(_.initialValuesExpressions)
+ .flatMap(e => e.initialValuesExpressions)
Review Comment:
undo
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/agg/batch/AggCodeGenHelper.scala:
##########
@@ -84,7 +86,7 @@ object AggCodeGenHelper {
.map(index => Array(inputType.getTypeAt(index)))
val aggTypes = aggInfos
- .map(_.externalAccTypes.map(fromDataTypeToLogicalType))
+ .map({ a => a.externalAccTypes.map(fromDataTypeToLogicalType) })
Review Comment:
undo
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2201794672
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/serde/AggregateCallJsonDeserializer.java:
##########
@@ -85,6 +85,7 @@ public AggregateCall deserialize(JsonParser jsonParser, DeserializationContext c
distinct,
approximate,
ignoreNulls,
+ new ArrayList<>(),
Review Comment:
In commit which added this https://github.com/apache/calcite/commit/ddb4200f8f52b02afb1b866abb0785218e2c5994
I was not able to find any test using it...
it is mentioned that it is
>These changes are in preparation for [[CALCITE-4334](https://issues.apache.org/jira/browse/CALCITE-4334)] LITERAL_AGG.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2085206645
##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -3061,34 +3061,6 @@ SqlNode SqlReset() :
}
}
-
-/** Parses a TRY_CAST invocation. */
Review Comment:
Since now `TRY_CAST` comes with Calcite
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2110243058
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql/type/FlinkCalciteTableMappingRule.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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 com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+/** TODO. */
+public class FlinkCalciteTableMappingRule implements SqlTypeMappingRule {
+ private static final FlinkCalciteTableMappingRule INSTANCE;
Review Comment:
The behavior of `SqlTypeCoercionRule` was changed (https://issues.apache.org/jira/browse/CALCITE-5662) and started leading to a number of failures in `CastFunctionITCase` .
New mapping rule makes it working in same way as 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2110288376
##########
flink-table/flink-sql-parser/src/main/codegen/data/Parser.tdd:
##########
@@ -293,6 +292,8 @@
"DATETIME_DIFF"
"DATETIME_INTERVAL_CODE"
"DATETIME_INTERVAL_PRECISION"
+ "DAYOFWEEK"
Review Comment:
keep in sync with Calcite version https://github.com/apache/calcite/blob/75750b78b5ac692caa654f506fc1515d4d3991d6/core/src/main/codegen/templates/Parser.jj#L7963
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2122862409
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
Review Comment:
also rebased on https://github.com/apache/flink/commit/6248ceda1d9c311948fa93a25e452b166b4195f7
in order to make sure it continues passing
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2191951053
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
+ }
+
+ @Override
+ public DataType[] getAggBufferTypes() {
+ return new DataType[] {getResultType()};
+ }
+
+ @Override
+ public Expression[] initialValuesExpressions() {
+ return new Expression[] {nullOf(getResultType())};
+ }
+
+ @Override
+ public Expression[] accumulateExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] retractExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] mergeExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression getValueExpression() {
+ return literal(rexLiteral.getValue(), getResultType());
+ }
+
+ /** Built-in Boolean Literal aggregate function. */
+ public static class BooleanLiteralAggFunction extends LiteralAggFunction {
+
+ public BooleanLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.BOOLEAN();
+ }
+ }
+
+ /** Built-in Byte Literal aggregate function. */
+ public static class ByteLiteralAggFunction extends LiteralAggFunction {
+
+ public ByteLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.TINYINT();
+ }
+ }
+
+ /** Built-in Short Literal aggregate function. */
+ public static class ShortLiteralAggFunction extends LiteralAggFunction {
+
+ public ShortLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.SMALLINT();
+ }
+ }
+
+ /** Built-in Long Literal aggregate function. */
+ public static class LongLiteralAggFunction extends LiteralAggFunction {
Review Comment:
So far (Calcite 1.35) the only place where LiteralAgg might appear is https://github.com/apache/calcite/blob/df3b438ef124838f338fbbd1866c6da61b036890/core/src/main/java/org/apache/calcite/rel/rules/SubQueryRemoveRule.java#L753
no other types are used
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2110253073
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql2rel/AggConverter.java:
##########
@@ -0,0 +1,623 @@
+/*
+ * 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.sql2rel;
+
+import org.apache.flink.table.planner.calcite.FlinkSqlCallBinding;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.calcite.linq4j.Ord;
+import org.apache.calcite.rel.RelCollation;
+import org.apache.calcite.rel.RelCollations;
+import org.apache.calcite.rel.RelFieldCollation;
+import org.apache.calcite.rel.core.AggregateCall;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.runtime.PairList;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlDataTypeSpec;
+import org.apache.calcite.sql.SqlDynamicParam;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlIntervalQualifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlSelectKeyword;
+import org.apache.calcite.sql.SqlUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.util.SqlVisitor;
+import org.apache.calcite.sql.validate.AggregatingSelectScope;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Litmus;
+import org.apache.calcite.util.Util;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import static org.apache.calcite.linq4j.Nullness.castNonNull;
+
+/**
+ * FLINK modifications are at lines
+ *
+ * <ol>
+ * <li>Added in FLINK-34057, FLINK-34058, FLINK-34312: Lines 452 ~ 469
+ * </ol>
+ */
+class AggConverter implements SqlVisitor<Void> {
Review Comment:
The result of this commit https://github.com/apache/calcite/commit/568ce125e6d81fe0815f1317ccaf4d558688d945
where SqlToRelConverter was splitted into 2 classes
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2111067387
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql/type/FlinkCalciteTableMappingRule.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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 com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+/** Rules that determine whether a type is castable from another type. */
+public class FlinkCalciteTableMappingRule implements SqlTypeMappingRule {
+ private static final FlinkCalciteTableMappingRule INSTANCE;
Review Comment:
The behavior of `SqlTypeCoercionRule` was changed (https://issues.apache.org/jira/browse/CALCITE-5662) and started leading to a number of failures in `CastFunctionITCase`.
New mapping rule makes it working in same way as 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2122232666
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
Review Comment:
This method still stays, however all others now return empty collection.
Especially `initialValuesExpressions` which is participated in decision: create/not create accumulator https://github.com/apache/flink/blob/cf55d306f570134e2d3d332a0acba7d8737c87b7/flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/agg/DeclarativeAggCodeGen.scala#L105-L108
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2201786297
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
Review Comment:
removed together with turning off literalAgg
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/agg/batch/AggCodeGenHelper.scala:
##########
@@ -84,7 +90,15 @@ object AggCodeGenHelper {
.map(index => Array(inputType.getTypeAt(index)))
val aggTypes = aggInfos
- .map(_.externalAccTypes.map(fromDataTypeToLogicalType))
+ .map({
+ a =>
+ a.function match {
+ case f: LiteralAggFunction =>
+ Array(fromDataTypeToLogicalType(f.getResultType))
Review Comment:
removed together with turning off literalAgg
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "twalthr (via GitHub)" <gi...@apache.org>.
twalthr commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2200555209
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -40,8 +41,11 @@ public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
private static final Expression[] EMPTY_EXPRS = new Expression[0];
private static final DataType[] EMPTY_DATATYPES = new DataType[0];
+ protected final RexLiteral rexLiteral;
Review Comment:
can be private?
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/plan/utils/AggFunctionFactory.scala:
##########
@@ -285,22 +285,24 @@ class AggFunctionFactory(
}
}
- private def createLiteralAggFunction(relDataType: RelDataType): UserDefinedFunction = {
+ private def createLiteralAggFunction(
+ relDataType: RelDataType,
+ rexNode: RexNode): UserDefinedFunction = {
relDataType.getSqlTypeName match {
case SqlTypeName.BOOLEAN =>
- BooleanLiteralAggFunction.INSTANCE
+ new BooleanLiteralAggFunction(rexNode.asInstanceOf[RexLiteral])
Review Comment:
Let's just have one LiteralAggFunction class that takes the RexLiteral. The result type can be derived from the literal. Then we also don't need this type checking here. Simplifies the code significantly.
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/agg/batch/AggCodeGenHelper.scala:
##########
@@ -84,7 +90,15 @@ object AggCodeGenHelper {
.map(index => Array(inputType.getTypeAt(index)))
val aggTypes = aggInfos
- .map(_.externalAccTypes.map(fromDataTypeToLogicalType))
+ .map({
+ a =>
+ a.function match {
+ case f: LiteralAggFunction =>
+ Array(fromDataTypeToLogicalType(f.getResultType))
Review Comment:
Don't fully understand this logic.
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
+ }
+
+ @Override
+ public DataType[] getAggBufferTypes() {
+ return new DataType[] {getResultType()};
+ }
+
+ @Override
+ public Expression[] initialValuesExpressions() {
+ return new Expression[] {nullOf(getResultType())};
+ }
+
+ @Override
+ public Expression[] accumulateExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] retractExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] mergeExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression getValueExpression() {
+ return literal(rexLiteral.getValue(), getResultType());
+ }
+
+ /** Built-in Boolean Literal aggregate function. */
+ public static class BooleanLiteralAggFunction extends LiteralAggFunction {
+
+ public BooleanLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.BOOLEAN();
+ }
+ }
+
+ /** Built-in Byte Literal aggregate function. */
+ public static class ByteLiteralAggFunction extends LiteralAggFunction {
+
+ public ByteLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.TINYINT();
+ }
+ }
+
+ /** Built-in Short Literal aggregate function. */
+ public static class ShortLiteralAggFunction extends LiteralAggFunction {
+
+ public ShortLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.SMALLINT();
+ }
+ }
+
+ /** Built-in Long Literal aggregate function. */
+ public static class LongLiteralAggFunction extends LiteralAggFunction {
Review Comment:
If it is just this code location that causes all the literal agg trouble. Can't we checkout the old version. Merge this PR first and then work on the literal agg as a followup PR?
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/serde/AggregateCallJsonDeserializer.java:
##########
@@ -85,6 +85,7 @@ public AggregateCall deserialize(JsonParser jsonParser, DeserializationContext c
distinct,
approximate,
ignoreNulls,
+ new ArrayList<>(),
Review Comment:
So this list is not important for Flink? Are we sure we are not loosing any data?
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2110243058
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql/type/FlinkCalciteTableMappingRule.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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 com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+
+/** TODO. */
+public class FlinkCalciteTableMappingRule implements SqlTypeMappingRule {
+ private static final FlinkCalciteTableMappingRule INSTANCE;
Review Comment:
The behavior of `SqlTypeCoercionRule` was changed (https://issues.apache.org/jira/browse/CALCITE-5662) and started leading to a number of failures in `CastFunctionITCase` .
New mapping rule makes it working in same way as 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2201794672
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/plan/nodes/exec/serde/AggregateCallJsonDeserializer.java:
##########
@@ -85,6 +85,7 @@ public AggregateCall deserialize(JsonParser jsonParser, DeserializationContext c
distinct,
approximate,
ignoreNulls,
+ new ArrayList<>(),
Review Comment:
In commit which added this https://github.com/apache/calcite/commit/ddb4200f8f52b02afb1b866abb0785218e2c5994
it is mentioned that it is
>These changes are in preparation for [[CALCITE-4334](https://issues.apache.org/jira/browse/CALCITE-4334)] LITERAL_AGG.
I was not able to find any test using 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2201719497
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -40,8 +41,11 @@ public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
private static final Expression[] EMPTY_EXPRS = new Expression[0];
private static final DataType[] EMPTY_DATATYPES = new DataType[0];
+ protected final RexLiteral rexLiteral;
Review Comment:
removed together with turning off literalAgg
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/plan/utils/AggFunctionFactory.scala:
##########
@@ -285,22 +285,24 @@ class AggFunctionFactory(
}
}
- private def createLiteralAggFunction(relDataType: RelDataType): UserDefinedFunction = {
+ private def createLiteralAggFunction(
+ relDataType: RelDataType,
+ rexNode: RexNode): UserDefinedFunction = {
relDataType.getSqlTypeName match {
case SqlTypeName.BOOLEAN =>
- BooleanLiteralAggFunction.INSTANCE
+ new BooleanLiteralAggFunction(rexNode.asInstanceOf[RexLiteral])
Review Comment:
removed together with turning off literalAgg
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2094070481
##########
flink-table/flink-sql-parser/src/main/codegen/data/Parser.tdd:
##########
@@ -224,7 +224,6 @@
"RESUME"
"TABLES"
"TIMESTAMP_LTZ"
- "TRY_CAST"
Review Comment:
because it was added to keywords on Calcite level, no need to add it one more time
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on PR #26547:
URL: https://github.com/apache/flink/pull/26547#issuecomment-2871312507
@flinkbot run azure
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2114868899
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,166 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.sql.fun.SqlLiteralAggFunction;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+/**
+ * Built-in literal aggregate function. This function is used for internal optimizations. It accepts
+ * zero regular aggregate arguments and returns a constant value. For more details see <a
+ * href="https://issues.apache.org/jira/browse/CALCITE-4334">CALCITE-4334</a> and {@link
+ * SqlLiteralAggFunction}.
+ */
+public abstract class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final UnresolvedReferenceExpression literalAgg = unresolvedRef("literalAgg");
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {literalAgg};
+ }
+
+ @Override
+ public DataType[] getAggBufferTypes() {
+ return new DataType[] {getResultType()};
+ }
+
+ @Override
+ public Expression[] initialValuesExpressions() {
+ return new Expression[] {nullOf(getResultType())};
+ }
+
+ @Override
+ public Expression[] accumulateExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] retractExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression[] mergeExpressions() {
+ return new Expression[] {literal(rexLiteral.getValue(), getResultType())};
+ }
+
+ @Override
+ public Expression getValueExpression() {
+ return literal(rexLiteral.getValue(), getResultType());
+ }
+
+ /** Built-in Boolean Literal aggregate function. */
+ public static class BooleanLiteralAggFunction extends LiteralAggFunction {
+
+ public BooleanLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.BOOLEAN();
+ }
+ }
+
+ /** Built-in Byte Literal aggregate function. */
+ public static class ByteLiteralAggFunction extends LiteralAggFunction {
+
+ public ByteLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.TINYINT();
+ }
+ }
+
+ /** Built-in Short Literal aggregate function. */
+ public static class ShortLiteralAggFunction extends LiteralAggFunction {
+
+ public ShortLiteralAggFunction(RexLiteral rexLiteral) {
+ super(rexLiteral);
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.SMALLINT();
+ }
+ }
+
+ /** Built-in Long Literal aggregate function. */
+ public static class LongLiteralAggFunction extends LiteralAggFunction {
Review Comment:
I added for int as well
however we don't have any test calling anything else except for `boolean`
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2085208902
##########
flink-table/flink-sql-parser/src/main/java/org/apache/flink/sql/parser/validate/FlinkSqlConformance.java:
##########
@@ -168,8 +168,8 @@ public SqlLibrary semantics() {
}
@Override
- public boolean allowCoercionStringToArray() {
- return SqlConformanceEnum.DEFAULT.allowCoercionStringToArray();
+ public boolean allowLenientCoercion() {
Review Comment:
Suddenly was just renamed in Calcite https://github.com/apache/calcite/commit/b0420947ddeb2c9e18afe896dbf7adaf49a715a2
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2084038975
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/aggfunctions/LiteralAggFunction.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.flink.table.planner.functions.aggfunctions;
+
+import org.apache.flink.table.api.DataTypes;
+import org.apache.flink.table.expressions.Expression;
+import org.apache.flink.table.expressions.UnresolvedReferenceExpression;
+import org.apache.flink.table.functions.DeclarativeAggregateFunction;
+import org.apache.flink.table.types.DataType;
+import org.apache.flink.table.types.logical.DecimalType;
+import org.apache.flink.table.types.logical.LocalZonedTimestampType;
+import org.apache.flink.table.types.logical.TimeType;
+import org.apache.flink.table.types.logical.TimestampType;
+
+import org.apache.calcite.rex.RexLiteral;
+
+import static org.apache.flink.table.expressions.ApiExpressionUtils.unresolvedRef;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.literal;
+import static org.apache.flink.table.planner.expressions.ExpressionBuilder.nullOf;
+
+public class LiteralAggFunction extends DeclarativeAggregateFunction {
+
+ private final RexLiteral rexLiteral;
+
+ public LiteralAggFunction(RexLiteral rexLiteral) {
+ this.rexLiteral = rexLiteral;
+ }
+
+ @Override
+ public int operandCount() {
+ return 0;
+ }
+
+ @Override
+ public UnresolvedReferenceExpression[] aggBufferAttributes() {
+ return new UnresolvedReferenceExpression[] {unresolvedRef("literalAgg")};
+ }
+
+ @Override
+ public DataType[] getAggBufferTypes() {
+ return new DataType[] {getResultType()};
+ }
+
+ @Override
+ public DataType getResultType() {
+ return DataTypes.BOOLEAN();
Review Comment:
This is probably the main question: should we have it for different types or boolean is enough?
So far in existing tests there is only boolean in use
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2085206645
##########
flink-table/flink-sql-parser/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -3061,34 +3061,6 @@ SqlNode SqlReset() :
}
}
-
-/** Parses a TRY_CAST invocation. */
Review Comment:
Since now `TRY_CAST` comes with Calcite https://issues.apache.org/jira/browse/CALCITE-4771
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin merged PR #26547:
URL: https://github.com/apache/flink/pull/26547
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-35854][table] Upgrade Calcite version to 1.35.0 [flink]
Posted by "snuyanzin (via GitHub)" <gi...@apache.org>.
snuyanzin commented on code in PR #26547:
URL: https://github.com/apache/flink/pull/26547#discussion_r2110253073
##########
flink-table/flink-table-planner/src/main/java/org/apache/calcite/sql2rel/AggConverter.java:
##########
@@ -0,0 +1,623 @@
+/*
+ * 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.sql2rel;
+
+import org.apache.flink.table.planner.calcite.FlinkSqlCallBinding;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import org.apache.calcite.linq4j.Ord;
+import org.apache.calcite.rel.RelCollation;
+import org.apache.calcite.rel.RelCollations;
+import org.apache.calcite.rel.RelFieldCollation;
+import org.apache.calcite.rel.core.AggregateCall;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.runtime.PairList;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlCall;
+import org.apache.calcite.sql.SqlDataTypeSpec;
+import org.apache.calcite.sql.SqlDynamicParam;
+import org.apache.calcite.sql.SqlIdentifier;
+import org.apache.calcite.sql.SqlIntervalQualifier;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.calcite.sql.SqlLiteral;
+import org.apache.calcite.sql.SqlNode;
+import org.apache.calcite.sql.SqlNodeList;
+import org.apache.calcite.sql.SqlSelectKeyword;
+import org.apache.calcite.sql.SqlUtil;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.parser.SqlParserPos;
+import org.apache.calcite.sql.util.SqlVisitor;
+import org.apache.calcite.sql.validate.AggregatingSelectScope;
+import org.apache.calcite.sql.validate.SqlValidator;
+import org.apache.calcite.sql.validate.SqlValidatorUtil;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.Litmus;
+import org.apache.calcite.util.Util;
+import org.checkerframework.checker.nullness.qual.Nullable;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.stream.Collectors;
+
+import static org.apache.calcite.linq4j.Nullness.castNonNull;
+
+/**
+ * FLINK modifications are at lines
+ *
+ * <ol>
+ * <li>Added in FLINK-34057, FLINK-34058, FLINK-34312: Lines 452 ~ 469
+ * </ol>
+ */
+class AggConverter implements SqlVisitor<Void> {
Review Comment:
The result of this commit https://github.com/apache/calcite/commit/568ce125e6d81fe0815f1317ccaf4d558688d945
where `SqlToRelConverter` was splitted into 2 classes
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org