xtern commented on code in PR #2671:
URL: https://github.com/apache/ignite-3/pull/2671#discussion_r1358297466


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/ExpressionFactoryImpl.java:
##########
@@ -765,36 +773,64 @@ private class RangeConditionImpl implements 
RangeCondition<RowT> {
         /** Upper row. */
         private @Nullable RowT upperRow;
 
-        /** Cached skip range flag. */
-        private Boolean skip;
+        /** Lower bound row factory. */
+        private final RowFactory<RowT> factoryLower;
 
-        /** Row factory. */
-        private final RowFactory<RowT> factory;
+        /** Upper bound row factory. */
+        private final RowFactory<RowT> factoryUpper;
+
+        /** Cached skip range flag. */
+        private @Nullable Boolean skip;
 
         private RangeConditionImpl(
-                SingleScalar lowerBound,
-                SingleScalar upperBound,
+                List<RexNode> lower,
+                List<RexNode> upper,
+                RelDataType rowType,
                 boolean lowerInclude,
                 boolean upperInclude,
-                RowFactory<RowT> factory
+                RowFactory<RowT> factory,
+                boolean containNulls
         ) {
-            this.lowerBound = lowerBound;
-            this.upperBound = upperBound;
+            RelDataType rowTypeLower;
+            RelDataType rowTypeUpper;
+
+            if (containNulls) {
+                Entry<List<RexNode>, RelDataType> res = 
shrinkBounds(ctx.getTypeFactory(), lower, rowType);
+                lower = res.getKey();
+                rowTypeLower = res.getValue();
+
+                res = shrinkBounds(ctx.getTypeFactory(), upper, rowType);
+                upper = res.getKey();
+                rowTypeUpper = res.getValue();
+            } else {
+                rowTypeLower = rowType;
+                rowTypeUpper = rowType;
+            }
+
+            this.lowerBound = scalar(lower, rowTypeLower);
+            this.upperBound = scalar(upper, rowTypeUpper);
             this.lowerInclude = lowerInclude;
             this.upperInclude = upperInclude;
-            this.factory = factory;
+
+            factoryLower = containNulls
+                    ? 
ctx.rowHandler().factory(TypeUtils.rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rowTypeLower)))
+                    : factory;
+
+            factoryUpper = containNulls
+                    ? 
ctx.rowHandler().factory(TypeUtils.rowSchemaFromRelTypes(RelOptUtil.getFieldTypeList(rowTypeUpper)))
+                    : factory;
         }
 
         /** {@inheritDoc} */
         @Override
-        public RowT lower() {
-            return lowerRow != null ? lowerRow : (lowerRow = 
getRow(lowerBound));
+        public @Nullable RowT lower() {
+            return lowerRow != null ? lowerRow : getRow(lowerBound, true);

Review Comment:
   Is `lowerRow/upperRow` is always `null` now?
   Also, instead of passing a boolean flag, might it be better to pass the 
target row factory?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to