This is an automated email from the ASF dual-hosted git repository. mbudiu pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 39652ff0a06e3b5dc6faa1d3b1807a829f39a7cc Author: Oliver Lee <[email protected]> AuthorDate: Thu Oct 31 16:59:43 2024 -0700 [CALCITE-6662] RelJson.rangePointFromJson cannot create Double numerics --- .../apache/calcite/rel/externalize/RelJson.java | 105 ++++++++++++++++++++- .../org/apache/calcite/plan/RelWriterTest.java | 12 +++ .../java/org/apache/calcite/util/RangeSetTest.java | 46 ++++++--- 3 files changed, 146 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java b/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java index e319253c48..41262c065b 100644 --- a/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java +++ b/core/src/main/java/org/apache/calcite/rel/externalize/RelJson.java @@ -519,7 +519,7 @@ public class RelJson { } /** Serializes a {@link Range} that can be deserialized using - * {@link RelJson#rangeFromJson(List)}. */ + * {@link RelJson#rangeFromJson(List, RelDataType)}. */ public <C extends Comparable<C>> List<String> toJson(Range<C> range) { return RangeSets.map(range, RangeToJsonConverter.instance()); } @@ -827,7 +827,7 @@ public class RelJson { final RelDataType type = toType(typeFactory, get(map, "type")); if (literal instanceof Map && ((Map<?, ?>) literal).containsKey("rangeSet")) { - Sarg sarg = sargFromJson((Map) literal); + Sarg sarg = sargFromJson((Map) literal, type); return rexBuilder.makeSearchArgumentLiteral(sarg, type); } if (type.getSqlTypeName() == SqlTypeName.SYMBOL) { @@ -842,7 +842,7 @@ public class RelJson { return rexBuilder.makeNullLiteral(type); } final RelDataType type = toType(typeFactory, get(map, "type")); - Sarg sarg = sargFromJson((Map) sargObject); + Sarg sarg = sargFromJson((Map) sargObject, type); return rexBuilder.makeSearchArgumentLiteral(sarg, type); } if (map.containsKey("dynamicParam")) { @@ -870,6 +870,7 @@ public class RelJson { } } + @Deprecated /** Converts a JSON object to a {@code Sarg}. * * <p>For example, @@ -886,6 +887,16 @@ public class RelJson { RelJson.<C>rangeSetFromJson(rangeSet)); } + public 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(RelEnumTypes.toEnum(nullAs), + RelJson.<C>rangeSetFromJson(rangeSet, type)); + } + + @Deprecated /** Converts a JSON list to a {@link RangeSet}. */ public static <C extends Comparable<C>> RangeSet<C> rangeSetFromJson( List<List<String>> rangeSetsJson) { @@ -898,6 +909,19 @@ public class RelJson { return builder.build(); } + /** Converts a JSON list to a {@link RangeSet} with supplied value typing. */ + public 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(); + } + + @Deprecated /** Creates a {@link Range} from a JSON object. * * <p>The JSON object is as serialized using {@link RelJson#toJson(Range)}, @@ -936,7 +960,46 @@ public class RelJson { } } + /** Creates a {@link Range} from a JSON object. + * + * <p>The JSON object is as serialized using {@link RelJson#toJson(Range)}, + * e.g. {@code ["[", ")", 10, "-"]}. + * + * @see RangeToJsonConverter */ + public 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) { @@ -951,6 +1014,42 @@ public class RelJson { 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)"); + } + } + private void addRexFieldCollationList(List<RexFieldCollation> list, RelInput relInput, @Nullable List<Map<String, Object>> order) { if (order == null) { diff --git a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java index 39ea28a1a1..501b6c0bde 100644 --- a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java +++ b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java @@ -880,6 +880,18 @@ class RelWriterTest { assertThat(result, isLinux(expected)); } + RexNode betweenDoubles = + rexBuilder.makeBetween(b.literal(45), + b.literal(20.0000000000000049), + b.literal(30.0000000000000049)); + consumer.accept(betweenDoubles); + + RexNode betweenDecimal = + rexBuilder.makeBetween(b.literal(45), + b.literal(BigDecimal.valueOf(20.0)), + b.literal(BigDecimal.valueOf(30.0))); + consumer.accept(betweenDecimal); + RexNode between = rexBuilder.makeBetween(b.literal(45), b.literal(20), diff --git a/core/src/test/java/org/apache/calcite/util/RangeSetTest.java b/core/src/test/java/org/apache/calcite/util/RangeSetTest.java index 5624081e67..64af0d70a9 100644 --- a/core/src/test/java/org/apache/calcite/util/RangeSetTest.java +++ b/core/src/test/java/org/apache/calcite/util/RangeSetTest.java @@ -17,6 +17,10 @@ package org.apache.calcite.util; import org.apache.calcite.linq4j.Ord; import org.apache.calcite.rel.externalize.RelJson; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rel.type.RelDataTypeSystem; +import org.apache.calcite.sql.type.BasicSqlType; +import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.test.Matchers; import com.google.common.collect.ImmutableList; @@ -52,10 +56,16 @@ import static java.util.Arrays.asList; class RangeSetTest { /** Tests {@link org.apache.calcite.rel.externalize.RelJson#toJson(Range)} - * and {@link RelJson#rangeFromJson(List)}. */ + * and {@link RelJson#rangeFromJson(List, RelDataType)}. */ @Test void testRangeSetSerializeDeserialize() { RelJson relJson = RelJson.create(); - final Range<BigDecimal> point = Range.singleton(BigDecimal.valueOf(0)); + RelDataType integerType = new BasicSqlType(RelDataTypeSystem.DEFAULT, SqlTypeName.INTEGER); + RelDataType decimalType = new BasicSqlType(RelDataTypeSystem.DEFAULT, SqlTypeName.DECIMAL); + RelDataType doubleType = new BasicSqlType(RelDataTypeSystem.DEFAULT, SqlTypeName.DOUBLE); + + final Range<Integer> integerPoint = Range.singleton(Integer.valueOf(0)); + final Range<BigDecimal> bigDecimalPoint = Range.singleton(BigDecimal.valueOf(0)); + final Range<Double> doublePoint = Range.singleton(Double.valueOf(0)); final Range<BigDecimal> closedRange1 = Range.closed(BigDecimal.valueOf(0), BigDecimal.valueOf(5)); final Range<BigDecimal> closedRange2 = @@ -67,19 +77,27 @@ class RangeSetTest { final Range<BigDecimal> am1 = Range.atMost(BigDecimal.valueOf(3)); // Test serialize/deserialize Range - // Point - assertThat(RelJson.rangeFromJson(relJson.toJson(point)), is(point)); + // Integer Point + // Deserializes as BigDecimal because Calcite uses BigDecimal for exact numerics + assertThat(RelJson.rangeFromJson(relJson.toJson(integerPoint), integerType), + is(bigDecimalPoint)); + // BigDecimal Point + assertThat(RelJson.rangeFromJson(relJson.toJson(bigDecimalPoint), decimalType), + is(bigDecimalPoint)); + // Double Point + assertThat(RelJson.rangeFromJson(relJson.toJson(doublePoint), doubleType), + is(doublePoint)); // Closed Range - assertThat(RelJson.rangeFromJson(relJson.toJson(closedRange1)), + assertThat(RelJson.rangeFromJson(relJson.toJson(closedRange1), decimalType), is(closedRange1)); // Open Range - assertThat(RelJson.rangeFromJson(relJson.toJson(gt1)), is(gt1)); - assertThat(RelJson.rangeFromJson(relJson.toJson(al1)), is(al1)); - assertThat(RelJson.rangeFromJson(relJson.toJson(lt1)), is(lt1)); - assertThat(RelJson.rangeFromJson(relJson.toJson(am1)), is(am1)); + assertThat(RelJson.rangeFromJson(relJson.toJson(gt1), decimalType), is(gt1)); + assertThat(RelJson.rangeFromJson(relJson.toJson(al1), decimalType), is(al1)); + assertThat(RelJson.rangeFromJson(relJson.toJson(lt1), decimalType), is(lt1)); + assertThat(RelJson.rangeFromJson(relJson.toJson(am1), decimalType), is(am1)); // Test closed single RangeSet final RangeSet<BigDecimal> closedRangeSet = ImmutableRangeSet.of(closedRange1); - assertThat(RelJson.rangeSetFromJson(relJson.toJson(closedRangeSet)), + assertThat(RelJson.rangeSetFromJson(relJson.toJson(closedRangeSet), decimalType), is(closedRangeSet)); // Test complex RangeSets final RangeSet<BigDecimal> complexClosedRangeSet1 = @@ -88,21 +106,21 @@ class RangeSetTest { .add(closedRange2) .build(); assertThat( - RelJson.rangeSetFromJson(relJson.toJson(complexClosedRangeSet1)), + RelJson.rangeSetFromJson(relJson.toJson(complexClosedRangeSet1), decimalType), is(complexClosedRangeSet1)); final RangeSet<BigDecimal> complexClosedRangeSet2 = ImmutableRangeSet.<BigDecimal>builder() .add(gt1) .add(am1) .build(); - assertThat(RelJson.rangeSetFromJson(relJson.toJson(complexClosedRangeSet2)), + assertThat(RelJson.rangeSetFromJson(relJson.toJson(complexClosedRangeSet2), decimalType), is(complexClosedRangeSet2)); // Test None and All final RangeSet<BigDecimal> setNone = ImmutableRangeSet.of(); final RangeSet<BigDecimal> setAll = setNone.complement(); - assertThat(RelJson.rangeSetFromJson(relJson.toJson(setNone)), is(setNone)); - assertThat(RelJson.rangeSetFromJson(relJson.toJson(setAll)), is(setAll)); + assertThat(RelJson.rangeSetFromJson(relJson.toJson(setNone), decimalType), is(setNone)); + assertThat(RelJson.rangeSetFromJson(relJson.toJson(setAll), decimalType), is(setAll)); } /** Tests {@link RangeSets#minus(RangeSet, Range)}. */
