You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by an...@apache.org on 2016/02/05 19:11:54 UTC

phoenix git commit: PHOENIX-2647 Duplicate results in reverse scan when guideposts are traversed (Ankit Singhal)

Repository: phoenix
Updated Branches:
  refs/heads/4.x-HBase-0.98 650ae264d -> 7bbca60c4


PHOENIX-2647 Duplicate results in reverse scan when guideposts are traversed (Ankit Singhal)


Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/7bbca60c
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/7bbca60c
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/7bbca60c

Branch: refs/heads/4.x-HBase-0.98
Commit: 7bbca60c4a1add594d37e50107a1a7613bdd59bf
Parents: 650ae26
Author: Ankit Singhal <an...@gmail.com>
Authored: Fri Feb 5 23:41:36 2016 +0530
Committer: Ankit Singhal <an...@gmail.com>
Committed: Fri Feb 5 23:41:36 2016 +0530

----------------------------------------------------------------------
 .../apache/phoenix/end2end/ReverseScanIT.java   | 21 +++++++++
 .../phoenix/end2end/StatsCollectorIT.java       | 28 +++++++----
 .../java/org/apache/phoenix/util/ScanUtil.java  | 49 ++++++++++----------
 3 files changed, 64 insertions(+), 34 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/7bbca60c/phoenix-core/src/it/java/org/apache/phoenix/end2end/ReverseScanIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ReverseScanIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ReverseScanIT.java
index 35a8025..2722be1 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ReverseScanIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ReverseScanIT.java
@@ -168,6 +168,27 @@ public class ReverseScanIT extends BaseHBaseManagedTimeIT {
     }
 
     @Test
