slinkydeveloper commented on a change in pull request #17256:
URL: https://github.com/apache/flink/pull/17256#discussion_r707995221



##########
File path: 
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/Expressions.java
##########
@@ -535,6 +535,18 @@ public static ApiExpression ifThenElse(Object condition, 
Object ifTrue, Object i
         return apiCall(BuiltInFunctionDefinitions.IF, condition, ifTrue, 
ifFalse);
     }
 
+    /**
+     * Coalesce specifies a series of expressions, and returns the first 
expression whose value is
+     * not null. If all the expressions evaluate as null, coalesce returns a 
null value.
+     *
+     * <p>e.g. coalesce($("f0"), "-") leads to "-" if f0 is null

Review comment:
       > Having a couple examples would be good, especially ones that don't 
rely on external fields (like f0) but would work out of the box.
   
   I think it's still worth showing with external fields, because showing the 
coalesce with constants doesn't really show the point IMO. Look at the rephrase 
now, i have added two examples to make my point (hopefully) clear enough.

##########
File path: 
flink-table/flink-table-common/src/main/java/org/apache/flink/table/types/inference/strategies/CoalesceTypeStrategy.java
##########
@@ -0,0 +1,57 @@
+/*
+ * 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.types.inference.strategies;
+
+import org.apache.flink.annotation.Internal;
+import org.apache.flink.table.types.DataType;
+import org.apache.flink.table.types.inference.CallContext;
+import org.apache.flink.table.types.inference.TypeStrategy;
+import org.apache.flink.table.types.logical.LogicalType;
+import org.apache.flink.table.types.logical.utils.LogicalTypeMerging;
+import org.apache.flink.table.types.utils.TypeConversions;
+
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+/**
+ * A type strategy for COALESCE return type. The return type of COALESCE is 
the least restrictive
+ * type of its arguments, and it's nullable only if all its input args are 
nullable.
+ */
+@Internal
+public final class CoalesceTypeStrategy implements TypeStrategy {
+
+    public CoalesceTypeStrategy() {}
+
+    @Override
+    public Optional<DataType> inferType(CallContext callContext) {

Review comment:
       I've reused `NullableIfArgsTypeStrategy`, removing the code here. Now 
`TypeStrategies` has both `nullableIfArgs` and `nullableIfAllArgs`. Perhaps 
should i rename `nullableIfArgs` in `nullableIfAnyArgs`? Should I do it in this 
PR (separate commit) or another one?




-- 
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


Reply via email to