xuyangzhong commented on code in PR #24183:
URL: https://github.com/apache/flink/pull/24183#discussion_r1468816825
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/FunctionITCase.java:
##########
@@ -1527,11 +1605,37 @@ public String eval(Integer arg1, Integer arg2) {
@ArgumentHint(name = "in1", type =
@DataTypeHint("string")),
@ArgumentHint(name = "in2", type = @DataTypeHint("string"))
})
+ public String eval(String arg1, String arg2) {
+ return (arg1 + ":" + arg2);
Review Comment:
Nit: add a `space` after `:`
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/inference/OperatorBindingCallContext.java:
##########
@@ -67,27 +70,50 @@ public OperatorBindingCallContext(
new AbstractList<DataType>() {
@Override
public DataType get(int pos) {
+ RelDataType relDataType;
if (binding instanceof SqlCallBinding) {
SqlCallBinding sqlCallBinding = (SqlCallBinding)
binding;
- List<SqlNode> operands = sqlCallBinding.operands();
- final RelDataType relDataType =
- sqlCallBinding
- .getValidator()
- .deriveType(
- sqlCallBinding.getScope(),
operands.get(pos));
- final LogicalType logicalType =
-
FlinkTypeFactory.toLogicalType(relDataType);
- return
TypeConversions.fromLogicalToDataType(logicalType);
+ SqlNode sqlNode =
sqlCallBinding.operands().get(pos);
+ if (sqlNode.getKind() == SqlKind.DEFAULT) {
+ RelDataType[] defaultTypes =
+ FlinkRexUtil.extractDefaultTypes(
+ sqlCallBinding.getOperator(),
+
sqlCallBinding.getTypeFactory());
+ relDataType = defaultTypes[pos];
+ } else {
+ relDataType =
+ sqlCallBinding
+ .getValidator()
+
.deriveType(sqlCallBinding.getScope(), sqlNode);
+ }
+ } else if (binding instanceof RexCallBinding) {
+ RexCallBinding rexCallBinding = (RexCallBinding)
binding;
+ RexNode rexNode =
rexCallBinding.operands().get(pos);
Review Comment:
ditto
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/calcite/FlinkConvertletTable.java:
##########
@@ -69,6 +72,10 @@ public SqlRexConvertlet get(SqlCall call) {
return this::convertSetSemanticsWindowTableFunction;
}
+ if (isContainsDefaultNode(call)) {
Review Comment:
Nit: just a little curious, can we correct the type of `default` and replace
the RexCall, and then it is not necessary to do the same logic in
`CallBindingCallContext` and `OperatorBindingCallContext` again and again...
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/calcite/FlinkConvertletTable.java:
##########
@@ -202,4 +215,24 @@ private static int parseFieldIdx(RexNode e) {
// should not happen
throw new TableException("Unsupported partition key with type: " +
e.getKind());
}
+
+ private RexNode convertSqlCallWithDefaultNode(SqlRexContext cx, final
SqlCall call) {
Review Comment:
Nit: could you add some comments here?
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/operations/converters/SqlProcedureCallConverter.java:
##########
@@ -126,19 +129,29 @@ public Operation convertSqlNode(SqlNode sqlNode,
ConvertContext context) {
typeInferResult.getOutputDataType());
}
- private List<RexNode> reduceOperands(List<SqlNode> sqlNodes,
ConvertContext context) {
+ private List<RexNode> reduceOperands(SqlCallBinding sqlCallBinding,
ConvertContext context) {
// we don't really care about the input row type while converting to
RexNode
// since call procedure shouldn't refer any inputs.
// so, construct an empty row for it.
RelDataType inputRowType =
- LogicalRelDataTypeConverter.toRelDataType(
+ toRelDataType(
DataTypes.ROW().getLogicalType(),
context.getSqlValidator().getTypeFactory());
List<RexNode> rexNodes = new ArrayList<>();
- for (SqlNode sqlNode : sqlNodes) {
- RexNode rexNode = context.toRexNode(sqlNode, inputRowType, null);
+ List<SqlNode> operands = sqlCallBinding.operands();
+ RelDataType[] defaultTypes =
+ FlinkRexUtil.extractDefaultTypes(
+ sqlCallBinding.getCall(), context.getSqlValidator());
+ for (int i = 0; i < operands.size(); i++) {
+ SqlNode sqlNode = operands.get(i);
+ RexNode rexNode = context.toRexNode(operands.get(i), inputRowType,
null);
+ if (sqlNode.getKind() == SqlKind.DEFAULT) {
Review Comment:
ditto
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/runtime/stream/sql/FunctionITCase.java:
##########
@@ -1527,11 +1605,37 @@ public String eval(Integer arg1, Integer arg2) {
@ArgumentHint(name = "in1", type =
@DataTypeHint("string")),
@ArgumentHint(name = "in2", type = @DataTypeHint("string"))
})
+ public String eval(String arg1, String arg2) {
+ return (arg1 + ":" + arg2);
+ }
+ }
+
+ /** Function with optional arguments. */
+ public static class NamedArgumentsScalarFunctionWithOptionalArguments
extends ScalarFunction {
+ @FunctionHint(
+ output = @DataTypeHint("STRING"),
+ argument = {
+ @ArgumentHint(name = "in1", type =
@DataTypeHint("STRING"), isOptional = true),
+ @ArgumentHint(name = "in2", type =
@DataTypeHint("STRING"), isOptional = true)
+ })
public String eval(String arg1, String arg2) {
return (arg1 + ": " + arg2);
}
}
+ /** Function with optional arguments. */
+ public static class NamedArgumentsScalarFunctionWithOptionalArguments2
extends ScalarFunction {
Review Comment:
It seems no tests use this class. Remove it?
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/inference/OperatorBindingCallContext.java:
##########
@@ -67,27 +70,50 @@ public OperatorBindingCallContext(
new AbstractList<DataType>() {
@Override
public DataType get(int pos) {
+ RelDataType relDataType;
if (binding instanceof SqlCallBinding) {
SqlCallBinding sqlCallBinding = (SqlCallBinding)
binding;
- List<SqlNode> operands = sqlCallBinding.operands();
- final RelDataType relDataType =
- sqlCallBinding
- .getValidator()
- .deriveType(
- sqlCallBinding.getScope(),
operands.get(pos));
- final LogicalType logicalType =
-
FlinkTypeFactory.toLogicalType(relDataType);
- return
TypeConversions.fromLogicalToDataType(logicalType);
+ SqlNode sqlNode =
sqlCallBinding.operands().get(pos);
+ if (sqlNode.getKind() == SqlKind.DEFAULT) {
Review Comment:
I think it would be better to extract this into a utility method, otherwise
it seems a bit too redundant.
Just like use SqlCallBinding/RexCallBinding and index as inputs.
--
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-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
