You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by mb...@apache.org on 2024/11/04 07:50:13 UTC
(calcite) 01/01: [CALCITE-6662] RelJson.rangePointFromJson cannot create Double numerics
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 <ol...@google.com>
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)}. */