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 04c65469b2dcc5f30e068b5fd8e5b7ba3bc6a62e Author: Oliver Lee <[email protected]> AuthorDate: Fri Nov 1 14:48:30 2024 -0700 Add in new methods that take in RelDataType to make typing explicit; deprecate existing functions; update tests --- .../apache/calcite/rel/externalize/RelJson.java | 127 +++++++++++++++++---- .../org/apache/calcite/plan/RelWriterTest.java | 6 + .../java/org/apache/calcite/util/RangeSetTest.java | 46 +++++--- 3 files changed, 143 insertions(+), 36 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 6cd706c303..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 @@ -16,8 +16,6 @@ */ package org.apache.calcite.rel.externalize; -import javax.naming.directory.InvalidAttributesException; - import org.apache.calcite.avatica.AvaticaUtils; import org.apache.calcite.avatica.util.ByteString; import org.apache.calcite.avatica.util.TimeUnit; @@ -116,12 +114,9 @@ public class RelJson { .configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true); private static final List<Class> VALUE_CLASSES = - ImmutableList.of(NlsString.class, ByteString.class, + ImmutableList.of(NlsString.class, BigDecimal.class, ByteString.class, Boolean.class, TimestampString.class, DateString.class, TimeString.class); - private static final List<Class> NUMERIC_VALUE_CLASSES = - ImmutableList.of(Double.class, BigDecimal.class); - private final Map<String, Constructor> constructorMap = new HashMap<>(); private final @Nullable JsonBuilder jsonBuilder; private final InputTranslator inputTranslator; @@ -524,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()); } @@ -832,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) { @@ -847,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")) { @@ -875,6 +870,7 @@ public class RelJson { } } + @Deprecated /** Converts a JSON object to a {@code Sarg}. * * <p>For example, @@ -891,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) { @@ -903,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)}, @@ -941,24 +960,52 @@ 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 : NUMERIC_VALUE_CLASSES) { - try { - if (OBJECT_MAPPER.readValue((String) o, clsType).toString().equals(o.toString())){ - return (C) OBJECT_MAPPER.readValue((String) o, clsType); - } else { - throw new InvalidAttributesException("Incorrect conversion between approximate and exact numeric "); - } - } catch (JsonProcessingException|InvalidAttributesException ex) { - e = ex; - } - } for (Class clsType : VALUE_CLASSES) { try { - return (C) OBJECT_MAPPER.readValue((String) o, clsType); - } catch (JsonProcessingException ex) { + return (C) OBJECT_MAPPER.readValue((String) o, clsType); + } catch (JsonProcessingException ex) { e = ex; } } @@ -967,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 1a1b2be4d6..501b6c0bde 100644 --- a/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java +++ b/core/src/test/java/org/apache/calcite/plan/RelWriterTest.java @@ -886,6 +886,12 @@ class RelWriterTest { 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)}. */