+    public void testReverseScanForSpecificRangeInRegion() throws Exception {
+        Connection conn;
+        ResultSet rs;
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        conn = DriverManager.getConnection(getUrl(), props);
+        conn.createStatement()
+                .execute("CREATE TABLE T" + " ( k VARCHAR, c1.a bigint,c2.b bigint CONSTRAINT pk PRIMARY KEY (k)) ");
+        conn.createStatement().execute("upsert into T values ('a',1,3)");
+        conn.createStatement().execute("upsert into T values ('b',1,3)");
+        conn.createStatement().execute("upsert into T values ('c',1,3)");
+        conn.createStatement().execute("upsert into T values ('d',1,3)");
+        conn.createStatement().execute("upsert into T values ('e',1,3)");
+        conn.commit();
+        rs = conn.createStatement().executeQuery("SELECT k FROM T where k>'b' and k<'d' order by k desc");
+        assertTrue(rs.next());
+        assertEquals("c", rs.getString(1));
+        assertTrue(!rs.next());
+        conn.close();
+    }
+
+    @Test
     public void testReverseScanIndex() throws Exception {
         String tenantId = getOrganizationId();
         initATableValues(tenantId, getSplitsAtRowKeys(tenantId), getUrl());

http://git-wip-us.apache.org/repos/asf/phoenix/blob/7bbca60c/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsCollectorIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsCollectorIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsCollectorIT.java
index caba259..4450152 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsCollectorIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/StatsCollectorIT.java
@@ -123,31 +123,41 @@ public class StatsCollectorIT extends StatsCollectorAbstractIT {
         conn.close();
     }
 
-    @Test
-    public void testNoDuplicatesAfterUpdateStats() throws Throwable {
+    private void testNoDuplicatesAfterUpdateStats(String splitKey) throws Throwable {
         Connection conn;
         PreparedStatement stmt;
         ResultSet rs;
         Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
         conn = DriverManager.getConnection(getUrl(), props);
         conn.createStatement()
-                .execute("CREATE TABLE " + fullTableName +" ( k VARCHAR, c1.a bigint,c2.b bigint CONSTRAINT pk PRIMARY KEY (k))" + tableDDLOptions );
-        conn.createStatement().execute("upsert into " + fullTableName +" values ('abc',1,3)");
-        conn.createStatement().execute("upsert into " + fullTableName +" values ('def',2,4)");
+                .execute("CREATE TABLE " + fullTableName
+                        + " ( k VARCHAR, c1.a bigint,c2.b bigint CONSTRAINT pk PRIMARY KEY (k)) "
+                        + (splitKey != null ? "split on (" + splitKey + ")" : ""));
+        conn.createStatement().execute("upsert into " + fullTableName + " values ('abc',1,3)");
+        conn.createStatement().execute("upsert into " + fullTableName + " values ('def',2,4)");
         conn.commit();
-        // CAll the update statistics query here
         stmt = conn.prepareStatement("UPDATE STATISTICS " + fullTableName);
         stmt.execute();
-        rs = conn.createStatement().executeQuery("SELECT k FROM " + fullTableName);
-        assertTrue(rs.next());
-        assertEquals("abc", rs.getString(1));
+        rs = conn.createStatement().executeQuery("SELECT k FROM " + fullTableName + " order by k desc");
         assertTrue(rs.next());
         assertEquals("def", rs.getString(1));
+        assertTrue(rs.next());
+        assertEquals("abc", rs.getString(1));
         assertTrue(!rs.next());
         conn.close();
     }
 
     @Test
+    public void testNoDuplicatesAfterUpdateStatsWithSplits() throws Throwable {
+        testNoDuplicatesAfterUpdateStats("'abc','def'");
+    }
+
+    @Test
+    public void testNoDuplicatesAfterUpdateStatsWithDesc() throws Throwable {
+        testNoDuplicatesAfterUpdateStats(null);
+    }
+
+    @Test
     public void testUpdateStatsWithMultipleTables() throws Throwable {
         String fullTableName2 = fullTableName+"_2";
         Connection conn;

http://git-wip-us.apache.org/repos/asf/phoenix/blob/7bbca60c/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 ded68d2..360db2c 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
@@ -583,34 +583,33 @@ public class ScanUtil {
         scan.setAttribute(BaseScannerRegionObserver.REVERSE_SCAN, PDataType.TRUE_BYTES);
     }
 
+    private static byte[] getReversedRow(byte[] startRow) {
+        /*
+         * Must get previous key because this is going from an inclusive start key to an exclusive stop key, and we need
+         * the start key to be included. We get the previous key by decrementing the last byte by one. However, with
+         * variable length data types, we need to fill with the max byte value, otherwise, if the start key is 'ab', we
+         * lower it to 'aa' which would cause 'aab' to be included (which isn't correct). So we fill with a 0xFF byte to
+         * prevent this. A single 0xFF would be enough for our primitive types (as that byte wouldn't occur), but for an
+         * arbitrary VARBINARY key we can't know how many bytes to tack on. It's lame of HBase to force us to do this.
+         */
+        byte[] newStartRow = startRow;
+        if (startRow.length != 0) {
+            newStartRow = Arrays.copyOf(startRow, startRow.length + MAX_FILL_LENGTH_FOR_PREVIOUS_KEY.length);
+            if (ByteUtil.previousKey(newStartRow, startRow.length)) {
+                System.arraycopy(MAX_FILL_LENGTH_FOR_PREVIOUS_KEY, 0, newStartRow, startRow.length,
+                        MAX_FILL_LENGTH_FOR_PREVIOUS_KEY.length);
+            } else {
+                newStartRow = HConstants.EMPTY_START_ROW;
+            }
+        }
+        return newStartRow;
+    }
+
     // Start/stop row must be swapped if scan is being done in reverse
     public static void setupReverseScan(Scan scan) {
         if (isReversed(scan)) {
-            byte[] startRow = scan.getStartRow();
-            byte[] stopRow = scan.getStopRow();
-            byte[] newStartRow = startRow;
-            byte[] newStopRow = stopRow;
-            if (startRow.length != 0) {
-                /*
-                 * Must get previous key because this is going from an inclusive start key to an exclusive stop key, and
-                 * we need the start key to be included. We get the previous key by decrementing the last byte by one.
-                 * However, with variable length data types, we need to fill with the max byte value, otherwise, if the
-                 * start key is 'ab', we lower it to 'aa' which would cause 'aab' to be included (which isn't correct).
-                 * So we fill with a 0xFF byte to prevent this. A single 0xFF would be enough for our primitive types (as
-                 * that byte wouldn't occur), but for an arbitrary VARBINARY key we can't know how many bytes to tack
-                 * on. It's lame of HBase to force us to do this.
-                 */
-                newStartRow = Arrays.copyOf(startRow, startRow.length + MAX_FILL_LENGTH_FOR_PREVIOUS_KEY.length);
-                if (ByteUtil.previousKey(newStartRow, startRow.length)) {
-                    System.arraycopy(MAX_FILL_LENGTH_FOR_PREVIOUS_KEY, 0, newStartRow, startRow.length, MAX_FILL_LENGTH_FOR_PREVIOUS_KEY.length);
-                } else {
-                    newStartRow = HConstants.EMPTY_START_ROW;
-                }
-            }
-            if (stopRow.length != 0) {
-                // Must add null byte because we need the start to be exclusive while it was inclusive
-                newStopRow = ByteUtil.concat(stopRow, QueryConstants.SEPARATOR_BYTE_ARRAY);
-            }
+            byte[] newStartRow = getReversedRow(scan.getStartRow());
+            byte[] newStopRow = getReversedRow(scan.getStopRow());
             scan.setStartRow(newStopRow);
             scan.setStopRow(newStartRow);
             scan.setReversed(true);