You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/02/14 00:14:41 UTC
[GitHub] [phoenix] yanxinyi opened a new pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
yanxinyi opened a new pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708
…g scan with non-pk ordered pks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on issue #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on issue #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#issuecomment-588562714
While Daniel Wong and I were debugging and discussing the potential way to fix this RVC problem by query rewrite; however, the current WhereOptimizer generates the wrong scan, see PHOENIX-5740. So the current fix is adding an extra flag to identify whether the ordering of conditions matters or not.
This PR is only addressing the fixing logic, I will add more tests tomorrow.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385496567
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE, ID3 BIGINT NOT NULL \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2, ID3)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 3)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, ID3) IN " +
+ "(('005xx000001Sv6o', '000000000000500',1)," +
+ "('005xx000001Sv6o', '000000000000400',2)," +
+ "('005xx000001Sv6o', '000000000000300',3))");
+
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, ID1, ID3) IN " +
+ "(('000000000000400', '005xx000001Sv6o', 1)," +
+ "('000000000000300', '005xx000001Sv6o', 2))");
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (ID3, ID2, ID1) IN " +
+ "((1, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000400", rs.getString(1));
+ }
+ }
+
+ private String getExplainPlanString(ResultSet rs) throws SQLException {
+ StringBuilder builder = new StringBuilder();
+ while (rs.next()) {
+ for (int i = 0; i < rs.getMetaData().getColumnCount(); i++) {
+ Object col = rs.getObject(i + 1);
+ if (col != null) {
+ builder.append(col.toString());
+ builder.append(",");
+ }
+ }
+ builder.append("\n");
+ }
+ return builder.toString();
+ }
+
+ @Test
+ public void testInListExpressionGeneratesRightScan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true); Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " ID3 BIGINT, ID4 BIGINT \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ }
+ testFullPkListPlan(tenantView);
+ testFullPkPlusNonPkInListPlan(tenantView);
+ testPartialPkListPlan(tenantView);
+ testPartialPkPlusNonPkListPlan(tenantView);
+ testNonPkListPlan(tenantView);
+ }
+
+ private void testFullPkPlusNonPkInListPlan(String tenantView) throws Exception {
+ Long numberOfRowsToScan = new Long(2);
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ PreparedStatement preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID1, ID2, ID3) IN " +
+ "(('005xx000001Sv6o', '000000000000500', 1)," +
+ "('005xx000001Sv6o', '000000000000400', 2))");
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertEquals(numberOfRowsToScan, queryPlan.getEstimatedRowsToScan());
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
Review comment:
other strings are not easy to abstract due to the way how to generates the diff query plan
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on issue #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on issue #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#issuecomment-596059185
@dbwong @ChinmaySKulkarni
The previous PR doesn't have a full set of tests, so I added tests to cover:
1. view on the table
2. view on the global view
3. index on view
4. in list expression with function
For the implementation, the InListExpression does handle function as well, it might be easier to rewrite visitors, but I'm uncertain of all restrictions. Please provide more insights here, thanks.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384686986
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
Review comment:
Yes. I do want to check if my changes still delete the right row.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385941012
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -539,151 +551,180 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
- @Test
- public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
- String fullTableName = generateUniqueName();
- String fullViewName = generateUniqueName();
- String tenantView = generateUniqueName();
-
+ private void buildSchema(String fullTableName, String fullViewName, boolean isDecOrder) throws Exception {
try (Connection conn = DriverManager.getConnection(getUrl())) {
conn.setAutoCommit(true);
- Statement stmt = conn.createStatement();
- stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
- " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
- stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
- " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
- "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ try (Statement stmt = conn.createStatement()) {
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ if (isDecOrder) {
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " ID3 BIGINT, ID4 BIGINT \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ try (Statement tenantStmt = viewConn.createStatement()) {
+ tenantStmt.execute("CREATE VIEW IF NOT EXISTS " + this.descViewName + " AS SELECT * FROM " + fullViewName);
+ }
+ }
+ } else {
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " ID3 BIGINT, ID4 BIGINT \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ try (Statement tenantStmt = viewConn.createStatement()) {
+ tenantStmt.execute("CREATE VIEW IF NOT EXISTS " + this.ascViewName + " AS SELECT * FROM " + fullViewName);
+ }
+ }
+ }
+ }
}
+ }
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan1() throws Exception {
+ String tenantView = generateUniqueName();
try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
viewConn.setAutoCommit(true);
- Statement stmt = viewConn.createStatement();
- stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
- viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
- "('005xx000001Sv6o', '000000000000300', 1532458230000)");
- viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
- "('005xx000001Sv6o', '000000000000400', 1532458240000)");
- viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
- "('005xx000001Sv6o', '000000000000500', 1532458250000)");
- viewConn.commit();
+ try (Statement stmt = viewConn.createStatement()) {
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + descViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2) VALUES " +
+ "('foo', '000000000000300')");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2) VALUES " +
+ "('005xx000001Sv6o', '000000000000500')");
+ viewConn.commit();
+
+ try (PreparedStatement preparedStmt = viewConn.prepareStatement(
+ "SELECT * FROM " + tenantView + " WHERE (ID1, ID2) " +
+ "IN (('005xx000001Sv6o', '000000000000500'))")) {
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
- ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
- "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
- "('005xx000001Sv6o', '000000000000300', 1532458230000))");
- assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+ try (PreparedStatement preparedStmt = viewConn.prepareStatement(
+ "SELECT * FROM " + tenantView + " WHERE (ID2, ID1) " +
+ "IN (('000000000000500', '005xx000001Sv6o'))")) {
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
- rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
- "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
- "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
- assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (ID2, ID1) IN " +
+ "(('bar', '005xx000001Sv6o'))");
- rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
- "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
- "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
- assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+ ResultSet rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+ }
- rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
- "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
- "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
- assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+ @Ignore
Review comment:
Please comment why we are ignoring.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385359360
##########
File path: phoenix-core/src/test/java/org/apache/phoenix/expression/InListExpressionTest.java
##########
@@ -62,4 +68,83 @@ public void testHashCode() throws Exception {
+ hashCode, firstHashCode, hashCode);
}
}
+
+ @Test
+ public void testKeyPartsForOrderMatters() {
Review comment:
Also consider adding comments to make the tests clearer
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385371626
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE, ID3 BIGINT NOT NULL \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2, ID3)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 3)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, ID3) IN " +
+ "(('005xx000001Sv6o', '000000000000500',1)," +
+ "('005xx000001Sv6o', '000000000000400',2)," +
+ "('005xx000001Sv6o', '000000000000300',3))");
+
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, ID1, ID3) IN " +
+ "(('000000000000400', '005xx000001Sv6o', 1)," +
+ "('000000000000300', '005xx000001Sv6o', 2))");
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (ID3, ID2, ID1) IN " +
+ "((1, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000400", rs.getString(1));
+ }
+ }
+
+ private String getExplainPlanString(ResultSet rs) throws SQLException {
+ StringBuilder builder = new StringBuilder();
+ while (rs.next()) {
+ for (int i = 0; i < rs.getMetaData().getColumnCount(); i++) {
+ Object col = rs.getObject(i + 1);
+ if (col != null) {
+ builder.append(col.toString());
+ builder.append(",");
+ }
+ }
+ builder.append("\n");
+ }
+ return builder.toString();
+ }
+
+ @Test
+ public void testInListExpressionGeneratesRightScan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true); Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " ID3 BIGINT, ID4 BIGINT \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ }
+ testFullPkListPlan(tenantView);
+ testFullPkPlusNonPkInListPlan(tenantView);
+ testPartialPkListPlan(tenantView);
+ testPartialPkPlusNonPkListPlan(tenantView);
+ testNonPkListPlan(tenantView);
+ }
+
+ private void testFullPkPlusNonPkInListPlan(String tenantView) throws Exception {
+ Long numberOfRowsToScan = new Long(2);
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ PreparedStatement preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID1, ID2, ID3) IN " +
+ "(('005xx000001Sv6o', '000000000000500', 1)," +
+ "('005xx000001Sv6o', '000000000000400', 2))");
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertEquals(numberOfRowsToScan, queryPlan.getEstimatedRowsToScan());
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
Review comment:
Any reason why we only extracted `POINT_LOOKUP_ON_STRING` string out in `ExplainTable` and not any of the other ones?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384403236
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
Review comment:
nit: statements should be in try-with-resources.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385359980
##########
File path: phoenix-core/src/test/java/org/apache/phoenix/expression/InListExpressionTest.java
##########
@@ -62,4 +68,83 @@ public void testHashCode() throws Exception {
+ hashCode, firstHashCode, hashCode);
}
}
+
+ @Test
+ public void testKeyPartsForOrderMatters() {
+ List<Expression> expressionList = new ArrayList<>();
+ final RowKeyColumnExpression rowKeyColumnExpressionMock0 = Mockito.mock(RowKeyColumnExpression.class);
+ when(rowKeyColumnExpressionMock0.getPosition()).thenReturn(0);
+ when(rowKeyColumnExpressionMock0.getDeterminism()).thenReturn(Determinism.ALWAYS);
+
+ final RowKeyColumnExpression rowKeyColumnExpressionMock1 = Mockito.mock(RowKeyColumnExpression.class);
+ when(rowKeyColumnExpressionMock1.getPosition()).thenReturn(1);
+ when(rowKeyColumnExpressionMock1.getDeterminism()).thenReturn(Determinism.ALWAYS);
+ expressionList.add(rowKeyColumnExpressionMock1);
+ expressionList.add(rowKeyColumnExpressionMock0);
+ RowValueConstructorExpression rvc = new RowValueConstructorExpression(expressionList, false);
+
+ final PColumn pColumn0 = Mockito.mock(PColumn.class);
+ final PColumn pColumn1 = Mockito.mock(PColumn.class);
+ List<PColumn> pColumnList = new ArrayList<PColumn>(){{add(pColumn1); add(pColumn0);}};
+ PTable pTable = Mockito.mock(PTable.class);
+ when(pTable.getPKColumns()).thenReturn(pColumnList);
+
+ final WhereOptimizer.KeyExpressionVisitor.BaseKeyPart baseKeyPart0 =
+ new WhereOptimizer.KeyExpressionVisitor.BaseKeyPart(pTable, null, new ArrayList<Expression>(){{add(rowKeyColumnExpressionMock0);}});
+ final WhereOptimizer.KeyExpressionVisitor.BaseKeyPart baseKeyPart1 =
+ new WhereOptimizer.KeyExpressionVisitor.BaseKeyPart(pTable, null, new ArrayList<Expression>(){{add(rowKeyColumnExpressionMock1);}});
+ List<WhereOptimizer.KeyExpressionVisitor.KeySlots> children =
+ new ArrayList<WhereOptimizer.KeyExpressionVisitor.KeySlots>() {{
+ add(new WhereOptimizer.KeyExpressionVisitor.SingleKeySlot(baseKeyPart1,1, null));
+ add(new WhereOptimizer.KeyExpressionVisitor.SingleKeySlot(baseKeyPart0,0, null));
+ }};
+
+ WhereOptimizer.KeyExpressionVisitor visitor = new WhereOptimizer.KeyExpressionVisitor(null, pTable);
+ visitor.setOrderMatterToTrue();
+ WhereOptimizer.KeyExpressionVisitor.KeySlots resultKeySlots = visitor.newRowValueConstructorKeyParts(rvc, children);
+ WhereOptimizer.KeyExpressionVisitor.RowValueConstructorKeyPart keyPart =
+ (WhereOptimizer.KeyExpressionVisitor.RowValueConstructorKeyPart)
+ resultKeySlots.getSlots().iterator().next().getKeyPart();
+ assertEquals(1, keyPart.getChildSlots().get(0).getSlots().get(0).getPKPosition());
+ }
+
+ @Test
+ public void testKeyPartsForOrderDoesNotMatter() {
+ List<Expression> expressionList = new ArrayList<>();
+ final RowKeyColumnExpression rowKeyColumnExpressionMock0 = Mockito.mock(RowKeyColumnExpression.class);
+ when(rowKeyColumnExpressionMock0.getPosition()).thenReturn(0);
+ when(rowKeyColumnExpressionMock0.getDeterminism()).thenReturn(Determinism.ALWAYS);
+
+ final RowKeyColumnExpression rowKeyColumnExpressionMock1 = Mockito.mock(RowKeyColumnExpression.class);
+ when(rowKeyColumnExpressionMock1.getPosition()).thenReturn(1);
+ when(rowKeyColumnExpressionMock1.getDeterminism()).thenReturn(Determinism.ALWAYS);
Review comment:
Ideally you should move all this into a `setup` BeforeClass/Before method since it's not really part of the test. Only the steps that are different for each unit test should be contained here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384409557
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
##########
@@ -590,11 +591,35 @@ private KeySlots newKeyParts(KeySlot slot, List<Expression> extractNodes, List<K
return new SingleKeySlot(new BaseKeyPart(table, slot.getKeyPart().getColumn(), extractNodes), slot.getPKPosition(), slot.getPKSpan(), keyRanges, slot.getOrderPreserving());
}
- private KeySlots newRowValueConstructorKeyParts(RowValueConstructorExpression rvc, List<KeySlots> childSlots) {
+ public KeySlots newRowValueConstructorKeyParts(RowValueConstructorExpression rvc, List<KeySlots> childSlots) {
if (childSlots.isEmpty() || rvc.isStateless()) {
return null;
}
-
+ if (!this.orderMatter) {
+ Collections.sort(childSlots, new Comparator<KeySlots>() {
+ @Override
+ public int compare(KeySlots o1, KeySlots o2) {
+ return o1.getSlots().get(0).getPKPosition() -
Review comment:
I think get(0) is valid because we don't allow nesting on RVCs which is the only case I'm currently aware of that uses multiple slots. Is that the case? And if so we should comment.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385506276
##########
File path: phoenix-core/src/test/java/org/apache/phoenix/expression/InListExpressionTest.java
##########
@@ -62,4 +68,83 @@ public void testHashCode() throws Exception {
+ hashCode, firstHashCode, hashCode);
}
}
+
+ @Test
+ public void testKeyPartsForOrderMatters() {
Review comment:
@dbwong this is only testing one thing here, which is whether we have pk ordering or not.
Due to the way how Phoenix implemented, it's very hard to do the unit tests.
@ChinmaySKulkarni sure will do.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384408595
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
##########
@@ -546,6 +545,8 @@ public Expression visitLeave(AndExpression node, List<Expression> l) {
* operators, CASE statements, and string concatenation.
*/
public static class KeyExpressionVisitor extends StatelessTraverseNoExpressionVisitor<KeyExpressionVisitor.KeySlots> {
+ private boolean orderMatter;
Review comment:
nit: maybe rename, something like orderDependent
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384410655
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
##########
@@ -661,6 +686,17 @@ private KeySlots newCoerceKeyPart(KeySlot slot, final CoerceExpression node) {
childPart, ptr, node, extractNodes), slot.getPKPosition(), slot.getKeyRanges());
}
+ public boolean isOrderMatter() {
+ return this.orderMatter;
+ }
+
+ public void setOrderMatterToTrue() {
Review comment:
nit: I prefer a generic boolean setter, also rename with above.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384403085
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
Review comment:
nit: statements should be in try-with-resources.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385496604
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE, ID3 BIGINT NOT NULL \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2, ID3)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 3)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, ID3) IN " +
+ "(('005xx000001Sv6o', '000000000000500',1)," +
+ "('005xx000001Sv6o', '000000000000400',2)," +
+ "('005xx000001Sv6o', '000000000000300',3))");
+
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, ID1, ID3) IN " +
+ "(('000000000000400', '005xx000001Sv6o', 1)," +
+ "('000000000000300', '005xx000001Sv6o', 2))");
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (ID3, ID2, ID1) IN " +
+ "((1, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000400", rs.getString(1));
+ }
+ }
+
+ private String getExplainPlanString(ResultSet rs) throws SQLException {
Review comment:
removed
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385357036
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
##########
@@ -1814,7 +1853,7 @@ public boolean isPartialExtraction() {
public static class SingleKeySlot implements KeySlots {
private final List<KeySlot> slots;
- SingleKeySlot(KeyPart part, int pkPosition, List<KeyRange> ranges) {
+ public SingleKeySlot(KeyPart part, int pkPosition, List<KeyRange> ranges) {
Review comment:
Why are all these made public now? Do we need them in tests only? If yes, can we use `VisibleForTesting` annotations accordingly?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384687723
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
Review comment:
I want to check after deletion behavior since its end to end testing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384412396
##########
File path: phoenix-core/src/test/java/org/apache/phoenix/expression/InListExpressionTest.java
##########
@@ -19,13 +19,19 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.when;
import com.google.common.collect.Lists;
import java.util.ArrayList;
import java.util.List;
+
import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.compile.WhereOptimizer;
import org.apache.phoenix.hbase.index.util.ImmutableBytesPtr;
+import org.apache.phoenix.schema.PColumn;
+import org.apache.phoenix.schema.PTable;
import org.junit.Test;
+import org.mockito.Mockito;
public class InListExpressionTest {
Review comment:
The tests for this should not live in this class. This class should be only for InListExpression changes, of which you have none.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384404305
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
Review comment:
If this is testing plans we likely do not need individual data upserted.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384688480
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
Review comment:
the previous one tests DESC order by the PK, this one is ASC, which is not using the same schema.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385371065
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE, ID3 BIGINT NOT NULL \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2, ID3)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
Review comment:
I feel like a lot of the lines of code can be shared between various tests with respect table/view creation/upserting data, etc. It will make the tests much more readable as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385507255
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE, ID3 BIGINT NOT NULL \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2, ID3)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
Review comment:
created two views at the initial time, so we don't need to recreate all tables, global views again and again, and this will make our IT tests much faster.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384696212
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java
##########
@@ -367,6 +368,13 @@ public void write(DataOutput output) throws IOException {
@Override
public final <T> T accept(ExpressionVisitor<T> visitor) {
+// if (visitor instanceof WhereOptimizer.KeyExpressionVisitor) {
Review comment:
Actually, this is not a dead comment. I just found that we need to have this additional check for the RVC comparison
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384842849
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java
##########
@@ -367,6 +368,13 @@ public void write(DataOutput output) throws IOException {
@Override
public final <T> T accept(ExpressionVisitor<T> visitor) {
+// if (visitor instanceof WhereOptimizer.KeyExpressionVisitor) {
Review comment:
Can we add a UT for this then?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384694061
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
##########
@@ -590,11 +591,35 @@ private KeySlots newKeyParts(KeySlot slot, List<Expression> extractNodes, List<K
return new SingleKeySlot(new BaseKeyPart(table, slot.getKeyPart().getColumn(), extractNodes), slot.getPKPosition(), slot.getPKSpan(), keyRanges, slot.getOrderPreserving());
}
- private KeySlots newRowValueConstructorKeyParts(RowValueConstructorExpression rvc, List<KeySlots> childSlots) {
+ public KeySlots newRowValueConstructorKeyParts(RowValueConstructorExpression rvc, List<KeySlots> childSlots) {
if (childSlots.isEmpty() || rvc.isStateless()) {
return null;
}
-
+ if (!this.orderMatter) {
+ Collections.sort(childSlots, new Comparator<KeySlots>() {
+ @Override
+ public int compare(KeySlots o1, KeySlots o2) {
+ return o1.getSlots().get(0).getPKPosition() -
Review comment:
comment added
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385369711
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
Review comment:
@yanxinyi So here the where condition is basically on the full row key: <TENANT_ID, KEY_PREFIX, ID1, ID2, EVENT_DATE> such that:
```sql
TENANT_ID=<fixed constant> AND
KEY_PREFIX= '0CY' AND
((ID1='005xx000001Sv6o' AND ID2='000000000000400' AND EVENT_DATE= 1532458240000) OR
(ID1='005xx000001Sv6o' AND ID2='000000000000300' AND EVENT_DATE= 1532458230000))
```
In this case, instead of a range scan, shouldn't the client be able to issue 2 deletes since this is basically 2 point lookups?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384411244
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java
##########
@@ -367,6 +368,13 @@ public void write(DataOutput output) throws IOException {
@Override
public final <T> T accept(ExpressionVisitor<T> visitor) {
+// if (visitor instanceof WhereOptimizer.KeyExpressionVisitor) {
Review comment:
Remove dead comments.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384405682
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
Review comment:
Same comments as above, can also make these tests shared methods with a schema attribute to reuse code.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384842554
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
Review comment:
Right but what schema used can be an attribute as i mentioned.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] yanxinyi closed pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
yanxinyi closed pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384407117
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE, ID3 BIGINT NOT NULL \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2, ID3)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 3)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, ID3) IN " +
+ "(('005xx000001Sv6o', '000000000000500',1)," +
+ "('005xx000001Sv6o', '000000000000400',2)," +
+ "('005xx000001Sv6o', '000000000000300',3))");
+
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, ID1, ID3) IN " +
+ "(('000000000000400', '005xx000001Sv6o', 1)," +
+ "('000000000000300', '005xx000001Sv6o', 2))");
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (ID3, ID2, ID1) IN " +
+ "((1, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000400", rs.getString(1));
+ }
+ }
+
+ private String getExplainPlanString(ResultSet rs) throws SQLException {
+ StringBuilder builder = new StringBuilder();
+ while (rs.next()) {
+ for (int i = 0; i < rs.getMetaData().getColumnCount(); i++) {
+ Object col = rs.getObject(i + 1);
+ if (col != null) {
+ builder.append(col.toString());
+ builder.append(",");
+ }
+ }
+ builder.append("\n");
+ }
+ return builder.toString();
+ }
+
+ @Test
+ public void testInListExpressionGeneratesRightScan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true); Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " ID3 BIGINT, ID4 BIGINT \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ }
+ testFullPkListPlan(tenantView);
+ testFullPkPlusNonPkInListPlan(tenantView);
+ testPartialPkListPlan(tenantView);
+ testPartialPkPlusNonPkListPlan(tenantView);
+ testNonPkListPlan(tenantView);
+ }
+
+ private void testFullPkPlusNonPkInListPlan(String tenantView) throws Exception {
+ Long numberOfRowsToScan = new Long(2);
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ PreparedStatement preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID1, ID2, ID3) IN " +
+ "(('005xx000001Sv6o', '000000000000500', 1)," +
+ "('005xx000001Sv6o', '000000000000400', 2))");
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertEquals(numberOfRowsToScan, queryPlan.getEstimatedRowsToScan());
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+
+ preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID2, ID1, ID3) IN " +
+ "(('000000000000500','005xx000001Sv6o', 1)," +
+ "('000000000000400','005xx000001Sv6o', 2))");
+ queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertEquals(numberOfRowsToScan, queryPlan.getEstimatedRowsToScan());
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+
+ viewConn.prepareStatement("DELETE FROM " + tenantView + " WHERE (ID1, ID2) IN " +
+ "(('005xx000001Sv6o', '000000000000500', 1)," +
+ "('005xx000001Sv6o', '000000000000400', 2))");
+ queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertEquals(numberOfRowsToScan, queryPlan.getEstimatedRowsToScan());
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
+ }
+
+ private void testFullPkListPlan(String tenantView) throws Exception {
+ Long numberOfRowsToScan = new Long(2);
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ PreparedStatement preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID1, ID2) IN " +
+ "(('005xx000001Sv6o', '000000000000500')," +
+ "('005xx000001Sv6o', '000000000000400'))");
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertEquals(numberOfRowsToScan, queryPlan.getEstimatedRowsToScan());
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+
+ viewConn.prepareStatement("DELETE FROM " + tenantView + " WHERE (ID1, ID2) IN " +
+ "(('005xx000001Sv6o', '000000000000500')," +
+ "('005xx000001Sv6o', '000000000000400'))");
+ queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertEquals(numberOfRowsToScan, queryPlan.getEstimatedRowsToScan());
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
+ }
+
+ private void testPartialPkListPlan(String tenantView) throws Exception {
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ PreparedStatement preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID1) IN " +
+ "(('005xx000001Sv6o')," +
+ "('005xx000001Sv6o'))");
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains("CLIENT PARALLEL 1-WAY RANGE SCAN OVER"));
+
+ viewConn.prepareStatement("DELETE FROM " + tenantView + " WHERE (ID1) IN " +
+ "(('005xx000001Sv6o')," +
+ "('005xx000001Sv6o'))");
+ queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains("CLIENT PARALLEL 1-WAY RANGE SCAN OVER"));
+
+ preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID2) IN " +
+ "(('000000000000500')," +
+ "('000000000000400'))");
+ queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains("CLIENT PARALLEL 1-WAY RANGE SCAN OVER"));
+
+ viewConn.prepareStatement("DELETE FROM " + tenantView + " WHERE (ID2) IN " +
+ "(('000000000000500')," +
+ "('000000000000400'))");
+ queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains("CLIENT PARALLEL 1-WAY RANGE SCAN OVER"));
+ }
+ }
+
+ private void testPartialPkPlusNonPkListPlan(String tenantView) throws Exception {
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ PreparedStatement preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID1, ID3) IN " +
+ "(('005xx000001Sv6o', 1)," +
+ "('005xx000001Sv6o', 2))");
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains("CLIENT PARALLEL 1-WAY RANGE SCAN OVER"));
+
+ viewConn.prepareStatement("DELETE FROM " + tenantView + " WHERE (ID1, ID3) IN " +
+ "(('005xx000001Sv6o', 1)," +
+ "('005xx000001Sv6o', 2))");
+ queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains("CLIENT PARALLEL 1-WAY RANGE SCAN OVER"));
+
+ preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID2, ID3) IN " +
+ "(('000000000000500', 1)," +
+ "('000000000000400', 2))");
+ queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains("CLIENT PARALLEL 1-WAY RANGE SCAN OVER"));
+
+ viewConn.prepareStatement("DELETE FROM " + tenantView + " WHERE (ID2, ID3) IN " +
+ "(('000000000000500', 1)," +
+ "('000000000000400', 2))");
+ queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains("CLIENT PARALLEL 1-WAY RANGE SCAN OVER"));
+ }
+ }
+
+ private void testNonPkListPlan(String tenantView) throws Exception {
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1) ) {
+ PreparedStatement preparedStmt = viewConn.prepareStatement("SELECT * FROM " + tenantView + " WHERE (ID3, ID4) IN " +
+ "((1, 1)," +
+ "(2, 2))");
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains("CLIENT PARALLEL 1-WAY RANGE SCAN OVER"));
Review comment:
Maybe add a comment mentioning this is only a range scan due to the tenant connection.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385941145
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -539,151 +551,180 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
- @Test
- public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
- String fullTableName = generateUniqueName();
- String fullViewName = generateUniqueName();
- String tenantView = generateUniqueName();
-
+ private void buildSchema(String fullTableName, String fullViewName, boolean isDecOrder) throws Exception {
try (Connection conn = DriverManager.getConnection(getUrl())) {
conn.setAutoCommit(true);
- Statement stmt = conn.createStatement();
- stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
- " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
- stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
- " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
- "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ try (Statement stmt = conn.createStatement()) {
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ if (isDecOrder) {
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " ID3 BIGINT, ID4 BIGINT \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ try (Statement tenantStmt = viewConn.createStatement()) {
+ tenantStmt.execute("CREATE VIEW IF NOT EXISTS " + this.descViewName + " AS SELECT * FROM " + fullViewName);
+ }
+ }
+ } else {
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " ID3 BIGINT, ID4 BIGINT \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ try (Statement tenantStmt = viewConn.createStatement()) {
+ tenantStmt.execute("CREATE VIEW IF NOT EXISTS " + this.ascViewName + " AS SELECT * FROM " + fullViewName);
+ }
+ }
+ }
+ }
}
+ }
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan1() throws Exception {
+ String tenantView = generateUniqueName();
try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
viewConn.setAutoCommit(true);
- Statement stmt = viewConn.createStatement();
- stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
- viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
- "('005xx000001Sv6o', '000000000000300', 1532458230000)");
- viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
- "('005xx000001Sv6o', '000000000000400', 1532458240000)");
- viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
- "('005xx000001Sv6o', '000000000000500', 1532458250000)");
- viewConn.commit();
+ try (Statement stmt = viewConn.createStatement()) {
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + descViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2) VALUES " +
+ "('foo', '000000000000300')");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2) VALUES " +
+ "('005xx000001Sv6o', '000000000000500')");
+ viewConn.commit();
+
+ try (PreparedStatement preparedStmt = viewConn.prepareStatement(
+ "SELECT * FROM " + tenantView + " WHERE (ID1, ID2) " +
+ "IN (('005xx000001Sv6o', '000000000000500'))")) {
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
- ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
- "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
- "('005xx000001Sv6o', '000000000000300', 1532458230000))");
- assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+ try (PreparedStatement preparedStmt = viewConn.prepareStatement(
+ "SELECT * FROM " + tenantView + " WHERE (ID2, ID1) " +
+ "IN (('000000000000500', '005xx000001Sv6o'))")) {
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
- rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
- "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
- "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
- assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (ID2, ID1) IN " +
+ "(('bar', '005xx000001Sv6o'))");
- rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
- "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
- "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
- assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+ ResultSet rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+ }
- rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
- "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
- "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
- assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+ @Ignore
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan2() throws Exception {
+ String tenantView = generateUniqueName();
- stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
- "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
- "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ try (Statement stmt = viewConn.createStatement()) {
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + descViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2) VALUES " +
+ "('foo', '000000000000300')");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2) VALUES " +
+ "('bar', '000000000000400')");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2) VALUES " +
+ "('005xx000001Sv6o', '000000000000500')");
+ viewConn.commit();
+
+ try (PreparedStatement preparedStmt = viewConn.prepareStatement(
+ "SELECT * FROM " + tenantView + " WHERE (ID1, ID2) " +
+ "IN (('005xx000001Sv6o', '000000000000500')," +
+ "('bar', '000000000000400')," +
+ "('foo', '000000000000300'))")) {
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
- rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
- assertTrue(rs.next());
- assertEquals("000000000000500", rs.getString(1));
+ try (PreparedStatement preparedStmt = viewConn.prepareStatement(
+ "SELECT * FROM " + tenantView + " WHERE (ID2, ID1) IN " +
+ "(('bar', '005xx000001Sv6o')," +
+ "('foo', '005xx000001Sv6o'))")) {
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (ID2, ID1) IN " +
+ "(('bar', '005xx000001Sv6o')," +
+ "('foo', '005xx000001Sv6o'))");
+
+ ResultSet rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ stmt.execute("DELETE * FROM " + tenantView);
+ }
}
}
@Test
public void testInListExpressionWithRightQueryPlan() throws Exception {
- String fullTableName = generateUniqueName();
- String fullViewName = generateUniqueName();
String tenantView = generateUniqueName();
- try (Connection conn = DriverManager.getConnection(getUrl())) {
- conn.setAutoCommit(true);
- Statement stmt = conn.createStatement();
- stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
- " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
- stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE, ID3 BIGINT NOT NULL \n" +
- " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2, ID3)) " +
- "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
- }
-
try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
viewConn.setAutoCommit(true);
- Statement stmt = viewConn.createStatement();
- stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
-
- viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
- "('005xx000001Sv6o', '000000000000300', 1)");
- viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
- "('005xx000001Sv6o', '000000000000400', 3)");
- viewConn.commit();
-
- ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, ID3) IN " +
- "(('005xx000001Sv6o', '000000000000500',1)," +
- "('005xx000001Sv6o', '000000000000400',2)," +
- "('005xx000001Sv6o', '000000000000300',3))");
-
- assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+ try (Statement stmt = viewConn.createStatement()) {
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + ascViewName);
+
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2) VALUES " +
+ "('005xx000001Sv6o', '000000000000300')");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2) VALUES " +
+ "('005xx000001Sv6o', '000000000000400')");
+ viewConn.commit();
+
+ try (PreparedStatement preparedStmt = viewConn.prepareStatement(
+ "SELECT * FROM " + tenantView + " WHERE (ID1, ID2) IN " +
+ "(('005xx000001Sv6o', '000000000000500')," +
+ "('005xx000001Sv6o', '000000000000400')," +
+ "('005xx000001Sv6o', '000000000000300'))")) {
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
- rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, ID1, ID3) IN " +
- "(('000000000000400', '005xx000001Sv6o', 1)," +
- "('000000000000300', '005xx000001Sv6o', 2))");
- assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+ try (PreparedStatement preparedStmt = viewConn.prepareStatement(
+ "SELECT * FROM " + tenantView + " WHERE (ID2, ID1) IN " +
+ "(('000000000000400', '005xx000001Sv6o')," +
+ "('000000000000300', '005xx000001Sv6o'))")) {
+ QueryPlan queryPlan = PhoenixRuntime.getOptimizedQueryPlan(preparedStmt);
+ assertTrue(queryPlan.getExplainPlan().toString().contains(ExplainTable.POINT_LOOKUP_ON_STRING));
+ }
- stmt.execute("DELETE FROM " + tenantView + " WHERE (ID3, ID2, ID1) IN " +
- "((1, '000000000000300', '005xx000001Sv6o'))");
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (ID2, ID1) IN " +
+ "(('000000000000300', '005xx000001Sv6o'))");
- rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
- assertTrue(rs.next());
- assertEquals("000000000000400", rs.getString(1));
+ ResultSet rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000400", rs.getString(1));
+ }
}
}
- private String getExplainPlanString(ResultSet rs) throws SQLException {
- StringBuilder builder = new StringBuilder();
- while (rs.next()) {
- for (int i = 0; i < rs.getMetaData().getColumnCount(); i++) {
- Object col = rs.getObject(i + 1);
- if (col != null) {
- builder.append(col.toString());
- builder.append(",");
- }
- }
- builder.append("\n");
- }
- return builder.toString();
+ @Test
+ public void testInListExpressionGeneratesRightScanForAsc() throws Exception {
+ testFullPkListPlan(this.ascViewName);
+ testFullPkPlusNonPkInListPlan(this.ascViewName);
+ testPartialPkListPlan(this.ascViewName);
+ testPartialPkPlusNonPkListPlan(this.ascViewName);
+ testNonPkListPlan(this.ascViewName);
}
+ @Ignore
Review comment:
Same on why we are ignoring.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384403942
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
Review comment:
Do we need this select what is this testing to the this test case?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384406189
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000500", rs.getString(1));
+ }
+ }
+
+ @Test
+ public void testInListExpressionWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE, ID3 BIGINT NOT NULL \n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (ID1, ID2, ID3)) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, ID3) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 3)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, ID3) IN " +
+ "(('005xx000001Sv6o', '000000000000500',1)," +
+ "('005xx000001Sv6o', '000000000000400',2)," +
+ "('005xx000001Sv6o', '000000000000300',3))");
+
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, ID1, ID3) IN " +
+ "(('000000000000400', '005xx000001Sv6o', 1)," +
+ "('000000000000300', '005xx000001Sv6o', 2))");
+ assertTrue(getExplainPlanString(rs).contains("DELETE SINGLE ROW"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (ID3, ID2, ID1) IN " +
+ "((1, '000000000000300', '005xx000001Sv6o'))");
+
+ rs = stmt.executeQuery("SELECT ID2 FROM " + tenantView);
+ assertTrue(rs.next());
+ assertEquals("000000000000400", rs.getString(1));
+ }
+ }
+
+ private String getExplainPlanString(ResultSet rs) throws SQLException {
Review comment:
Doesn't a similar method already exist in PhoenixRuntime? If not consider adding to TestUtils or another class.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385356617
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
##########
@@ -661,6 +686,17 @@ private KeySlots newCoerceKeyPart(KeySlot slot, final CoerceExpression node) {
childPart, ptr, node, extractNodes), slot.getPKPosition(), slot.getKeyRanges());
}
+ public boolean isOrderMatter() {
+ return this.orderMatter;
+ }
+
+ public void setOrderMatterToTrue() {
+ this.orderMatter = true;
+ }
+
+ public void setOrderMatterToFalse() {
Review comment:
Can't we just use 1 setter i.e. `public void setOrderMatters(boolean trueOrFalse)`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385358359
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
##########
@@ -546,6 +545,8 @@ public Expression visitLeave(AndExpression node, List<Expression> l) {
* operators, CASE statements, and string concatenation.
*/
public static class KeyExpressionVisitor extends StatelessTraverseNoExpressionVisitor<KeyExpressionVisitor.KeySlots> {
+ private boolean orderMatter;
Review comment:
@yanxinyi Does `orderMatter` true mean that the RVC is row-key ordered?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384404073
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/InListIT.java
##########
@@ -537,4 +539,267 @@ public void testInListExpressionWithDesc() throws Exception {
}
}
+ @Test
+ public void testInListExpressionWithDescOrderWithRightQueryPlan() throws Exception {
+ String fullTableName = generateUniqueName();
+ String fullViewName = generateUniqueName();
+ String tenantView = generateUniqueName();
+
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ conn.setAutoCommit(true);
+ Statement stmt = conn.createStatement();
+ stmt.execute("CREATE TABLE " + fullTableName + "(\n" + " TENANT_ID CHAR(15) NOT NULL,\n" + " KEY_PREFIX CHAR(3) NOT NULL,\n" +
+ " CONSTRAINT PK PRIMARY KEY (\n" + " TENANT_ID," + " KEY_PREFIX" + ")) MULTI_TENANT=TRUE");
+ stmt.execute("CREATE VIEW " + fullViewName + "(\n" + " ID1 VARCHAR NOT NULL,\n" + " ID2 VARCHAR NOT NULL,\n" + " EVENT_DATE DATE NOT NULL,\n" +
+ " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n" + " ID1, ID2 DESC, EVENT_DATE DESC\n" + ")) " +
+ "AS SELECT * FROM " + fullTableName + " WHERE KEY_PREFIX = '0CY'");
+ }
+
+
+ try (Connection viewConn = DriverManager.getConnection(TENANT_SPECIFIC_URL1)) {
+ viewConn.setAutoCommit(true);
+ Statement stmt = viewConn.createStatement();
+ stmt.execute("CREATE VIEW IF NOT EXISTS " + tenantView + " AS SELECT * FROM " + fullViewName);
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000400', 1532458240000)");
+ viewConn.createStatement().execute("UPSERT INTO " + tenantView + "(ID1, ID2, EVENT_DATE) VALUES " +
+ "('005xx000001Sv6o', '000000000000500', 1532458250000)");
+ viewConn.commit();
+
+ ResultSet rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, ID2, EVENT_DATE) " +
+ "IN (('005xx000001Sv6o', '000000000000400', 1532458240000)," +
+ "('005xx000001Sv6o', '000000000000300', 1532458230000))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID1, EVENT_DATE, ID2) IN " +
+ "(('005xx000001Sv6o', 1532458240000, '000000000000400')," +
+ "('005xx000001Sv6o', 1532458230000, '000000000000300'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (ID2, EVENT_DATE, ID1) IN " +
+ "(('000000000000400', 1532458240000, '005xx000001Sv6o')," +
+ "('000000000000300', 1532458230000, '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ rs = stmt.executeQuery("EXPLAIN DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
+ "((1532458240000, '000000000000400', '005xx000001Sv6o')," +
+ "(1532458230000, '000000000000300', '005xx000001Sv6o'))");
+ assertTrue(getExplainPlanString(rs).contains("CLIENT PARALLEL 1-WAY RANGE SCAN"));
+
+ stmt.execute("DELETE FROM " + tenantView + " WHERE (EVENT_DATE, ID2, ID1) IN " +
Review comment:
Do we need this delete what is this testing to the this test case?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384411123
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
##########
@@ -1952,7 +1991,7 @@ public PTable getTable() {
}
}
- private class RowValueConstructorKeyPart implements KeyPart {
+ public class RowValueConstructorKeyPart implements KeyPart {
Review comment:
On these and the other public methods consider VisibleForTesting annotation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
dbwong commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r384414162
##########
File path: phoenix-core/src/test/java/org/apache/phoenix/expression/InListExpressionTest.java
##########
@@ -62,4 +68,83 @@ public void testHashCode() throws Exception {
+ hashCode, firstHashCode, hashCode);
}
}
+
+ @Test
+ public void testKeyPartsForOrderMatters() {
Review comment:
This is not a great unit test as its testing multiple things. Consider reducing the scope and adding additional tests. Same for below.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #708: PHOENIX-5698 Phoenix Query with RVC IN list expression generates wron…
URL: https://github.com/apache/phoenix/pull/708#discussion_r385355734
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
##########
@@ -590,11 +591,35 @@ private KeySlots newKeyParts(KeySlot slot, List<Expression> extractNodes, List<K
return new SingleKeySlot(new BaseKeyPart(table, slot.getKeyPart().getColumn(), extractNodes), slot.getPKPosition(), slot.getPKSpan(), keyRanges, slot.getOrderPreserving());
}
- private KeySlots newRowValueConstructorKeyParts(RowValueConstructorExpression rvc, List<KeySlots> childSlots) {
+ public KeySlots newRowValueConstructorKeyParts(RowValueConstructorExpression rvc, List<KeySlots> childSlots) {
if (childSlots.isEmpty() || rvc.isStateless()) {
return null;
}
-
+ if (!this.orderMatter) {
+ Collections.sort(childSlots, new Comparator<KeySlots>() {
+ @Override
+ public int compare(KeySlots o1, KeySlots o2) {
+ return o1.getSlots().get(0).getPKPosition() -
Review comment:
@yanxinyi can you add the comment referenced in this conversation about what exactly is happening here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services