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/b64edb75 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/b64edb75 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/b64edb75 Branch: refs/heads/calcite Commit: b64edb75455d56fc2a5086043bdd6fa1064f2ca7 Parents: a82a0ff Author: Ankit Singhal <ankitsingha...@gmail.com> Authored: Fri Feb 5 23:45:04 2016 +0530 Committer: Ankit Singhal <ankitsingha...@gmail.com> Committed: Fri Feb 5 23:45:04 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/b64edb75/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/b64edb75/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/b64edb75/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);