Repository: phoenix Updated Branches: refs/heads/4.8-HBase-1.0 1afcb2e35 -> a28119d52
PHOENIX-3382 Query using Row Value Constructor comprised of non leading PK columns returns incorrect results Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/a28119d5 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/a28119d5 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/a28119d5 Branch: refs/heads/4.8-HBase-1.0 Commit: a28119d52ede8107c8237740678ea1ddd9ceb2c1 Parents: 1afcb2e Author: James Taylor <jamestay...@apache.org> Authored: Sat Oct 15 17:05:21 2016 -0700 Committer: James Taylor <jamestay...@apache.org> Committed: Tue Oct 25 17:21:33 2016 -0700 ---------------------------------------------------------------------- .../org/apache/phoenix/end2end/QueryMoreIT.java | 103 +++++++++++++++++++ .../apache/phoenix/compile/WhereOptimizer.java | 4 + .../phoenix/compile/WhereOptimizerTest.java | 61 +++++++++++ 3 files changed, 168 insertions(+) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/a28119d5/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryMoreIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryMoreIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryMoreIT.java index c4e4665..c8eb95a 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryMoreIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/QueryMoreIT.java @@ -368,4 +368,107 @@ public class QueryMoreIT extends BaseHBaseManagedTimeTableReuseIT { assertNull(rs.getBigDecimal(2, 10)); } } + + // FIXME: this repros PHOENIX-3382, but turned up two more issues: + // 1) PHOENIX-3383 Comparison between descending row keys used in RVC is reverse + // 2) PHOENIX-3384 Optimize RVC expressions for non leading row key columns + @Test + public void testRVCOnDescWithLeadingPKEquality() throws Exception { + final Connection conn = DriverManager.getConnection(getUrl()); + String fullTableName = generateRandomString(); + try (Statement stmt = conn.createStatement()) { + stmt.execute("CREATE TABLE " + fullTableName + "(\n" + + " ORGANIZATION_ID CHAR(15) NOT NULL,\n" + + " SCORE DOUBLE NOT NULL,\n" + + " ENTITY_ID CHAR(15) NOT NULL\n" + + " CONSTRAINT PAGE_SNAPSHOT_PK PRIMARY KEY (\n" + + " ORGANIZATION_ID,\n" + + " SCORE DESC,\n" + + " ENTITY_ID DESC\n" + + " )\n" + + ") MULTI_TENANT=TRUE"); + } + + conn.createStatement().execute("UPSERT INTO " + fullTableName + " VALUES ('org1',3,'01')"); + conn.createStatement().execute("UPSERT INTO " + fullTableName + " VALUES ('org1',2,'04')"); + conn.createStatement().execute("UPSERT INTO " + fullTableName + " VALUES ('org1',2,'03')"); + conn.createStatement().execute("UPSERT INTO " + fullTableName + " VALUES ('org1',1,'02')"); + conn.commit(); + + // FIXME: PHOENIX-3383 + // This comparison is really backwards: it should be (score, entity_id) < (2, '04'), + // but because we're matching a descending key, our comparison has to be switched. + try (Statement stmt = conn.createStatement()) { + final ResultSet rs = stmt.executeQuery("SELECT entity_id, score\n" + + "FROM " + fullTableName + "\n" + + "WHERE organization_id = 'org1'\n" + + "AND (score, entity_id) > (2, '04')\n" + + "ORDER BY score DESC, entity_id DESC\n" + + "LIMIT 3"); + assertTrue(rs.next()); + assertEquals("03", rs.getString(1)); + assertEquals(2.0, rs.getDouble(2), 0.001); + assertTrue(rs.next()); + assertEquals("02", rs.getString(1)); + assertEquals(1.0, rs.getDouble(2), 0.001); + assertFalse(rs.next()); + } + // FIXME: PHOENIX-3384 + // It should not be necessary to specify organization_id in this query + try (Statement stmt = conn.createStatement()) { + final ResultSet rs = stmt.executeQuery("SELECT entity_id, score\n" + + "FROM " + fullTableName + "\n" + + "WHERE organization_id = 'org1'\n" + + "AND (organization_id, score, entity_id) > ('org1', 2, '04')\n" + + "ORDER BY score DESC, entity_id DESC\n" + + "LIMIT 3"); + assertTrue(rs.next()); + assertEquals("03", rs.getString(1)); + assertEquals(2.0, rs.getDouble(2), 0.001); + assertTrue(rs.next()); + assertEquals("02", rs.getString(1)); + assertEquals(1.0, rs.getDouble(2), 0.001); + assertFalse(rs.next()); + } + } + + @Test + public void testSingleDescPKColumnComparison() throws Exception { + final Connection conn = DriverManager.getConnection(getUrl()); + String fullTableName = generateRandomString(); + try (Statement stmt = conn.createStatement()) { + stmt.execute("CREATE TABLE " + fullTableName + "(\n" + + " ORGANIZATION_ID CHAR(15) NOT NULL,\n" + + " SCORE DOUBLE NOT NULL,\n" + + " ENTITY_ID CHAR(15) NOT NULL\n" + + " CONSTRAINT PAGE_SNAPSHOT_PK PRIMARY KEY (\n" + + " ORGANIZATION_ID,\n" + + " SCORE DESC,\n" + + " ENTITY_ID DESC\n" + + " )\n" + + ") MULTI_TENANT=TRUE"); + } + + conn.createStatement().execute("UPSERT INTO " + fullTableName + " VALUES ('org1',3,'01')"); + conn.createStatement().execute("UPSERT INTO " + fullTableName + " VALUES ('org1',2,'04')"); + conn.createStatement().execute("UPSERT INTO " + fullTableName + " VALUES ('org1',2,'03')"); + conn.createStatement().execute("UPSERT INTO " + fullTableName + " VALUES ('org1',1,'02')"); + conn.commit(); + + try (Statement stmt = conn.createStatement()) { + // Even though SCORE is descending, the > comparison makes sense logically + // and doesn't have to be reversed as RVC expression comparisons do (which + // is really just a bug - see PHOENIX-3383). + final ResultSet rs = stmt.executeQuery("SELECT entity_id, score\n" + + "FROM " + fullTableName + "\n" + + "WHERE organization_id = 'org1'\n" + + "AND score > 2.0\n" + + "ORDER BY score DESC\n" + + "LIMIT 3"); + assertTrue(rs.next()); + assertEquals("01", rs.getString(1)); + assertEquals(3.0, rs.getDouble(2), 0.001); + assertFalse(rs.next()); + } + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/a28119d5/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java index f49aa52..ba1dc8b 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java @@ -487,6 +487,10 @@ public class WhereOptimizer { int initPosition = (table.getBucketNum() ==null ? 0 : 1) + (this.context.getConnection().getTenantId() != null && table.isMultiTenant() ? 1 : 0) + (table.getViewIndexId() == null ? 0 : 1); if (initPosition == slot.getPKPosition()) { minMaxRange = keyRange; + } else { + // If we cannot set the minMaxRange, then we must not extract the expression since + // we wouldn't be constraining the range at all based on it. + extractNode = null; } } return newKeyParts(slot, extractNode, keyRanges, minMaxRange); http://git-wip-us.apache.org/repos/asf/phoenix/blob/a28119d5/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 c50975b..7275c59 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 @@ -62,6 +62,7 @@ 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.PDouble; import org.apache.phoenix.schema.types.PInteger; import org.apache.phoenix.schema.types.PLong; import org.apache.phoenix.schema.types.PUnsignedLong; @@ -2032,4 +2033,64 @@ public class WhereOptimizerTest extends BaseConnectionlessQueryTest { context = compileStatement("select avg(pk1) from test order by count(distinct pk2)"); assertEquals(2, context.getAggregationManager().getAggregators().getAggregatorCount()); } + + @Test + public void testRVCWithLeadingPKEq() throws SQLException { + String tenantId = "o1"; + Connection conn = DriverManager.getConnection(getUrl()); + conn.createStatement().execute("CREATE TABLE COMMUNITIES.TEST (\n" + + " ORGANIZATION_ID CHAR(2) NOT NULL,\n" + + " SCORE DOUBLE NOT NULL,\n" + + " ENTITY_ID CHAR(2) NOT NULL\n" + + " CONSTRAINT PAGE_SNAPSHOT_PK PRIMARY KEY (\n" + + " ORGANIZATION_ID,\n" + + " SCORE,\n" + + " ENTITY_ID\n" + + " )\n" + + ") VERSIONS=1, MULTI_TENANT=TRUE"); + String query = "SELECT entity_id, score\n" + + "FROM communities.test\n" + + "WHERE organization_id = '" + tenantId + "'\n" + + "AND (score, entity_id) > (2.0, '04')\n" + + "ORDER BY score, entity_id"; + Scan scan = compileStatement(query).getScan(); + assertNotNull(scan.getFilter()); + + // See PHOENIX-3384: Optimize RVC expressions for non leading row key columns. + // FIXME: We should be able to optimize this better, taking into account the + // (score, entity_id) > (2.0, '04') to form more of the start/stop row. + assertArrayEquals(PVarchar.INSTANCE.toBytes(tenantId), scan.getStartRow()); + assertArrayEquals(ByteUtil.nextKey(PVarchar.INSTANCE.toBytes(tenantId)), scan.getStopRow()); + } + + @Test + public void testRVCWithCompDescRowKey() throws SQLException { + String tenantId = "o1"; + Connection conn = DriverManager.getConnection(getUrl()); + conn.createStatement().execute("CREATE TABLE COMMUNITIES.TEST (\n" + + " ORGANIZATION_ID CHAR(2) NOT NULL,\n" + + " SCORE DOUBLE NOT NULL,\n" + + " ENTITY_ID CHAR(2) NOT NULL\n" + + " CONSTRAINT PAGE_SNAPSHOT_PK PRIMARY KEY (\n" + + " ORGANIZATION_ID,\n" + + " SCORE DESC,\n" + + " ENTITY_ID DESC\n" + + " )\n" + + ") VERSIONS=1, MULTI_TENANT=TRUE"); + String query = "SELECT entity_id, score\n" + + "FROM communities.test\n" + + "WHERE organization_id = '" + tenantId + "'\n" + + "AND (organization_id, score, entity_id) < ('" + tenantId + "',2.0, '04')\n" + + "ORDER BY score DESC, entity_id DESC"; + Scan scan = compileStatement(query).getScan(); + assertNull(scan.getFilter()); + + // FIXME See PHOENIX-3383: Comparison between descending row keys used in RVC is reverse + // This should set the startRow, but instead it's setting the stopRow + byte[] startRow = PChar.INSTANCE.toBytes(tenantId); + assertArrayEquals(startRow, scan.getStartRow()); + byte[] stopRow = ByteUtil.concat(PChar.INSTANCE.toBytes(tenantId), PDouble.INSTANCE.toBytes(2.0, SortOrder.DESC), PChar.INSTANCE.toBytes("04", SortOrder.DESC)); + assertArrayEquals(stopRow, scan.getStopRow()); + } + }
