You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "solveme (via GitHub)" <gi...@apache.org> on 2026/02/05 07:28:44 UTC

[PR] IGNITE-27648 Sync RelJson related classes with calcite 1.41 [ignite-3]

solveme opened a new pull request, #7532:
URL: https://github.com/apache/ignite-3/pull/7532

   Thank you for submitting the pull request.
   
   To streamline the review process of the patch and ensure better code quality
   we ask both an author and a reviewer to verify the following:
   
   ### The Review Checklist
   - [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:  
   \- There is a single JIRA ticket related to the pull request.  
   \- The web-link to the pull request is attached to the JIRA ticket.  
   \- The JIRA ticket has the Patch Available state.  
   \- The description of the JIRA ticket explains WHAT was made, WHY and HOW.  
   \- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
   - [ ] **Design:** new code conforms with the design principles of the components it is added to.
   - [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
   - [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
   - [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
   
   ### Notes
   - [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)


-- 
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: notifications-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-27648 Sync RelJson related classes with calcite 1.41 [ignite-3]

Posted by "solveme (via GitHub)" <gi...@apache.org>.
solveme commented on code in PR #7532:
URL: https://github.com/apache/ignite-3/pull/7532#discussion_r2798515169


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java:
##########
@@ -923,6 +955,116 @@ RexNode toRex(RelInput relInput, Object o) {
         }
     }
 
+    private static <C extends Comparable<C>> Sarg<C> sargFromJson(Map<String, Object> map, RelDataType type) {
+        final String nullAs = requireNonNull((String) map.get("nullAs"), "nullAs");
+        final List<List<String>> rangeSet =
+                requireNonNull((List<List<String>>) map.get("rangeSet"), "rangeSet");
+        return Sarg.of(ENUM_BY_NAME.get(nullAs),
+                RelJson.<C>rangeSetFromJson(rangeSet, type));
+    }
+
+    /** Converts a JSON list to a {@link RangeSet} with supplied value typing. */
+    private static <C extends Comparable<C>> RangeSet<C> rangeSetFromJson(
+            List<List<String>> rangeSetsJson, RelDataType type) {
+        final ImmutableRangeSet.Builder<C> builder = ImmutableRangeSet.builder();
+        try {
+            rangeSetsJson.forEach(list -> builder.add(rangeFromJson(list, type)));
+        } catch (Exception e) {
+            throw new RuntimeException("Error creating RangeSet from JSON: ", e);
+        }
+        return builder.build();
+    }
+
+    /**
+     * Creates a {@link Range} from a JSON object.
+     *
+     * <p>The JSON object is as serialized using {@link #toJson(Range)},
+     * e.g. {@code ["[", ")", 10, "-"]}.
+     */
+    private static <C extends Comparable<C>> Range<C> rangeFromJson(
+            List<String> list, RelDataType type) {
+        switch (list.get(0)) {
+            case "all":
+                return Range.all();
+            case "atLeast":
+                return Range.atLeast(rangeEndPointFromJson(list.get(1), type));
+            case "atMost":
+                return Range.atMost(rangeEndPointFromJson(list.get(1), type));
+            case "greaterThan":
+                return Range.greaterThan(rangeEndPointFromJson(list.get(1), type));
+            case "lessThan":
+                return Range.lessThan(rangeEndPointFromJson(list.get(1), type));
+            case "singleton":
+                return Range.singleton(rangeEndPointFromJson(list.get(1), type));
+            case "closed":
+                return Range.closed(rangeEndPointFromJson(list.get(1), type),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "closedOpen":
+                return Range.closedOpen(rangeEndPointFromJson(list.get(1)),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "openClosed":
+                return Range.openClosed(rangeEndPointFromJson(list.get(1)),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "open":
+                return Range.open(rangeEndPointFromJson(list.get(1), type),
+                        rangeEndPointFromJson(list.get(2), type));
+            default:
+                throw new AssertionError("unknown range type " + list.get(0));
+        }
+    }
+
+    @SuppressWarnings({"rawtypes", "unchecked"})
+    @Deprecated
+    private static <C extends Comparable<C>> C rangeEndPointFromJson(Object o) {
+        Exception e = null;
+        for (Class clsType : VALUE_CLASSES) {
+            try {
+                return (C) OBJECT_MAPPER.readValue((String) o, clsType);
+            } catch (JsonProcessingException ex) {
+                e = ex;
+            }
+        }
+        throw new RuntimeException(
+                "Error deserializing range endpoint (did not find compatible type): ",
+                e);
+    }
+
+    private static <C extends Comparable<C>> C rangeEndPointFromJson(Object o, RelDataType type) {
+        Exception e;
+        try {
+            Class clsType = determineRangeEndpointValueClass(type);
+            return (C) OBJECT_MAPPER.readValue((String) o, clsType);
+        } catch (JsonProcessingException ex) {
+            e = ex;
+        }
+        throw new RuntimeException(
+                "Error deserializing range endpoint (did not find compatible type): ",
+                e);
+    }
+
+    private static Class determineRangeEndpointValueClass(RelDataType type) {
+        SqlTypeName typeName = RexLiteral.strictTypeName(type);
+        switch (typeName) {
+            case DECIMAL:
+                return BigDecimal.class;
+            case DOUBLE:
+                return Double.class;
+            case CHAR:
+                return NlsString.class;
+            case BOOLEAN:
+                return Boolean.class;
+            case TIMESTAMP:
+                return TimestampString.class;
+            case DATE:
+                return DateString.class;
+            case TIME:
+                return TimeString.class;
+            default:
+                throw new RuntimeException(
+                        "Error deserializing range endpoint (did not find compatible type)");
+        }

Review Comment:
   This piece of code is copied as is from Calcite. If type set is not sufficient it should be evaluated and  extended in separate task



-- 
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: notifications-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-27648 Sync RelJson related classes with calcite 1.41 [ignite-3]

Posted by "xtern (via GitHub)" <gi...@apache.org>.
xtern commented on code in PR #7532:
URL: https://github.com/apache/ignite-3/pull/7532#discussion_r2787896130


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java:
##########
@@ -904,6 +918,24 @@ RexNode toRex(RelInput relInput, Object o) {
                 return rexBuilder.makeLiteral(literal, type, true);
             }
 
+            if (map.containsKey("sargLiteral")) {
+                Object sargObject = map.get("sargLiteral");
+                if (sargObject == null) {
+                    final RelDataType type = toType(typeFactory, map.get("type"));
+                    return rexBuilder.makeNullLiteral(type);
+                }
+                final RelDataType type = toType(typeFactory, map.get("type"));
+                Sarg sarg = sargFromJson((Map) sargObject, type);
+                return rexBuilder.makeSearchArgumentLiteral(sarg, type);
+            }
+
+            if (map.containsKey("dynamicParam")) {

Review Comment:
   looks like this condition is unreachable until we add similar serialization for dyn params.
   E.g. you added deserialization of dynamic parameter by the `dynamicParam` key, but we don't use this key when serializing dynamic parameters.
   So my suggestion  is to remove this condition.



-- 
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: notifications-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-27648 Sync RelJson related classes with calcite 1.41 [ignite-3]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #7532:
URL: https://github.com/apache/ignite-3/pull/7532#discussion_r2787949533


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java:
##########
@@ -814,9 +810,21 @@ RexNode toRex(RelInput relInput, Object o) {
                         upperBound = null;
                         physical = false;
                     }
+
+                    final RexWindowExclusion exclude;
+                    if (window.get("exclude") != null) {
+                        exclude = toRexWindowExclusion((Map) window.get("exclude"));
+                    } else {
+                        exclude = RexWindowExclusion.EXCLUDE_NO_OTHER;
+                    }
+
                     boolean distinct = (Boolean) map.get("distinct");
                     return rexBuilder.makeOver(type, operator, rexOperands, partitionKeys,
-                            ImmutableList.copyOf(orderKeys), lowerBound, upperBound, physical,
+                            ImmutableList.copyOf(orderKeys),
+                            requireNonNull(lowerBound, "lowerBound"),
+                            requireNonNull(upperBound, "upperBound"),
+                            requireNonNull(exclude, "exclude"),
+                            physical,

Review Comment:
   `toRex` now calls `requireNonNull(lowerBound/upperBound)` when deserializing a windowed aggregate. However `toJson(RexWindow)` intentionally omits bounds when there is no ROWS/RANGE clause, which leaves both bounds null and will cause deserialization to fail for such windows. Consider either (a) serializing explicit default bounds/exclusion in `toJson(RexWindow)`, or (b) in the "No ROWS or RANGE clause" branch, set Calcite-equivalent default bounds (instead of null) before calling `makeOver`.



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java:
##########
@@ -923,6 +955,116 @@ RexNode toRex(RelInput relInput, Object o) {
         }
     }
 
+    private static <C extends Comparable<C>> Sarg<C> sargFromJson(Map<String, Object> map, RelDataType type) {
+        final String nullAs = requireNonNull((String) map.get("nullAs"), "nullAs");
+        final List<List<String>> rangeSet =
+                requireNonNull((List<List<String>>) map.get("rangeSet"), "rangeSet");
+        return Sarg.of(ENUM_BY_NAME.get(nullAs),
+                RelJson.<C>rangeSetFromJson(rangeSet, type));
+    }
+
+    /** Converts a JSON list to a {@link RangeSet} with supplied value typing. */
+    private static <C extends Comparable<C>> RangeSet<C> rangeSetFromJson(
+            List<List<String>> rangeSetsJson, RelDataType type) {
+        final ImmutableRangeSet.Builder<C> builder = ImmutableRangeSet.builder();
+        try {
+            rangeSetsJson.forEach(list -> builder.add(rangeFromJson(list, type)));
+        } catch (Exception e) {
+            throw new RuntimeException("Error creating RangeSet from JSON: ", e);
+        }
+        return builder.build();
+    }
+
+    /**
+     * Creates a {@link Range} from a JSON object.
+     *
+     * <p>The JSON object is as serialized using {@link #toJson(Range)},
+     * e.g. {@code ["[", ")", 10, "-"]}.
+     */
+    private static <C extends Comparable<C>> Range<C> rangeFromJson(
+            List<String> list, RelDataType type) {
+        switch (list.get(0)) {
+            case "all":
+                return Range.all();
+            case "atLeast":
+                return Range.atLeast(rangeEndPointFromJson(list.get(1), type));
+            case "atMost":
+                return Range.atMost(rangeEndPointFromJson(list.get(1), type));
+            case "greaterThan":
+                return Range.greaterThan(rangeEndPointFromJson(list.get(1), type));
+            case "lessThan":
+                return Range.lessThan(rangeEndPointFromJson(list.get(1), type));
+            case "singleton":
+                return Range.singleton(rangeEndPointFromJson(list.get(1), type));
+            case "closed":
+                return Range.closed(rangeEndPointFromJson(list.get(1), type),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "closedOpen":
+                return Range.closedOpen(rangeEndPointFromJson(list.get(1)),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "openClosed":
+                return Range.openClosed(rangeEndPointFromJson(list.get(1)),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "open":
+                return Range.open(rangeEndPointFromJson(list.get(1), type),
+                        rangeEndPointFromJson(list.get(2), type));
+            default:
+                throw new AssertionError("unknown range type " + list.get(0));
+        }
+    }
+
+    @SuppressWarnings({"rawtypes", "unchecked"})
+    @Deprecated
+    private static <C extends Comparable<C>> C rangeEndPointFromJson(Object o) {
+        Exception e = null;
+        for (Class clsType : VALUE_CLASSES) {
+            try {
+                return (C) OBJECT_MAPPER.readValue((String) o, clsType);
+            } catch (JsonProcessingException ex) {
+                e = ex;
+            }
+        }
+        throw new RuntimeException(
+                "Error deserializing range endpoint (did not find compatible type): ",
+                e);
+    }
+
+    private static <C extends Comparable<C>> C rangeEndPointFromJson(Object o, RelDataType type) {
+        Exception e;
+        try {
+            Class clsType = determineRangeEndpointValueClass(type);
+            return (C) OBJECT_MAPPER.readValue((String) o, clsType);
+        } catch (JsonProcessingException ex) {
+            e = ex;
+        }
+        throw new RuntimeException(
+                "Error deserializing range endpoint (did not find compatible type): ",
+                e);
+    }
+
+    private static Class determineRangeEndpointValueClass(RelDataType type) {
+        SqlTypeName typeName = RexLiteral.strictTypeName(type);
+        switch (typeName) {
+            case DECIMAL:
+                return BigDecimal.class;
+            case DOUBLE:
+                return Double.class;
+            case CHAR:
+                return NlsString.class;
+            case BOOLEAN:
+                return Boolean.class;
+            case TIMESTAMP:
+                return TimestampString.class;
+            case DATE:
+                return DateString.class;
+            case TIME:
+                return TimeString.class;
+            default:
+                throw new RuntimeException(
+                        "Error deserializing range endpoint (did not find compatible type)");
+        }

Review Comment:
   `determineRangeEndpointValueClass` only supports a small subset of `SqlTypeName` values (e.g., DECIMAL/DOUBLE/CHAR/BOOLEAN/TIMESTAMP/DATE/TIME). Sarg ranges can be produced for other common types (e.g., INTEGER/BIGINT/SMALLINT/TINYINT/REAL/FLOAT/VARCHAR), and this method will throw at runtime for them. Expand the mapping to cover the relevant Calcite literal value representations (e.g., exact numerics -> BigDecimal, approximate -> Double, character types -> NlsString, etc.).



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java:
##########
@@ -923,6 +955,116 @@ RexNode toRex(RelInput relInput, Object o) {
         }
     }
 
+    private static <C extends Comparable<C>> Sarg<C> sargFromJson(Map<String, Object> map, RelDataType type) {
+        final String nullAs = requireNonNull((String) map.get("nullAs"), "nullAs");
+        final List<List<String>> rangeSet =
+                requireNonNull((List<List<String>>) map.get("rangeSet"), "rangeSet");
+        return Sarg.of(ENUM_BY_NAME.get(nullAs),
+                RelJson.<C>rangeSetFromJson(rangeSet, type));
+    }

Review Comment:
   Sarg serialization/deserialization is newly introduced here (`toJson(Sarg)`, `sargFromJson`, `rangeSetFromJson`, `rangeFromJson`). There are existing planner tests that exercise RelJson round-tripping; adding focused tests that round-trip RexNodes containing Sarg/RangeSet (including different type families and nullAs values) would help prevent regressions, especially around endpoint typing and range shapes.



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java:
##########
@@ -923,6 +955,116 @@ RexNode toRex(RelInput relInput, Object o) {
         }
     }
 
+    private static <C extends Comparable<C>> Sarg<C> sargFromJson(Map<String, Object> map, RelDataType type) {
+        final String nullAs = requireNonNull((String) map.get("nullAs"), "nullAs");
+        final List<List<String>> rangeSet =
+                requireNonNull((List<List<String>>) map.get("rangeSet"), "rangeSet");
+        return Sarg.of(ENUM_BY_NAME.get(nullAs),
+                RelJson.<C>rangeSetFromJson(rangeSet, type));
+    }
+
+    /** Converts a JSON list to a {@link RangeSet} with supplied value typing. */
+    private static <C extends Comparable<C>> RangeSet<C> rangeSetFromJson(
+            List<List<String>> rangeSetsJson, RelDataType type) {
+        final ImmutableRangeSet.Builder<C> builder = ImmutableRangeSet.builder();
+        try {
+            rangeSetsJson.forEach(list -> builder.add(rangeFromJson(list, type)));
+        } catch (Exception e) {
+            throw new RuntimeException("Error creating RangeSet from JSON: ", e);
+        }
+        return builder.build();
+    }
+
+    /**
+     * Creates a {@link Range} from a JSON object.
+     *
+     * <p>The JSON object is as serialized using {@link #toJson(Range)},
+     * e.g. {@code ["[", ")", 10, "-"]}.
+     */
+    private static <C extends Comparable<C>> Range<C> rangeFromJson(
+            List<String> list, RelDataType type) {
+        switch (list.get(0)) {
+            case "all":
+                return Range.all();
+            case "atLeast":
+                return Range.atLeast(rangeEndPointFromJson(list.get(1), type));
+            case "atMost":
+                return Range.atMost(rangeEndPointFromJson(list.get(1), type));
+            case "greaterThan":
+                return Range.greaterThan(rangeEndPointFromJson(list.get(1), type));
+            case "lessThan":
+                return Range.lessThan(rangeEndPointFromJson(list.get(1), type));
+            case "singleton":
+                return Range.singleton(rangeEndPointFromJson(list.get(1), type));
+            case "closed":
+                return Range.closed(rangeEndPointFromJson(list.get(1), type),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "closedOpen":
+                return Range.closedOpen(rangeEndPointFromJson(list.get(1)),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "openClosed":
+                return Range.openClosed(rangeEndPointFromJson(list.get(1)),

Review Comment:
   In `rangeFromJson`, the `closedOpen` branch uses the deprecated `rangeEndPointFromJson(Object)` overload for the lower endpoint (`rangeEndPointFromJson(list.get(1))`), while other branches use the typed overload that considers `RelDataType`. This looks like an accidental mismatch and can lead to inconsistent endpoint typing; use the `(Object, RelDataType)` overload for both endpoints.
   ```suggestion
                   return Range.closedOpen(rangeEndPointFromJson(list.get(1), type),
                           rangeEndPointFromJson(list.get(2), type));
               case "openClosed":
                   return Range.openClosed(rangeEndPointFromJson(list.get(1), type),
   ```



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java:
##########
@@ -923,6 +955,116 @@ RexNode toRex(RelInput relInput, Object o) {
         }
     }
 
+    private static <C extends Comparable<C>> Sarg<C> sargFromJson(Map<String, Object> map, RelDataType type) {
+        final String nullAs = requireNonNull((String) map.get("nullAs"), "nullAs");
+        final List<List<String>> rangeSet =
+                requireNonNull((List<List<String>>) map.get("rangeSet"), "rangeSet");
+        return Sarg.of(ENUM_BY_NAME.get(nullAs),
+                RelJson.<C>rangeSetFromJson(rangeSet, type));
+    }
+
+    /** Converts a JSON list to a {@link RangeSet} with supplied value typing. */
+    private static <C extends Comparable<C>> RangeSet<C> rangeSetFromJson(
+            List<List<String>> rangeSetsJson, RelDataType type) {
+        final ImmutableRangeSet.Builder<C> builder = ImmutableRangeSet.builder();
+        try {
+            rangeSetsJson.forEach(list -> builder.add(rangeFromJson(list, type)));
+        } catch (Exception e) {
+            throw new RuntimeException("Error creating RangeSet from JSON: ", e);
+        }
+        return builder.build();
+    }
+
+    /**
+     * Creates a {@link Range} from a JSON object.
+     *
+     * <p>The JSON object is as serialized using {@link #toJson(Range)},
+     * e.g. {@code ["[", ")", 10, "-"]}.
+     */
+    private static <C extends Comparable<C>> Range<C> rangeFromJson(
+            List<String> list, RelDataType type) {
+        switch (list.get(0)) {
+            case "all":
+                return Range.all();
+            case "atLeast":
+                return Range.atLeast(rangeEndPointFromJson(list.get(1), type));
+            case "atMost":
+                return Range.atMost(rangeEndPointFromJson(list.get(1), type));
+            case "greaterThan":
+                return Range.greaterThan(rangeEndPointFromJson(list.get(1), type));
+            case "lessThan":
+                return Range.lessThan(rangeEndPointFromJson(list.get(1), type));
+            case "singleton":
+                return Range.singleton(rangeEndPointFromJson(list.get(1), type));
+            case "closed":
+                return Range.closed(rangeEndPointFromJson(list.get(1), type),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "closedOpen":
+                return Range.closedOpen(rangeEndPointFromJson(list.get(1)),
+                        rangeEndPointFromJson(list.get(2), type));
+            case "openClosed":
+                return Range.openClosed(rangeEndPointFromJson(list.get(1)),

Review Comment:
   In `rangeFromJson`, the `openClosed` branch also uses the deprecated `rangeEndPointFromJson(Object)` overload for the lower endpoint. To keep Sarg/RangeSet deserialization type-safe and consistent, use the `(Object, RelDataType)` overload for both endpoints.
   ```suggestion
                   return Range.closedOpen(rangeEndPointFromJson(list.get(1), type),
                           rangeEndPointFromJson(list.get(2), type));
               case "openClosed":
                   return Range.openClosed(rangeEndPointFromJson(list.get(1), type),
   ```



-- 
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: notifications-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-27648 Sync RelJson related classes with calcite 1.41 [ignite-3]

Posted by "solveme (via GitHub)" <gi...@apache.org>.
solveme commented on code in PR #7532:
URL: https://github.com/apache/ignite-3/pull/7532#discussion_r2804107347


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java:
##########
@@ -904,6 +918,24 @@ RexNode toRex(RelInput relInput, Object o) {
                 return rexBuilder.makeLiteral(literal, type, true);
             }
 
+            if (map.containsKey("sargLiteral")) {
+                Object sargObject = map.get("sargLiteral");
+                if (sargObject == null) {
+                    final RelDataType type = toType(typeFactory, map.get("type"));
+                    return rexBuilder.makeNullLiteral(type);
+                }
+                final RelDataType type = toType(typeFactory, map.get("type"));
+                Sarg sarg = sargFromJson((Map) sargObject, type);
+                return rexBuilder.makeSearchArgumentLiteral(sarg, type);
+            }
+
+            if (map.containsKey("dynamicParam")) {

Review Comment:
   I've double checked this as well, and you're right. Thanks for pointing this out.



-- 
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: notifications-unsubscribe@ignite.apache.org

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


Re: [PR] IGNITE-27648 Sync RelJson related classes with calcite 1.41 [ignite-3]

Posted by "solveme (via GitHub)" <gi...@apache.org>.
solveme commented on code in PR #7532:
URL: https://github.com/apache/ignite-3/pull/7532#discussion_r2804107347


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/externalize/RelJson.java:
##########
@@ -904,6 +918,24 @@ RexNode toRex(RelInput relInput, Object o) {
                 return rexBuilder.makeLiteral(literal, type, true);
             }
 
+            if (map.containsKey("sargLiteral")) {
+                Object sargObject = map.get("sargLiteral");
+                if (sargObject == null) {
+                    final RelDataType type = toType(typeFactory, map.get("type"));
+                    return rexBuilder.makeNullLiteral(type);
+                }
+                final RelDataType type = toType(typeFactory, map.get("type"));
+                Sarg sarg = sargFromJson((Map) sargObject, type);
+                return rexBuilder.makeSearchArgumentLiteral(sarg, type);
+            }
+
+            if (map.containsKey("dynamicParam")) {

Review Comment:
   I've double checked this as well, and you're right. I removed this piece of code. Thanks for pointing this out.



-- 
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: notifications-unsubscribe@ignite.apache.org

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