walterddr commented on code in PR #9829:
URL: https://github.com/apache/pinot/pull/9829#discussion_r1025904662


##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/operands/FilterOperand.java:
##########
@@ -230,14 +230,41 @@ public Predicate(List<RexExpression> functionOperands, 
DataSchema dataSchema) {
           "Expected 2 function ops for Predicate but got:" + 
functionOperands.size());
       _lhs = TransformOperand.toTransformOperand(functionOperands.get(0), 
dataSchema);
       _rhs = TransformOperand.toTransformOperand(functionOperands.get(1), 
dataSchema);
-      if (_lhs._resultType != null && _lhs._resultType != 
DataSchema.ColumnDataType.OBJECT) {
-        _resultType = _lhs._resultType;
-      } else if (_rhs._resultType != null && _rhs._resultType != 
DataSchema.ColumnDataType.OBJECT) {
-        _resultType = _rhs._resultType;
+      _resultType = resolveResultType(_lhs._resultType, _rhs._resultType);
+    }
+
+    private static DataSchema.ColumnDataType 
resolveResultType(DataSchema.ColumnDataType lhsType,
+        DataSchema.ColumnDataType rhsType) {
+      if (lhsType == null) {
+        return rhsType;
+      } else if (rhsType == null) {
+        return lhsType;
       } else {
-        // TODO: we should correctly throw exception here. Currently exception 
thrown during constructor is not
-        // piped back to query dispatcher, thus we set it to null and 
deliberately make the processing throw exception.
-        _resultType = null;
+        if (isSuperType(lhsType, rhsType)) {
+          return lhsType;
+        } else if (isSuperType(rhsType, lhsType)) {
+          return rhsType;
+        } else {
+          return null;
+        }
+      }
+    }
+
+    private static boolean isSuperType(DataSchema.ColumnDataType superType, 
DataSchema.ColumnDataType subType) {

Review Comment:
   - actually good idea using `getNullPlaceholder()` since it already has the 
java class.
   - Object handling should be correct but I might be wrong as I haven't add a 
unit test, yet
     - Object is not always Comparable and thus we need to rely on the other 
side to cast it (hopefully) to the appropriate comparable object.



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

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

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


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

Reply via email to