You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/11/18 00:27:15 UTC

[GitHub] [pinot] 61yao commented on a diff in pull request #9829: [multistage][hotfix] fix filter type hoisting

61yao commented on code in PR #9829:
URL: https://github.com/apache/pinot/pull/9829#discussion_r1025870467


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

Review Comment:
   If the TODO is not true anymore, we can throw an exception here?



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