PHOENIX-4071 PDataType.compareTo(Object,Object,PDataType) does not handle null values
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/5c16a29b Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/5c16a29b Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/5c16a29b Branch: refs/heads/4.x-HBase-1.1 Commit: 5c16a29b6e541146475aaf7bfd2cc4474c015822 Parents: 8bc5832 Author: James Taylor <[email protected]> Authored: Mon Aug 7 11:52:42 2017 -0700 Committer: James Taylor <[email protected]> Committed: Mon Aug 7 23:05:00 2017 -0700 ---------------------------------------------------------------------- .../phoenix/schema/types/PArrayDataType.java | 9 + .../apache/phoenix/schema/types/PBoolean.java | 9 + .../apache/phoenix/schema/types/PDataType.java | 5 +- .../org/apache/phoenix/schema/types/PDate.java | 9 + .../apache/phoenix/schema/types/PDecimal.java | 9 + .../apache/phoenix/schema/types/PDouble.java | 9 + .../org/apache/phoenix/schema/types/PLong.java | 9 + .../apache/phoenix/schema/types/PTimestamp.java | 9 + .../phoenix/schema/types/PUnsignedDouble.java | 259 ++++++++++--------- .../phoenix/schema/types/PUnsignedLong.java | 9 + .../apache/phoenix/schema/types/PVarchar.java | 9 + .../phoenix/schema/types/PDataTypeTest.java | 31 +++ 12 files changed, 250 insertions(+), 126 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java index c06dd1c..7d742e2 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PArrayDataType.java @@ -1098,6 +1098,15 @@ public abstract class PArrayDataType<T> extends PDataType<T> { @Override public int compareTo(Object lhs, Object rhs) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } PhoenixArray lhsArr = (PhoenixArray)lhs; PhoenixArray rhsArr = (PhoenixArray)rhs; if (lhsArr.equals(rhsArr)) { return 0; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PBoolean.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PBoolean.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PBoolean.java index f90d70b..ea33818 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PBoolean.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PBoolean.java @@ -113,6 +113,15 @@ public class PBoolean extends PDataType<Boolean> { @Override public int compareTo(Object lhs, Object rhs, PDataType rhsType) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } return Booleans.compare((Boolean) lhs, (Boolean) rhs); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java index c3366f4..1e29d6f 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDataType.java @@ -1102,9 +1102,12 @@ public abstract class PDataType<T> implements DataType<T>, Comparable<PDataType< } public String toStringLiteral(Object o, Format formatter) { + if (o == null) { + return String.valueOf(o); + } if (formatter != null) { return formatter.format(o); - } else if (null == o) { return String.valueOf(o); } + } return o.toString(); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDate.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDate.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDate.java index d70a658..3dd6c69 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDate.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDate.java @@ -128,6 +128,15 @@ public class PDate extends PDataType<Date> { @Override public int compareTo(Object lhs, Object rhs, PDataType rhsType) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } if (rhsType == PTimestamp.INSTANCE || rhsType == PUnsignedTimestamp.INSTANCE) { return -rhsType.compareTo(rhs, lhs, PTime.INSTANCE); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDecimal.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDecimal.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDecimal.java index b76febb..ff77b7e 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDecimal.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDecimal.java @@ -202,6 +202,15 @@ public class PDecimal extends PRealNumber<BigDecimal> { @Override public int compareTo(Object lhs, Object rhs, PDataType rhsType) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } if (rhsType == PDecimal.INSTANCE) { return ((BigDecimal) lhs).compareTo((BigDecimal) rhs); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDouble.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDouble.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDouble.java index f390813..5691149 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDouble.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PDouble.java @@ -35,6 +35,15 @@ public class PDouble extends PRealNumber<Double> { @Override public int compareTo(Object lhs, Object rhs, PDataType rhsType) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } if (rhsType == PDecimal.INSTANCE) { return -((BigDecimal) rhs).compareTo(BigDecimal.valueOf(((Number) lhs).doubleValue())); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PLong.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PLong.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PLong.java index 2d6ff27..80f3de4 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PLong.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PLong.java @@ -199,6 +199,15 @@ public class PLong extends PWholeNumber<Long> { @Override public int compareTo(Object lhs, Object rhs, PDataType rhsType) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } if (rhsType == PDecimal.INSTANCE) { return -((BigDecimal) rhs).compareTo(BigDecimal.valueOf(((Number) lhs).longValue())); } else if (equalsAny(rhsType, PDouble.INSTANCE, PFloat.INSTANCE, PUnsignedDouble.INSTANCE, PUnsignedFloat.INSTANCE)) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PTimestamp.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PTimestamp.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PTimestamp.java index c35a039..452a2be 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PTimestamp.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PTimestamp.java @@ -203,6 +203,15 @@ public class PTimestamp extends PDataType<Timestamp> { @Override public int compareTo(Object lhs, Object rhs, PDataType rhsType) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } if (equalsAny(rhsType, PTimestamp.INSTANCE, PUnsignedTimestamp.INSTANCE)) { return ((java.sql.Timestamp) lhs).compareTo((java.sql.Timestamp) rhs); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedDouble.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedDouble.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedDouble.java index 14e41e1..2937380 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedDouble.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedDouble.java @@ -27,134 +27,143 @@ import com.google.common.primitives.Doubles; public class PUnsignedDouble extends PRealNumber<PDouble> { - public static final PUnsignedDouble INSTANCE = new PUnsignedDouble(); - - private PUnsignedDouble() { - super("UNSIGNED_DOUBLE", 15, Double.class, new UnsignedDoubleCodec(), 20); - } - - @Override - public int compareTo(Object lhs, Object rhs, PDataType rhsType) { - if (rhsType == PDecimal.INSTANCE) { - return -((BigDecimal) rhs).compareTo(BigDecimal.valueOf(((Number) lhs).doubleValue())); - } - return Doubles.compare(((Number) lhs).doubleValue(), ((Number) rhs).doubleValue()); - } - - @Override - public boolean isFixedWidth() { - return true; - } - - @Override - public Integer getByteSize() { - return Bytes.SIZEOF_DOUBLE; - } - - @Override - public Integer getScale(Object o) { - return PDouble.INSTANCE.getScale(o); - } - - @Override - public Integer getMaxLength(Object o) { - return PDouble.INSTANCE.getMaxLength(o); - } - - @Override - public byte[] toBytes(Object object) { - byte[] b = new byte[Bytes.SIZEOF_DOUBLE]; - toBytes(object, b, 0); - return b; - } - - @Override - public int toBytes(Object object, byte[] bytes, int offset) { - if (object == null) { - throw newIllegalDataException(this + " may not be null"); - } - return this.getCodec().encodeDouble(((Number) object).doubleValue(), - bytes, offset); - } - - @Override - public Object toObject(String value) { - if (value == null || value.length() == 0) { - return null; - } - try { - Double d = Double.parseDouble(value); - if (d.doubleValue() < 0) { - throw newIllegalDataException("Value may not be negative(" - + d + ")"); - } - return d; - } catch (NumberFormatException e) { - throw newIllegalDataException(e); - } - } - - @Override - public Object toObject(Object object, PDataType actualType) { - Double v = (Double) PDouble.INSTANCE.toObject(object, actualType); - throwIfNonNegativeNumber(v); - return v; - } - - @Override - public Object toObject(byte[] b, int o, int l, PDataType actualType, SortOrder sortOrder, - Integer maxLength, Integer scale) { - Double v = (Double) PDouble.INSTANCE.toObject(b, o, l, actualType, sortOrder); - throwIfNonNegativeNumber(v); - return v; - } - - @Override - public boolean isCoercibleTo(PDataType targetType, Object value) { - return super.isCoercibleTo(targetType, value) || PDouble.INSTANCE - .isCoercibleTo(targetType, value); - } - - @Override - public boolean isCoercibleTo(PDataType targetType) { - return this.equals(targetType) || PDouble.INSTANCE.isCoercibleTo(targetType); - } - - @Override - public int getResultSetSqlType() { - return PDouble.INSTANCE.getResultSetSqlType(); - } - - @Override - public Object getSampleValue(Integer maxLength, Integer arrayLength) { - return Math.abs((Double) PDouble.INSTANCE.getSampleValue(maxLength, arrayLength)); - } - - static class UnsignedDoubleCodec extends PDouble.DoubleCodec { + public static final PUnsignedDouble INSTANCE = new PUnsignedDouble(); + + private PUnsignedDouble() { + super("UNSIGNED_DOUBLE", 15, Double.class, new UnsignedDoubleCodec(), 20); + } + + @Override + public int compareTo(Object lhs, Object rhs, PDataType rhsType) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } + if (rhsType == PDecimal.INSTANCE) { + return -((BigDecimal) rhs).compareTo(BigDecimal.valueOf(((Number) lhs).doubleValue())); + } + return Doubles.compare(((Number) lhs).doubleValue(), ((Number) rhs).doubleValue()); + } + + @Override + public boolean isFixedWidth() { + return true; + } + + @Override + public Integer getByteSize() { + return Bytes.SIZEOF_DOUBLE; + } @Override - public int encodeDouble(double v, byte[] b, int o) { - checkForSufficientLength(b, o, Bytes.SIZEOF_DOUBLE); - if (v < 0) { - throw newIllegalDataException(); - } - Bytes.putDouble(b, o, v); - return Bytes.SIZEOF_DOUBLE; + public Integer getScale(Object o) { + return PDouble.INSTANCE.getScale(o); } @Override - public double decodeDouble(byte[] b, int o, SortOrder sortOrder) { - Preconditions.checkNotNull(sortOrder); - checkForSufficientLength(b, o, Bytes.SIZEOF_DOUBLE); - if (sortOrder == SortOrder.DESC) { - b = SortOrder.invert(b, o, new byte[Bytes.SIZEOF_DOUBLE], 0, Bytes.SIZEOF_DOUBLE); - o = 0; - } - double v = Bytes.toDouble(b, o); - if (v < 0) { - throw newIllegalDataException(); - } - return v; - } - } + public Integer getMaxLength(Object o) { + return PDouble.INSTANCE.getMaxLength(o); + } + + @Override + public byte[] toBytes(Object object) { + byte[] b = new byte[Bytes.SIZEOF_DOUBLE]; + toBytes(object, b, 0); + return b; + } + + @Override + public int toBytes(Object object, byte[] bytes, int offset) { + if (object == null) { + throw newIllegalDataException(this + " may not be null"); + } + return this.getCodec().encodeDouble(((Number) object).doubleValue(), + bytes, offset); + } + + @Override + public Object toObject(String value) { + if (value == null || value.length() == 0) { + return null; + } + try { + Double d = Double.parseDouble(value); + if (d.doubleValue() < 0) { + throw newIllegalDataException("Value may not be negative(" + + d + ")"); + } + return d; + } catch (NumberFormatException e) { + throw newIllegalDataException(e); + } + } + + @Override + public Object toObject(Object object, PDataType actualType) { + Double v = (Double) PDouble.INSTANCE.toObject(object, actualType); + throwIfNonNegativeNumber(v); + return v; + } + + @Override + public Object toObject(byte[] b, int o, int l, PDataType actualType, SortOrder sortOrder, + Integer maxLength, Integer scale) { + Double v = (Double) PDouble.INSTANCE.toObject(b, o, l, actualType, sortOrder); + throwIfNonNegativeNumber(v); + return v; + } + + @Override + public boolean isCoercibleTo(PDataType targetType, Object value) { + return super.isCoercibleTo(targetType, value) || PDouble.INSTANCE + .isCoercibleTo(targetType, value); + } + + @Override + public boolean isCoercibleTo(PDataType targetType) { + return this.equals(targetType) || PDouble.INSTANCE.isCoercibleTo(targetType); + } + + @Override + public int getResultSetSqlType() { + return PDouble.INSTANCE.getResultSetSqlType(); + } + + @Override + public Object getSampleValue(Integer maxLength, Integer arrayLength) { + return Math.abs((Double) PDouble.INSTANCE.getSampleValue(maxLength, arrayLength)); + } + + static class UnsignedDoubleCodec extends PDouble.DoubleCodec { + + @Override + public int encodeDouble(double v, byte[] b, int o) { + checkForSufficientLength(b, o, Bytes.SIZEOF_DOUBLE); + if (v < 0) { + throw newIllegalDataException(); + } + Bytes.putDouble(b, o, v); + return Bytes.SIZEOF_DOUBLE; + } + + @Override + public double decodeDouble(byte[] b, int o, SortOrder sortOrder) { + Preconditions.checkNotNull(sortOrder); + checkForSufficientLength(b, o, Bytes.SIZEOF_DOUBLE); + if (sortOrder == SortOrder.DESC) { + b = SortOrder.invert(b, o, new byte[Bytes.SIZEOF_DOUBLE], 0, Bytes.SIZEOF_DOUBLE); + o = 0; + } + double v = Bytes.toDouble(b, o); + if (v < 0) { + throw newIllegalDataException(); + } + return v; + } + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedLong.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedLong.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedLong.java index 1ee46fd..e593f3c 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedLong.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PUnsignedLong.java @@ -135,6 +135,15 @@ public class PUnsignedLong extends PWholeNumber<Long> { @Override public int compareTo(Object lhs, Object rhs, PDataType rhsType) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } if (rhsType == PDecimal.INSTANCE) { return -((BigDecimal) rhs).compareTo(BigDecimal.valueOf(((Number) lhs).longValue())); } else if (equalsAny(rhsType, PDouble.INSTANCE, PFloat.INSTANCE, PUnsignedDouble.INSTANCE, http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PVarchar.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PVarchar.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PVarchar.java index 0ddf622..fb5a045 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PVarchar.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PVarchar.java @@ -133,6 +133,15 @@ public class PVarchar extends PDataType<String> { @Override public int compareTo(Object lhs, Object rhs, PDataType rhsType) { + if (lhs == rhs) { + return 0; + } + if (lhs == null) { + return -1; + } + if (rhs == null) { + return 1; + } return ((String) lhs).compareTo((String) rhs); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/5c16a29b/phoenix-core/src/test/java/org/apache/phoenix/schema/types/PDataTypeTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/schema/types/PDataTypeTest.java b/phoenix-core/src/test/java/org/apache/phoenix/schema/types/PDataTypeTest.java index c28e5b1..4b02cea 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/schema/types/PDataTypeTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/schema/types/PDataTypeTest.java @@ -1918,4 +1918,35 @@ public class PDataTypeTest { assertTrue(Bytes.compareTo(expectedLowerRange, range.getLowerRange()) == 0); assertTrue(Bytes.compareTo(upperRange, range.getUpperRange()) == 0); } + + @Test + public void testCompareToNull() { + for (PDataType type1 : PDataType.values()) { + Object value1 = type1.getSampleValue(); + for (PDataType type2 : PDataType.values()) { + Object value2 = null; + if (type1.isComparableTo(type2)) { + assertTrue(type1.compareTo(value1, value2, type2) > 0); + } + } + } + for (PDataType type1 : PDataType.values()) { + Object value1 = null; + for (PDataType type2 : PDataType.values()) { + Object value2 = type2.getSampleValue(); + if (type1.isComparableTo(type2)) { + assertTrue(type1.compareTo(value1, value2, type2) < 0); + } + } + } + for (PDataType type1 : PDataType.values()) { + Object value1 = null; + for (PDataType type2 : PDataType.values()) { + Object value2 = null; + if (type1.isComparableTo(type2)) { + assertTrue(type1.compareTo(value1, value2, type2) == 0); + } + } + } + } }
