[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #9702: [multistage][bugfix] fix case-when issue

[GitBox <gi...@apache.org>]

Jackie-Jiang commented on code in PR #9702:
URL: https://github.com/apache/pinot/pull/9702#discussion_r1012122737


##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ObjectFunctions.java:
##########
@@ -92,13 +94,40 @@ private static Object coalesceVar(Object... objects) {
     return null;
   }
 
-  @Nullable
-  private static Object coalesce(Object... objects) {
-    for (Object o : objects) {
-      if (o != null) {
-        return o;
+  @ScalarFunction
+  public static Object caseWhen(boolean c1, Object o1, Object oe) {

Review Comment:
   Is the function name `case` or `caseWhen`?



##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java:
##########
@@ -84,17 +84,40 @@ public void init(List<TransformFunction> arguments, Map<String, DataSource> data
     }
     int numWhenStatements = arguments.size() / 2;
     _whenStatements = new ArrayList<>(numWhenStatements);
+    _elseThenStatements = new ArrayList<>(numWhenStatements + 1);
+    constructStatementList(arguments);
+    _selections = new boolean[_elseThenStatements.size()];
+    Collections.reverse(_elseThenStatements);
+    Collections.reverse(_whenStatements);
+    _resultMetadata = calculateResultMetadata();
+  }
+
+  private void constructStatementList(List<TransformFunction> arguments) {
+    int numWhenStatements = arguments.size() / 2;
+    boolean hasLegacyFormat = false;
+    for (int i = 0; i < numWhenStatements; i++) {
+      if (arguments.get(i * 2).getResultMetadata().getDataType() != DataType.BOOLEAN) {
+        hasLegacyFormat = true;
+        break;

Review Comment:
   (MAJOR) This can break when the first loop didn't break but the second loop break because some wrong arguments already added to the list.
   Also, we should check in favor of the new order. We can check if the first half of the arguments are all boolean, and if the odd index arguments are all boolean. If both are all booleans or both are not all booleans or the first half are all booleans, use the new order; use the legacy one otherwise.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org