You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ja...@apache.org on 2015/08/12 03:40:03 UTC
[11/12] phoenix git commit: PHOENIX-2137 Range query on DECIMAL DESC
sometimes incorrect
PHOENIX-2137 Range query on DECIMAL DESC sometimes incorrect
Conflicts:
phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/b6425149
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/b6425149
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/b6425149
Branch: refs/heads/4.5-HBase-0.98
Commit: b6425149eeaf285cba18ea11057fde6db9fc0cdc
Parents: 233b48d
Author: James Taylor <jt...@salesforce.com>
Authored: Mon Aug 3 16:40:47 2015 -0700
Committer: James Taylor <jt...@salesforce.com>
Committed: Tue Aug 11 18:36:52 2015 -0700
----------------------------------------------------------------------
.../org/apache/phoenix/end2end/SortOrderIT.java | 110 ++++++++++++++++++-
.../org/apache/phoenix/compile/ScanRanges.java | 7 +-
.../DescVarLengthFastByteComparisons.java | 12 ++
.../apache/phoenix/filter/SkipScanFilter.java | 13 ++-
.../java/org/apache/phoenix/query/KeyRange.java | 42 +++----
.../apache/phoenix/schema/types/PDataType.java | 3 +-
.../java/org/apache/phoenix/util/ScanUtil.java | 64 +++++++++--
.../org/apache/phoenix/util/SchemaUtil.java | 2 +-
.../phoenix/compile/WhereOptimizerTest.java | 20 ++++
.../DescVarLengthFastByteComparisonsTest.java | 45 ++++++++
.../expression/SortOrderExpressionTest.java | 13 ++-
11 files changed, 288 insertions(+), 43 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/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 0e8fb4f..9228ab5 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
@@ -18,6 +18,7 @@
package org.apache.phoenix.end2end;
import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
+import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
@@ -28,12 +29,18 @@ import java.sql.Date;
import java.sql.DriverManager;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.ArrayList;
import java.util.Calendar;
+import java.util.Collections;
import java.util.GregorianCalendar;
import java.util.List;
import java.util.Properties;
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.util.PropertiesUtil;
import org.junit.Assert;
import org.junit.Test;
@@ -357,6 +364,102 @@ public class SortOrderIT extends BaseHBaseManagedTimeIT {
null, null, new OrderBy("id", OrderBy.Direction.DESC));
}
+ @Test
+ public void descVarLengthAscPKGT() throws Exception {
+ String ddl = "CREATE TABLE " + TABLE + " (k1 INTEGER NOT NULL, k2 VARCHAR, CONSTRAINT pk PRIMARY KEY (k1, k2))";
+ Object[][] insertedRows = new Object[][]{{0, null}, {1, "a"}, {2, "b"}, {3, "ba"}, {4, "baa"}, {5, "c"}, {6, "d"}};
+ Object[][] expectedRows = new Object[][]{{3}, {4}, {5}, {6}};
+ runQueryTest(ddl, upsert("k1", "k2"), select("k1"), insertedRows, expectedRows,
+ new WhereCondition("k2", ">", "'b'"), null, null);
+ }
+
+ @Test
+ public void descVarLengthDescPKGT() throws Exception {
+ String ddl = "CREATE TABLE " + TABLE + " (k1 INTEGER NOT NULL, k2 VARCHAR, CONSTRAINT pk PRIMARY KEY (k1, k2 desc))";
+ Object[][] insertedRows = new Object[][]{{0, null}, {1, "a"}, {2, "b"}, {3, "ba"}, {4, "baa"}, {5, "c"}, {6, "d"}};
+ Object[][] expectedRows = new Object[][]{{3}, {4}, {5}, {6}};
+ runQueryTest(ddl, upsert("k1", "k2"), select("k1"), insertedRows, expectedRows,
+ new WhereCondition("k2", ">", "'b'"), null, null);
+ }
+
+ @Test
+ public void descVarLengthDescPKLTE() throws Exception {
+ String ddl = "CREATE TABLE " + TABLE + " (k1 INTEGER NOT NULL, k2 VARCHAR, CONSTRAINT pk PRIMARY KEY (k1, k2 desc))";
+ Object[][] insertedRows = new Object[][]{{0, null}, {1, "a"}, {2, "b"}, {3, "ba"}, {4, "bb"}, {5, "bc"}, {6, "bba"}, {7, "c"}};
+ Object[][] expectedRows = new Object[][]{{1}, {2}, {3}, {4}};
+ runQueryTest(ddl, upsert("k1", "k2"), select("k1"), insertedRows, expectedRows,
+ new WhereCondition("k2", "<=", "'bb'"), null, null);
+ }
+
+ @Test
+ public void descVarLengthAscPKLTE() throws Exception {
+ String ddl = "CREATE TABLE " + TABLE + " (k1 INTEGER NOT NULL, k2 VARCHAR, CONSTRAINT pk PRIMARY KEY (k1, k2))";
+ Object[][] insertedRows = new Object[][]{{0, null}, {1, "a"}, {2, "b"}, {3, "ba"}, {4, "bb"}, {5, "bc"}, {6, "bba"}, {7, "c"}};
+ Object[][] expectedRows = new Object[][]{{1}, {2}, {3}, {4}};
+ runQueryTest(ddl, upsert("k1", "k2"), select("k1"), insertedRows, expectedRows,
+ new WhereCondition("k2", "<=", "'bb'"), null, null);
+ }
+
+ @Test
+ 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};
+ for (Integer saltBucket : saltBuckets) {
+ for (PDataType dataType : dataTypes) {
+ for (SortOrder sortOrder : SortOrder.values()) {
+ testCompareCompositeKey(saltBucket, dataType, sortOrder, "", expectedResults, "");
+ }
+ }
+ }
+ }
+
+ @Test
+ public void testSkipScanCompare() throws Exception {
+ List<Integer> expectedResults = Lists.newArrayList(2,4);
+ List<Integer> rExpectedResults = new ArrayList<>(expectedResults);
+ Collections.reverse(rExpectedResults);
+ Integer[] saltBuckets = new Integer[] {null,3};
+ PDataType[] dataTypes = new PDataType[] {PDecimal.INSTANCE};
+ for (Integer saltBucket : saltBuckets) {
+ for (PDataType dataType : dataTypes) {
+ for (SortOrder sortOrder : SortOrder.values()) {
+ testCompareCompositeKey(saltBucket, dataType, sortOrder, "k1 in (2,4)", expectedResults, "");
+ testCompareCompositeKey(saltBucket, dataType, sortOrder, "k1 in (2,4)", rExpectedResults, "ORDER BY k1 DESC");
+ }
+ }
+ }
+ }
+
+ private void testCompareCompositeKey(Integer saltBuckets, PDataType dataType, SortOrder sortOrder, String whereClause, List<Integer> expectedResults, String orderBy) throws SQLException {
+ String tableName = "t_" + saltBuckets + "_" + dataType + "_" + sortOrder;
+ String ddl = "create table if not exists " + tableName + " (k1 bigint not null, k2 " + dataType.getSqlTypeName() + (dataType.isFixedWidth() ? " not null" : "") + ", constraint pk primary key (k1,k2 " + sortOrder + "))" + (saltBuckets == null ? "" : (" SALT_BUCKETS= " + saltBuckets));
+ Connection conn = DriverManager.getConnection(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES));
+ conn.createStatement().execute(ddl);
+ if (!dataType.isFixedWidth()) {
+ conn.createStatement().execute("upsert into " + tableName + " values (0, null)");
+ }
+ conn.createStatement().execute("upsert into " + tableName + " values (1, 0.99)");
+ conn.createStatement().execute("upsert into " + tableName + " values (2, 1.01)");
+ conn.createStatement().execute("upsert into " + tableName + " values (3, 2.0)");
+ conn.createStatement().execute("upsert into " + tableName + " values (4, 1.001)");
+ conn.commit();
+
+ String query = "select k1 from " + tableName + " where " + (whereClause.length() > 0 ? (whereClause + " AND ") : "") + " k2>1.0 " + (orderBy.length() == 0 ? "" : orderBy);
+ try {
+ ResultSet rs = conn.createStatement().executeQuery(query);
+
+ for (int k : expectedResults) {
+ assertTrue (tableName, rs.next());
+ assertEquals(tableName, k,rs.getInt(1));
+ }
+
+ assertFalse(tableName, rs.next());
+ } finally {
+ conn.close();
+ }
+ }
+
private void runQueryTest(String ddl, String columnName, Object[][] rows, Object[][] expectedRows) throws Exception {
runQueryTest(ddl, new String[]{columnName}, rows, expectedRows, null);
}
@@ -546,8 +649,13 @@ public class SortOrderIT extends BaseHBaseManagedTimeIT {
return ">";
} else if (operator.equals(">")) {
return "<";
+ } else if (operator.equals(">=")) {
+ return "<=";
+ } else if (operator.equals("<=")) {
+ return ">=";
+ } else {
+ return operator;
}
- return operator;
}
}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java
index 80cfbfe..298cd4e 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/ScanRanges.java
@@ -37,6 +37,7 @@ import org.apache.phoenix.schema.RowKeySchema;
import org.apache.phoenix.schema.SaltingUtil;
import org.apache.phoenix.util.ByteUtil;
import org.apache.phoenix.util.ScanUtil;
+import org.apache.phoenix.util.ScanUtil.BytesComparator;
import org.apache.phoenix.util.SchemaUtil;
import com.google.common.collect.ImmutableList;
@@ -75,10 +76,12 @@ public class ScanRanges {
stripPrefix(minMaxRange.getUpperRange(),offset),
minMaxRange.upperUnbound());
}
+ // We have full keys here, so use field from our varbinary schema
+ BytesComparator comparator = ScanUtil.getComparator(SchemaUtil.VAR_BINARY_SCHEMA.getField(0));
for (byte[] key : keys) {
// Filter now based on unsalted minMaxRange and ignore the point key salt byte
- if ( unsaltedMinMaxRange.compareLowerToUpperBound(key, offset, key.length-offset, true) <= 0 &&
- unsaltedMinMaxRange.compareUpperToLowerBound(key, offset, key.length-offset, true) >= 0) {
+ if ( unsaltedMinMaxRange.compareLowerToUpperBound(key, offset, key.length-offset, true, comparator) <= 0 &&
+ unsaltedMinMaxRange.compareUpperToLowerBound(key, offset, key.length-offset, true, comparator) >= 0) {
keyRanges.add(KeyRange.getKeyRange(key));
}
}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/phoenix-core/src/main/java/org/apache/phoenix/execute/DescVarLengthFastByteComparisons.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/execute/DescVarLengthFastByteComparisons.java b/phoenix-core/src/main/java/org/apache/phoenix/execute/DescVarLengthFastByteComparisons.java
index 40960e0..67d23fc 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/execute/DescVarLengthFastByteComparisons.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/execute/DescVarLengthFastByteComparisons.java
@@ -90,6 +90,12 @@ public class DescVarLengthFastByteComparisons {
public int compareTo(byte[] buffer1, int offset1, int length1, byte[] buffer2, int offset2, int length2) {
// Short circuit equal case
if (buffer1 == buffer2 && offset1 == offset2 && length1 == length2) { return 0; }
+ if (length1 == 0 && length2 != 0) { // nulls sort first, even for descending
+ return -1;
+ }
+ if (length2 == 0 && length1 != 0) { // nulls sort first, even for descending
+ return 1;
+ }
// Bring WritableComparator code local
int end1 = offset1 + length1;
int end2 = offset2 + length2;
@@ -166,6 +172,12 @@ public class DescVarLengthFastByteComparisons {
public int compareTo(byte[] buffer1, int offset1, int length1, byte[] buffer2, int offset2, int length2) {
// Short circuit equal case
if (buffer1 == buffer2 && offset1 == offset2 && length1 == length2) { return 0; }
+ if (length1 == 0 && length2 != 0) { // nulls sort first, even for descending
+ return -1;
+ }
+ if (length2 == 0 && length1 != 0) { // nulls sort first, even for descending
+ return 1;
+ }
int minLength = Math.min(length1, length2);
int minWords = minLength / Longs.BYTES;
int offset1Adj = offset1 + BYTE_ARRAY_BASE_OFFSET;
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java b/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java
index 4dc888d..ff58a18 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/filter/SkipScanFilter.java
@@ -40,6 +40,7 @@ import org.apache.phoenix.query.KeyRange.Bound;
import org.apache.phoenix.schema.RowKeySchema;
import org.apache.phoenix.util.ByteUtil;
import org.apache.phoenix.util.ScanUtil;
+import org.apache.phoenix.util.ScanUtil.BytesComparator;
import org.apache.phoenix.util.SchemaUtil;
import com.google.common.base.Objects;
@@ -202,7 +203,7 @@ public class SkipScanFilter extends FilterBase implements Writable {
if (!lowerUnbound) {
// Find the position of the first slot of the lower range
schema.next(ptr, 0, schema.iterator(lowerInclusiveKey,ptr), slotSpan[0]);
- startPos = ScanUtil.searchClosestKeyRangeWithUpperHigherThanPtr(slots.get(0), ptr, 0);
+ startPos = ScanUtil.searchClosestKeyRangeWithUpperHigherThanPtr(slots.get(0), ptr, 0, schema.getField(0));
// Lower range is past last upper range of first slot, so cannot possibly be in range
if (startPos >= slots.get(0).size()) {
return false;
@@ -213,7 +214,7 @@ public class SkipScanFilter extends FilterBase implements Writable {
if (!upperUnbound) {
// Find the position of the first slot of the upper range
schema.next(ptr, 0, schema.iterator(upperExclusiveKey,ptr), slotSpan[0]);
- endPos = ScanUtil.searchClosestKeyRangeWithUpperHigherThanPtr(slots.get(0), ptr, startPos);
+ endPos = ScanUtil.searchClosestKeyRangeWithUpperHigherThanPtr(slots.get(0), ptr, startPos, schema.getField(0));
// Upper range lower than first lower range of first slot, so cannot possibly be in range
// if (endPos == 0 && Bytes.compareTo(upperExclusiveKey, slots.get(0).get(0).getLowerRange()) <= 0) {
// return false;
@@ -222,7 +223,7 @@ public class SkipScanFilter extends FilterBase implements Writable {
if (endPos >= slots.get(0).size()) {
upperUnbound = true;
endPos = slots.get(0).size()-1;
- } else if (slots.get(0).get(endPos).compareLowerToUpperBound(upperExclusiveKey) >= 0) {
+ } else if (slots.get(0).get(endPos).compareLowerToUpperBound(upperExclusiveKey, ScanUtil.getComparator(schema.getField(0))) >= 0) {
// We know that the endPos range is higher than the previous range, but we need
// to test if it ends before the next range starts.
endPos--;
@@ -389,8 +390,10 @@ public class SkipScanFilter extends FilterBase implements Writable {
int maxOffset = schema.iterator(currentKey, minOffset, length, ptr);
schema.next(ptr, ScanUtil.getRowKeyPosition(slotSpan, i), maxOffset, slotSpan[i]);
while (true) {
+ // Comparator depends on field in schema
+ BytesComparator comparator = ScanUtil.getComparator(schema.getField(ScanUtil.getRowKeyPosition(slotSpan, i)));
// Increment to the next range while the upper bound of our current slot is less than our current key
- while (position[i] < slots.get(i).size() && slots.get(i).get(position[i]).compareUpperToLowerBound(ptr) < 0) {
+ while (position[i] < slots.get(i).size() && slots.get(i).get(position[i]).compareUpperToLowerBound(ptr, comparator) < 0) {
position[i]++;
}
Arrays.fill(position, i+1, position.length, 0);
@@ -440,7 +443,7 @@ public class SkipScanFilter extends FilterBase implements Writable {
ByteUtil.nextKey(startKey, currentLength);
}
i = j;
- } else if (slots.get(i).get(position[i]).compareLowerToUpperBound(ptr) > 0) {
+ } else if (slots.get(i).get(position[i]).compareLowerToUpperBound(ptr, comparator) > 0) {
// Our current key is less than the lower range of the current position in the current slot.
// Seek to the lower range, since it's bigger than the current key
setStartKey(ptr, minOffset, i);
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java b/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
index 0612046..366a123 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
@@ -32,6 +32,7 @@ import org.apache.hadoop.io.Writable;
import org.apache.hadoop.io.WritableUtils;
import org.apache.phoenix.schema.SortOrder;
import org.apache.phoenix.util.ByteUtil;
+import org.apache.phoenix.util.ScanUtil.BytesComparator;
import com.google.common.base.Function;
import com.google.common.collect.ComparisonChain;
@@ -183,28 +184,28 @@ public class KeyRange implements Writable {
return isSingleKey;
}
- public int compareLowerToUpperBound(ImmutableBytesWritable ptr, boolean isInclusive) {
- return compareLowerToUpperBound(ptr.get(), ptr.getOffset(), ptr.getLength(), isInclusive);
+ public int compareLowerToUpperBound(ImmutableBytesWritable ptr, boolean isInclusive, BytesComparator comparator) {
+ return compareLowerToUpperBound(ptr.get(), ptr.getOffset(), ptr.getLength(), isInclusive, comparator);
}
- public int compareLowerToUpperBound(ImmutableBytesWritable ptr) {
- return compareLowerToUpperBound(ptr, true);
+ public int compareLowerToUpperBound(ImmutableBytesWritable ptr, BytesComparator comparator) {
+ return compareLowerToUpperBound(ptr, true, comparator);
}
- public int compareUpperToLowerBound(ImmutableBytesWritable ptr, boolean isInclusive) {
- return compareUpperToLowerBound(ptr.get(), ptr.getOffset(), ptr.getLength(), isInclusive);
+ public int compareUpperToLowerBound(ImmutableBytesWritable ptr, boolean isInclusive, BytesComparator comparator) {
+ return compareUpperToLowerBound(ptr.get(), ptr.getOffset(), ptr.getLength(), isInclusive, comparator);
}
- public int compareUpperToLowerBound(ImmutableBytesWritable ptr) {
- return compareUpperToLowerBound(ptr, true);
+ public int compareUpperToLowerBound(ImmutableBytesWritable ptr, BytesComparator comparator) {
+ return compareUpperToLowerBound(ptr, true, comparator);
}
- public int compareLowerToUpperBound( byte[] b, int o, int l) {
- return compareLowerToUpperBound(b,o,l,true);
+ public int compareLowerToUpperBound( byte[] b, int o, int l, BytesComparator comparator) {
+ return compareLowerToUpperBound(b,o,l,true, comparator);
}
- public int compareLowerToUpperBound( byte[] b) {
- return compareLowerToUpperBound(b,0,b.length);
+ public int compareLowerToUpperBound( byte[] b, BytesComparator comparator) {
+ return compareLowerToUpperBound(b,0,b.length, comparator);
}
/**
@@ -213,15 +214,16 @@ public class KeyRange implements Writable {
* @param o upper bound offset
* @param l upper bound length
* @param isInclusive upper bound inclusive
+ * @param comparator comparator used to do compare the byte array using offset and length
* @return -1 if the lower bound is less than the upper bound,
* 1 if the lower bound is greater than the upper bound,
* and 0 if they are equal.
*/
- public int compareLowerToUpperBound( byte[] b, int o, int l, boolean isInclusive) {
+ public int compareLowerToUpperBound( byte[] b, int o, int l, boolean isInclusive, BytesComparator comparator) {
if (lowerUnbound() || b == KeyRange.UNBOUND) {
return -1;
}
- int cmp = Bytes.compareTo(lowerRange, 0, lowerRange.length, b, o, l);
+ int cmp = comparator.compare(lowerRange, 0, lowerRange.length, b, o, l);
if (cmp > 0) {
return 1;
}
@@ -234,19 +236,19 @@ public class KeyRange implements Writable {
return 1;
}
- public int compareUpperToLowerBound(byte[] b) {
- return compareUpperToLowerBound(b,0,b.length);
+ public int compareUpperToLowerBound(byte[] b, BytesComparator comparator) {
+ return compareUpperToLowerBound(b,0,b.length, comparator);
}
- public int compareUpperToLowerBound(byte[] b, int o, int l) {
- return compareUpperToLowerBound(b,o,l, true);
+ public int compareUpperToLowerBound(byte[] b, int o, int l, BytesComparator comparator) {
+ return compareUpperToLowerBound(b,o,l, true, comparator);
}
- public int compareUpperToLowerBound(byte[] b, int o, int l, boolean isInclusive) {
+ public int compareUpperToLowerBound(byte[] b, int o, int l, boolean isInclusive, BytesComparator comparator) {
if (upperUnbound() || b == KeyRange.UNBOUND) {
return 1;
}
- int cmp = Bytes.compareTo(upperRange, 0, upperRange.length, b, o, l);
+ int cmp = comparator.compare(upperRange, 0, upperRange.length, b, o, l);
if (cmp > 0) {
return 1;
}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/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 43bab0e..d79de60 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
@@ -36,6 +36,7 @@ import org.apache.phoenix.schema.ConstraintViolationException;
import org.apache.phoenix.schema.IllegalDataException;
import org.apache.phoenix.schema.SortOrder;
import org.apache.phoenix.util.ByteUtil;
+import org.apache.phoenix.util.ScanUtil;
import com.google.common.base.Preconditions;
import com.google.common.math.LongMath;
@@ -762,7 +763,7 @@ public abstract class PDataType<T> implements DataType<T>, Comparable<PDataType<
}
return (length1 - length2);
}
- return Bytes.compareTo(ba1, offset1, length1, ba2, offset2, length2) * (so1 == SortOrder.DESC ? -1 : 1);
+ return (so1 == SortOrder.DESC ? -1 : 1) * ScanUtil.getComparator(length1 == length2, so1).compare(ba1, offset1, length1, ba2, offset2, length2);
}
public final int compareTo(ImmutableBytesWritable ptr1, SortOrder ptr1SortOrder, ImmutableBytesWritable ptr2,
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java
index 9d104ca..ae073e2 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java
@@ -40,11 +40,13 @@ import org.apache.hadoop.hbase.filter.FilterList;
import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
import org.apache.hadoop.hbase.io.TimeRange;
import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.hadoop.io.WritableComparator;
import org.apache.phoenix.compile.OrderByCompiler.OrderBy;
import org.apache.phoenix.compile.ScanRanges;
import org.apache.phoenix.compile.StatementContext;
import org.apache.phoenix.coprocessor.BaseScannerRegionObserver;
import org.apache.phoenix.coprocessor.MetaDataProtocol;
+import org.apache.phoenix.execute.DescVarLengthFastByteComparisons;
import org.apache.phoenix.filter.BooleanExpressionFilter;
import org.apache.phoenix.filter.SkipScanFilter;
import org.apache.phoenix.query.KeyRange;
@@ -55,6 +57,7 @@ import org.apache.phoenix.query.QueryServicesOptions;
import org.apache.phoenix.schema.PName;
import org.apache.phoenix.schema.PNameFactory;
import org.apache.phoenix.schema.RowKeySchema;
+import org.apache.phoenix.schema.SortOrder;
import org.apache.phoenix.schema.ValueSchema.Field;
import org.apache.phoenix.schema.types.PDataType;
import org.apache.phoenix.schema.types.PVarbinary;
@@ -283,7 +286,15 @@ public class ScanUtil {
for (int i = 0; i < position.length; i++) {
position[i] = bound == Bound.LOWER ? 0 : slots.get(i).size()-1;
KeyRange range = slots.get(i).get(position[i]);
- maxLength += range.getRange(bound).length + (schema.getField(i + slotSpan[i]).getDataType().isFixedWidth() ? 0 : 1);
+ Field field = schema.getField(i + slotSpan[i]);
+ int keyLength = range.getRange(bound).length;
+ if (!field.getDataType().isFixedWidth()) {
+ keyLength++;
+ if (range.isUnbound(bound) && !range.isInclusive(bound) && field.getSortOrder() == SortOrder.DESC) {
+ keyLength++;
+ }
+ }
+ maxLength += keyLength;
}
byte[] key = new byte[maxLength];
int length = setKey(schema, slots, slotSpan, position, bound, key, 0, 0, position.length);
@@ -371,8 +382,8 @@ public class ScanUtil {
// key slots would cause the flag to become true.
lastInclusiveUpperSingleKey = range.isSingleKey() && inclusiveUpper;
anyInclusiveUpperRangeKey |= !range.isSingleKey() && inclusiveUpper;
- // A match for IS NULL or IS NOT NULL should not have a DESC_SEPARATOR_BYTE as nulls sort first
- byte sepByte = SchemaUtil.getSeparatorByte(schema.rowKeyOrderOptimizable(), bytes.length == 0 || range == KeyRange.IS_NULL_RANGE || range == KeyRange.IS_NOT_NULL_RANGE, field);
+ // A null or empty byte array is always represented as a zero byte
+ byte sepByte = SchemaUtil.getSeparatorByte(schema.rowKeyOrderOptimizable(), bytes.length == 0, field);
if (!isFixedWidth && ( fieldIndex < schema.getMaxFields() || inclusiveUpper || exclusiveLower || sepByte == QueryConstants.DESC_SEPARATOR_BYTE)) {
key[offset++] = sepByte;
@@ -383,7 +394,7 @@ public class ScanUtil {
// If we are setting the lower bound with an exclusive range key, we need to bump the
// slot up for each key part. For an upper bound, we bump up an inclusive key, but
// only after the last key part.
- if (!range.isSingleKey() && exclusiveLower) {
+ if (exclusiveLower) {
if (!ByteUtil.nextKey(key, offset)) {
// Special case for not being able to increment.
// In this case we return a negative byteOffset to
@@ -392,6 +403,14 @@ public class ScanUtil {
// have an end key specified.
return -byteOffset;
}
+ // We're filtering on values being non null here, but we still need the 0xFF
+ // terminator, since DESC keys ignore the last byte as it's expected to be
+ // the terminator. Without this, we'd ignore the separator byte that was
+ // just added and incremented.
+ if (!isFixedWidth && bytes.length == 0
+ && SchemaUtil.getSeparatorByte(schema.rowKeyOrderOptimizable(), false, field) == QueryConstants.DESC_SEPARATOR_BYTE) {
+ key[offset++] = QueryConstants.DESC_SEPARATOR_BYTE;
+ }
}
}
if (lastInclusiveUpperSingleKey || anyInclusiveUpperRangeKey) {
@@ -409,7 +428,8 @@ public class ScanUtil {
// byte.
if (bound == Bound.LOWER) {
while (--i >= schemaStartIndex && offset > byteOffset &&
- !schema.getField(--fieldIndex).getDataType().isFixedWidth() &&
+ !(field=schema.getField(--fieldIndex)).getDataType().isFixedWidth() &&
+ field.getSortOrder() == SortOrder.ASC &&
key[offset-1] == QueryConstants.SEPARATOR_BYTE) {
offset--;
fieldIndex -= slotSpan[i];
@@ -417,19 +437,47 @@ public class ScanUtil {
}
return offset - byteOffset;
}
+
+ public static interface BytesComparator {
+ public int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2);
+ };
+ private static final BytesComparator DESC_VAR_WIDTH_COMPARATOR = new BytesComparator() {
+
+ @Override
+ public int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2) {
+ return DescVarLengthFastByteComparisons.compareTo(b1, s1, l1, b2, s2, l2);
+ }
+
+ };
+
+ private static final BytesComparator ASC_FIXED_WIDTH_COMPARATOR = new BytesComparator() {
+
+ @Override
+ public int compare(byte[] b1, int s1, int l1, byte[] b2, int s2, int l2) {
+ return WritableComparator.compareBytes(b1, s1, l1, b2, s2, l2);
+ }
+
+ };
+ public static BytesComparator getComparator(boolean isFixedWidth, SortOrder sortOrder) {
+ return isFixedWidth || sortOrder == SortOrder.ASC ? ASC_FIXED_WIDTH_COMPARATOR : DESC_VAR_WIDTH_COMPARATOR;
+ }
+ public static BytesComparator getComparator(Field field) {
+ return getComparator(field.getDataType().isFixedWidth(),field.getSortOrder());
+ }
/**
* Perform a binary lookup on the list of KeyRange for the tightest slot such that the slotBound
* of the current slot is higher or equal than the slotBound of our range.
* @return the index of the slot whose slot bound equals or are the tightest one that is
* smaller than rangeBound of range, or slots.length if no bound can be found.
*/
- public static int searchClosestKeyRangeWithUpperHigherThanPtr(List<KeyRange> slots, ImmutableBytesWritable ptr, int lower) {
+ public static int searchClosestKeyRangeWithUpperHigherThanPtr(List<KeyRange> slots, ImmutableBytesWritable ptr, int lower, Field field) {
int upper = slots.size() - 1;
int mid;
+ BytesComparator comparator = ScanUtil.getComparator(field.getDataType().isFixedWidth(), field.getSortOrder());
while (lower <= upper) {
mid = (lower + upper) / 2;
- int cmp = slots.get(mid).compareUpperToLowerBound(ptr, true);
+ int cmp = slots.get(mid).compareUpperToLowerBound(ptr, true, comparator);
if (cmp < 0) {
lower = mid + 1;
} else if (cmp > 0) {
@@ -439,7 +487,7 @@ public class ScanUtil {
}
}
mid = (lower + upper) / 2;
- if (mid == 0 && slots.get(mid).compareUpperToLowerBound(ptr, true) > 0) {
+ if (mid == 0 && slots.get(mid).compareUpperToLowerBound(ptr, true, comparator) > 0) {
return mid;
} else {
return ++mid;
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
index d01bf39..5414d4f 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
@@ -625,7 +625,7 @@ public class SchemaUtil {
}
public static int getMaxKeyLength(RowKeySchema schema, List<List<KeyRange>> slots) {
- int maxKeyLength = getTerminatorCount(schema);
+ int maxKeyLength = getTerminatorCount(schema) * 2;
for (List<KeyRange> slot : slots) {
int maxSlotLength = 0;
for (KeyRange range : slot) {
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java
index c1787ca..2e0a0c1 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java
@@ -31,6 +31,7 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import java.math.BigDecimal;
import java.sql.Connection;
import java.sql.Date;
import java.sql.DriverManager;
@@ -55,9 +56,12 @@ import org.apache.phoenix.query.BaseConnectionlessQueryTest;
import org.apache.phoenix.query.KeyRange;
import org.apache.phoenix.query.QueryConstants;
import org.apache.phoenix.schema.ColumnNotFoundException;
+import org.apache.phoenix.schema.SortOrder;
import org.apache.phoenix.schema.types.PChar;
import org.apache.phoenix.schema.types.PDate;
+import org.apache.phoenix.schema.types.PDecimal;
import org.apache.phoenix.schema.types.PInteger;
+import org.apache.phoenix.schema.types.PLong;
import org.apache.phoenix.schema.types.PUnsignedLong;
import org.apache.phoenix.schema.types.PVarchar;
import org.apache.phoenix.util.ByteUtil;
@@ -106,6 +110,22 @@ public class WhereOptimizerTest extends BaseConnectionlessQueryTest {
}
@Test
+ public void testDescDecimalRange() throws SQLException {
+ String ddl = "create table t (k1 bigint not null, k2 decimal, constraint pk primary key (k1,k2 desc))";
+ Connection conn = DriverManager.getConnection(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES));
+ conn.createStatement().execute(ddl);
+ String query = "select * from t where k1 in (1,2) and k2>1.0";
+ Scan scan = compileStatement(query).getScan();
+
+ byte[] startRow = ByteUtil.concat(PLong.INSTANCE.toBytes(1), ByteUtil.nextKey(QueryConstants.SEPARATOR_BYTE_ARRAY), QueryConstants.DESC_SEPARATOR_BYTE_ARRAY);
+ byte[] upperValue = PDecimal.INSTANCE.toBytes(BigDecimal.valueOf(1.0));
+ byte[] stopRow = ByteUtil.concat(PLong.INSTANCE.toBytes(2), SortOrder.invert(upperValue,0,upperValue.length), QueryConstants.DESC_SEPARATOR_BYTE_ARRAY);
+ assertTrue(scan.getFilter() instanceof SkipScanFilter);
+ assertArrayEquals(startRow, scan.getStartRow());
+ assertArrayEquals(stopRow, scan.getStopRow());
+ }
+
+ @Test
public void testSingleCharPaddedKeyExpression() throws SQLException {
String tenantId = "1";
String query = "select * from atable where organization_id='" + tenantId + "'";
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/phoenix-core/src/test/java/org/apache/phoenix/execute/DescVarLengthFastByteComparisonsTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/execute/DescVarLengthFastByteComparisonsTest.java b/phoenix-core/src/test/java/org/apache/phoenix/execute/DescVarLengthFastByteComparisonsTest.java
new file mode 100644
index 0000000..106471b
--- /dev/null
+++ b/phoenix-core/src/test/java/org/apache/phoenix/execute/DescVarLengthFastByteComparisonsTest.java
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.execute;
+
+import static org.junit.Assert.assertTrue;
+
+import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.util.ByteUtil;
+import org.junit.Test;
+
+public class DescVarLengthFastByteComparisonsTest {
+
+ @Test
+ public void testNullIsSmallest() {
+ byte[] b1 = ByteUtil.EMPTY_BYTE_ARRAY;
+ byte[] b2 = Bytes.toBytes("a");
+ int cmp = DescVarLengthFastByteComparisons.compareTo(b1, 0, b1.length, b2, 0, b2.length);
+ assertTrue(cmp < 0);
+ cmp = DescVarLengthFastByteComparisons.compareTo(b2, 0, b2.length, b1, 0, b1.length);
+ assertTrue(cmp > 0);
+ }
+
+ @Test
+ public void testShorterSubstringIsBigger() {
+ byte[] b1 = Bytes.toBytes("ab");
+ byte[] b2 = Bytes.toBytes("a");
+ int cmp = DescVarLengthFastByteComparisons.compareTo(b1, 0, b1.length, b2, 0, b2.length);
+ assertTrue(cmp < 0);
+ }
+}
http://git-wip-us.apache.org/repos/asf/phoenix/blob/b6425149/phoenix-core/src/test/java/org/apache/phoenix/expression/SortOrderExpressionTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/expression/SortOrderExpressionTest.java b/phoenix-core/src/test/java/org/apache/phoenix/expression/SortOrderExpressionTest.java
index b9ee0eb..e2ab684 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/expression/SortOrderExpressionTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/expression/SortOrderExpressionTest.java
@@ -292,17 +292,20 @@ public class SortOrderExpressionTest {
}
private void runCompareTest(CompareOp op, boolean expectedResult, Object lhsValue, PDataType lhsDataType, Object rhsValue, PDataType rhsDataType) throws Exception {
- List<Expression> args = Lists.newArrayList(getLiteral(lhsValue, lhsDataType), getLiteral(rhsValue, rhsDataType));
- evaluateAndAssertResult(new ComparisonExpression(args, op), expectedResult, "lhsDataType: " + lhsDataType + " rhsDataType: " + rhsDataType);
+ List<Expression> args;
+ ImmutableBytesWritable ptr = new ImmutableBytesWritable();
+
+ args = Lists.newArrayList(getLiteral(lhsValue, lhsDataType), getLiteral(rhsValue, rhsDataType));
+ evaluateAndAssertResult(ComparisonExpression.create(op, args, ptr, true), expectedResult, "lhsDataType: " + lhsDataType + " rhsDataType: " + rhsDataType);
args = Lists.newArrayList(getInvertedLiteral(lhsValue, lhsDataType), getLiteral(rhsValue, rhsDataType));
- evaluateAndAssertResult(new ComparisonExpression(args, op), expectedResult, "lhs (inverted) dataType: " + lhsDataType + " rhsDataType: " + rhsDataType);
+ evaluateAndAssertResult(ComparisonExpression.create(op, args, ptr, true), expectedResult, "lhs (inverted) dataType: " + lhsDataType + " rhsDataType: " + rhsDataType);
args = Lists.newArrayList(getLiteral(lhsValue, lhsDataType), getInvertedLiteral(rhsValue, rhsDataType));
- evaluateAndAssertResult(new ComparisonExpression(args, op), expectedResult, "lhsDataType: " + lhsDataType + " rhs (inverted) dataType: " + rhsDataType);
+ evaluateAndAssertResult(ComparisonExpression.create(op, args, ptr, true), expectedResult, "lhsDataType: " + lhsDataType + " rhs (inverted) dataType: " + rhsDataType);
args = Lists.newArrayList(getInvertedLiteral(lhsValue, lhsDataType), getInvertedLiteral(rhsValue, rhsDataType));
- evaluateAndAssertResult(new ComparisonExpression(args, op), expectedResult, "lhs (inverted) dataType: " + lhsDataType + " rhs (inverted) dataType: " + rhsDataType);
+ evaluateAndAssertResult(ComparisonExpression.create(op, args, ptr, true), expectedResult, "lhs (inverted) dataType: " + lhsDataType + " rhs (inverted) dataType: " + rhsDataType);
}
private void evaluateAndAssertResult(Expression expression, Object expectedResult) {