Repository: phoenix Updated Branches: refs/heads/master d401fa3a6 -> c435bba8f
PHOENIX-2171 DOUBLE and FLOAT DESC are stored as ASC Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/c435bba8 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/c435bba8 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/c435bba8 Branch: refs/heads/master Commit: c435bba8f375076c47739b757be12f5109bedace Parents: 5330c07 Author: James Taylor <jtay...@salesforce.com> Authored: Tue Aug 11 01:59:23 2015 -0700 Committer: James Taylor <jtay...@salesforce.com> Committed: Tue Aug 11 01:59:37 2015 -0700 ---------------------------------------------------------------------- .../org/apache/phoenix/end2end/SortOrderIT.java | 6 ++-- .../UngroupedAggregateRegionObserver.java | 6 ++++ .../org/apache/phoenix/schema/PTableImpl.java | 10 +++++- .../apache/phoenix/schema/types/PDataType.java | 4 +-- .../apache/phoenix/schema/types/PDouble.java | 18 +++++++---- .../org/apache/phoenix/schema/types/PFloat.java | 18 ++++++----- .../org/apache/phoenix/util/UpgradeUtil.java | 5 +++ .../phoenix/schema/types/PDataTypeTest.java | 33 ++++++++++++++++++++ 8 files changed, 82 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/c435bba8/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java index 9228ab5..fdbd26d 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java @@ -41,6 +41,8 @@ import org.apache.commons.lang.ArrayUtils; import org.apache.phoenix.schema.SortOrder; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PDecimal; +import org.apache.phoenix.schema.types.PDouble; +import org.apache.phoenix.schema.types.PFloat; import org.apache.phoenix.util.PropertiesUtil; import org.junit.Assert; import org.junit.Test; @@ -404,7 +406,7 @@ public class SortOrderIT extends BaseHBaseManagedTimeIT { public void testNonPKCompare() throws Exception { List<Integer> expectedResults = Lists.newArrayList(2,3,4); Integer[] saltBuckets = new Integer[] {null,3}; - PDataType[] dataTypes = new PDataType[] {PDecimal.INSTANCE}; + PDataType[] dataTypes = new PDataType[] {PDecimal.INSTANCE, PDouble.INSTANCE, PFloat.INSTANCE}; for (Integer saltBucket : saltBuckets) { for (PDataType dataType : dataTypes) { for (SortOrder sortOrder : SortOrder.values()) { @@ -420,7 +422,7 @@ public class SortOrderIT extends BaseHBaseManagedTimeIT { List<Integer> rExpectedResults = new ArrayList<>(expectedResults); Collections.reverse(rExpectedResults); Integer[] saltBuckets = new Integer[] {null,3}; - PDataType[] dataTypes = new PDataType[] {PDecimal.INSTANCE}; + PDataType[] dataTypes = new PDataType[] {PDecimal.INSTANCE, PDouble.INSTANCE, PFloat.INSTANCE}; for (Integer saltBucket : saltBuckets) { for (PDataType dataType : dataTypes) { for (SortOrder sortOrder : SortOrder.values()) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/c435bba8/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java index a7e3e44..6b51138 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/UngroupedAggregateRegionObserver.java @@ -88,6 +88,8 @@ import org.apache.phoenix.schema.tuple.MultiKeyValueTuple; import org.apache.phoenix.schema.types.PBinary; import org.apache.phoenix.schema.types.PChar; import org.apache.phoenix.schema.types.PDataType; +import org.apache.phoenix.schema.types.PDouble; +import org.apache.phoenix.schema.types.PFloat; import org.apache.phoenix.util.ByteUtil; import org.apache.phoenix.util.IndexUtil; import org.apache.phoenix.util.KeyValueUtil; @@ -312,6 +314,10 @@ public class UngroupedAggregateRegionObserver extends BaseScannerRegionObserver{ len--; } ptr.set(ptr.get(), ptr.getOffset(), len); + // Special case for re-writing DESC FLOAT and DOUBLE, as they're not inverted like they should be (PHOENIX-2171) + } else if (field.getDataType() == PFloat.INSTANCE || field.getDataType() == PDouble.INSTANCE) { + byte[] invertedBytes = SortOrder.invert(ptr.get(), ptr.getOffset(), ptr.getLength()); + ptr.set(invertedBytes); } } else if (field.getDataType() == PBinary.INSTANCE) { // Remove trailing space characters so that the setValues call below will replace them http://git-wip-us.apache.org/repos/asf/phoenix/blob/c435bba8/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java index 521fb42..8f9bf63 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/PTableImpl.java @@ -57,6 +57,8 @@ import org.apache.phoenix.schema.stats.PTableStatsImpl; import org.apache.phoenix.schema.types.PBinary; import org.apache.phoenix.schema.types.PChar; import org.apache.phoenix.schema.types.PDataType; +import org.apache.phoenix.schema.types.PDouble; +import org.apache.phoenix.schema.types.PFloat; import org.apache.phoenix.schema.types.PVarchar; import org.apache.phoenix.util.ByteUtil; import org.apache.phoenix.util.SchemaUtil; @@ -407,7 +409,13 @@ public class PTableImpl implements PTable { for (PColumn column : allColumns) { PName familyName = column.getFamilyName(); if (familyName == null) { - hasColumnsRequiringUpgrade |= (column.getSortOrder() == SortOrder.DESC && (!column.getDataType().isFixedWidth() || column.getDataType() == PChar.INSTANCE || column.getDataType() == PBinary.INSTANCE)) + hasColumnsRequiringUpgrade |= + ( column.getSortOrder() == SortOrder.DESC + && (!column.getDataType().isFixedWidth() + || column.getDataType() == PChar.INSTANCE + || column.getDataType() == PFloat.INSTANCE + || column.getDataType() == PDouble.INSTANCE + || column.getDataType() == PBinary.INSTANCE) ) || (column.getSortOrder() == SortOrder.ASC && column.getDataType() == PBinary.INSTANCE && column.getMaxLength() != null && column.getMaxLength() > 1); pkColumns.add(column); } http://git-wip-us.apache.org/repos/asf/phoenix/blob/c435bba8/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 d79de60..5d8852c 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 @@ -66,16 +66,15 @@ public abstract class PDataType<T> implements DataType<T>, Comparable<PDataType< this.ordinal = ordinal; } - @Deprecated public static PDataType[] values() { return PDataTypeFactory.getInstance().getOrderedTypes(); } - @Deprecated public int ordinal() { return ordinal; } + @SuppressWarnings("unchecked") @Override public Class<T> encodedClass() { return getJavaClass(); @@ -942,6 +941,7 @@ public abstract class PDataType<T> implements DataType<T>, Comparable<PDataType< public abstract Object toObject(byte[] bytes, int offset, int length, PDataType actualType, SortOrder sortOrder, Integer maxLength, Integer scale); + @SuppressWarnings("unchecked") @Override public T decode(PositionedByteRange pbr) { // default implementation based on existing PDataType methods. http://git-wip-us.apache.org/repos/asf/phoenix/blob/c435bba8/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 d11aedf..95a526e 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 @@ -236,15 +236,21 @@ public class PDouble extends PRealNumber<Double> { } @Override - public double decodeDouble(byte[] b, int o, SortOrder sortOrder) { + public double decodeDouble(byte[] bytes, int o, SortOrder sortOrder) { Preconditions.checkNotNull(sortOrder); - checkForSufficientLength(b, o, Bytes.SIZEOF_LONG); + checkForSufficientLength(bytes, o, Bytes.SIZEOF_LONG); + long l; if (sortOrder == SortOrder.DESC) { - for (int i = o; i < Bytes.SIZEOF_LONG; i++) { - b[i] = (byte) (b[i] ^ 0xff); - } + // Copied from Bytes.toLong(), but without using the toLongUnsafe + // TODO: would it be possible to use the toLongUnsafe? + l = 0; + for(int i = o; i < o + Bytes.SIZEOF_LONG; i++) { + l <<= 8; + l ^= (bytes[i] ^ 0xff) & 0xFF; + } + } else { + l = Bytes.toLong(bytes, o); } - long l = Bytes.toLong(b, o); l--; l ^= (~l >> Long.SIZE - 1) | Long.MIN_VALUE; return Double.longBitsToDouble(l); http://git-wip-us.apache.org/repos/asf/phoenix/blob/c435bba8/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PFloat.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PFloat.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PFloat.java index 67b7d9a..75f8efa 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PFloat.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/types/PFloat.java @@ -243,15 +243,19 @@ public class PFloat extends PRealNumber<Float> { public float decodeFloat(byte[] b, int o, SortOrder sortOrder) { Preconditions.checkNotNull(sortOrder); checkForSufficientLength(b, o, Bytes.SIZEOF_INT); + int value; if (sortOrder == SortOrder.DESC) { - for (int i = o; i < Bytes.SIZEOF_INT; i++) { - b[i] = (byte) (b[i] ^ 0xff); - } + value = 0; + for(int i = o; i < (o + Bytes.SIZEOF_INT); i++) { + value <<= 8; + value ^= (b[i] ^ 0xff) & 0xFF; + } + } else { + value = Bytes.toInt(b, o); } - int i = Bytes.toInt(b, o); - i--; - i ^= (~i >> Integer.SIZE - 1) | Integer.MIN_VALUE; - return Float.intBitsToFloat(i); + value--; + value ^= (~value >> Integer.SIZE - 1) | Integer.MIN_VALUE; + return Float.intBitsToFloat(value); } @Override http://git-wip-us.apache.org/repos/asf/phoenix/blob/c435bba8/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java index 2ab4f8b..c931851 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java @@ -84,6 +84,8 @@ import org.apache.phoenix.schema.types.PBoolean; import org.apache.phoenix.schema.types.PChar; import org.apache.phoenix.schema.types.PDataType; import org.apache.phoenix.schema.types.PDecimal; +import org.apache.phoenix.schema.types.PDouble; +import org.apache.phoenix.schema.types.PFloat; import org.apache.phoenix.schema.types.PInteger; import org.apache.phoenix.schema.types.PLong; import org.apache.phoenix.schema.types.PVarbinary; @@ -884,12 +886,15 @@ public class UpgradeUtil { // Return all types that are descending and either: // 1) variable length, which includes all array types (PHOENIX-2067) // 2) fixed length with padding (PHOENIX-2120) + // 3) float and double (PHOENIX-2171) // We exclude VARBINARY as we no longer support DESC for it. private static String getAffectedDataTypes() { StringBuilder buf = new StringBuilder("(" + PVarchar.INSTANCE.getSqlType() + "," + + PChar.INSTANCE.getSqlType() + "," + + PBinary.INSTANCE.getSqlType() + "," + + + PFloat.INSTANCE.getSqlType() + "," + + + PDouble.INSTANCE.getSqlType() + "," + + PDecimal.INSTANCE.getSqlType() + "," ); for (PDataType type : PDataType.values()) { http://git-wip-us.apache.org/repos/asf/phoenix/blob/c435bba8/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 7ab9093..5657c22 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 @@ -41,6 +41,7 @@ import org.apache.phoenix.exception.SQLExceptionCode; import org.apache.phoenix.query.QueryConstants; import org.apache.phoenix.schema.ConstraintViolationException; import org.apache.phoenix.schema.SortOrder; +import org.apache.phoenix.util.ScanUtil; import org.apache.phoenix.util.TestUtil; import org.junit.Test; @@ -743,6 +744,38 @@ public class PDataTypeTest { } @Test + public void testDoubleComparison() { + testRealNumberComparison(PDouble.INSTANCE, new Double[] {0.99, 1.0, 1.001, 1.01, 2.0}); + } + + @Test + public void testFloatComparison() { + testRealNumberComparison(PFloat.INSTANCE, new Float[] {0.99f, 1.0f, 1.001f, 1.01f, 2.0f}); + } + + @Test + public void testDecimalComparison() { + testRealNumberComparison(PDecimal.INSTANCE, new BigDecimal[] {BigDecimal.valueOf(0.99), BigDecimal.valueOf(1.0), BigDecimal.valueOf(1.001), BigDecimal.valueOf(1.01), BigDecimal.valueOf(2.0)}); + } + + private static void testRealNumberComparison(PDataType type, Object[] a) { + + for (SortOrder sortOrder : SortOrder.values()) { + int factor = (sortOrder == SortOrder.ASC ? 1 : -1); + byte[] prev_b = null; + Object prev_o = null; + for (Object o : a) { + byte[] b = type.toBytes(o, sortOrder); + if (prev_b != null) { + assertTrue("Compare of " + o + " with " + prev_o + " " + sortOrder + " failed.", ScanUtil.getComparator(type.isFixedWidth(), sortOrder).compare(prev_b, 0, prev_b.length, b, 0, b.length) * factor < 0); + } + prev_b = b; + prev_o = o; + } + } + } + + @Test public void testDouble() { Double na = 0.005; byte[] b = PDouble.INSTANCE.toBytes(na);