You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by go...@apache.org on 2022/03/14 21:51:25 UTC

[phoenix] branch master updated: PHOENIX-6662 Failed to delete when PK with DESC with IN clause

This is an automated email from the ASF dual-hosted git repository.

gokcen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/master by this push:
     new eb0d761  PHOENIX-6662 Failed to delete when PK with DESC with IN clause
eb0d761 is described below

commit eb0d7610831a67151f14e1d61c53fb40ddedaf33
Author: Gokcen Iskender <go...@gmail.com>
AuthorDate: Mon Mar 7 13:36:48 2022 -0800

    PHOENIX-6662 Failed to delete when PK with DESC with IN clause
    
    Signed-off-by: Gokcen Iskender <go...@gmail.com>
---
 .../apache/phoenix/end2end/SkipScanQueryIT.java    | 153 +++++++++++++++++++++
 .../org/apache/phoenix/compile/WhereOptimizer.java |  19 ++-
 .../java/org/apache/phoenix/query/KeyRange.java    |   2 +-
 3 files changed, 167 insertions(+), 7 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SkipScanQueryIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SkipScanQueryIT.java
index 74c9300..68abb0d 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/SkipScanQueryIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/SkipScanQueryIT.java
@@ -22,6 +22,7 @@ import static org.apache.phoenix.util.TestUtil.assertResultSet;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.io.StringReader;
 import java.math.BigDecimal;
@@ -814,6 +815,158 @@ public class SkipScanQueryIT extends ParallelStatsDisabledIT {
     }
 
     @Test
+    public void testRvcInListDelete() throws Exception {
+        String tableName = "TBL_" + generateUniqueName();
+        String viewName = "VW_" + generateUniqueName();
+        String baseTableDDL = "CREATE TABLE IF NOT EXISTS "+ tableName + " (\n" +
+                "    ORG_ID CHAR(15) NOT NULL,\n" +
+                "    KEY_PREFIX CHAR(3) NOT NULL,\n" +
+                "    DATE1 DATE,\n" +
+                "    CONSTRAINT PK PRIMARY KEY (\n" +
+                "        ORG_ID,\n" +
+                "        KEY_PREFIX\n" +
+                "    )\n" +
+                ") VERSIONS=1, MULTI_TENANT=true, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, COLUMN_ENCODED_BYTES=0";
+        String globalViewDDL = "CREATE VIEW  " + viewName +  "(\n" +
+                "    DOUBLE1 DECIMAL(12, 3) NOT NULL,\n" +
+                "    INT1 BIGINT NOT NULL,\n" +
+                "    DATE_TIME1 DATE,\n" +
+                "    COS_ID CHAR(15),\n" +
+                "    TEXT1 VARCHAR,\n" +
+                "    CONSTRAINT PKVIEW PRIMARY KEY\n" +
+                "    (\n" +
+                "        DOUBLE1 DESC, INT1\n" +
+                "    )\n" +
+                ")\n" +
+                "AS SELECT * FROM " + tableName + " WHERE KEY_PREFIX = '08K'";
+        String tenantViewName = tableName + ".\"08K\"";
+        String tenantViewDDL = "CREATE VIEW " + tenantViewName + " AS SELECT * FROM " + viewName;
+        createTestTable(getUrl(), baseTableDDL);
+        createTestTable(getUrl(), globalViewDDL);
+
+        Properties tenantProps = new Properties();
+        tenantProps.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, "tenant1");
+        String upsertOne = "UPSERT INTO " + tenantViewName + " (DOUBLE1, INT1) VALUES (10.0,10)";
+        String upsertTwo = "UPSERT INTO " + tenantViewName + " (DOUBLE1, INT1) VALUES (20.0,20)";
+        String deletion = "DELETE FROM " + tenantViewName + " WHERE (DOUBLE1,INT1) IN ((10.0, 10),(20.0, 20))";
+        String deletionNotPkOrder = "DELETE FROM " + tenantViewName + " WHERE (INT1,DOUBLE1) IN ((10, 10.0),(20, 20.0))";
+
+        try (Connection tenantConn = DriverManager.getConnection(getUrl(), tenantProps)) {
+            tenantConn.createStatement().execute(tenantViewDDL);
+            tenantConn.createStatement().execute(upsertOne);
+            tenantConn.createStatement().execute(upsertTwo);
+            tenantConn.commit();
+
+            ResultSet rs = tenantConn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tenantViewName);
+            if (rs.next()) {
+                int count = rs.getInt(1);
+                assertEquals(2, count);
+            } else {
+                fail();
+            }
+            tenantConn.createStatement().execute(deletion);
+            tenantConn.commit();
+            rs = tenantConn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tenantViewName);
+            if (rs.next()) {
+                int count = rs.getInt(1);
+                assertEquals(0, count);
+            } else {
+                fail();
+            }
+
+            tenantConn.createStatement().execute(upsertOne);
+            tenantConn.createStatement().execute(upsertTwo);
+            tenantConn.commit();
+            tenantConn.createStatement().execute(deletionNotPkOrder);
+            tenantConn.commit();
+            rs = tenantConn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tenantViewName);
+            if (rs.next()) {
+                int count = rs.getInt(1);
+                assertEquals(0, count);
+            } else {
+                fail();
+            }
+        }
+
+    }
+
+    @Test
+    public void testRvcInListDeleteBothDesc() throws Exception {
+        String tableName = "TBL_" + generateUniqueName();
+        String viewName = "VW_" + generateUniqueName();
+        String baseTableDDL = "CREATE TABLE IF NOT EXISTS "+ tableName + " (\n" +
+                "    ORG_ID CHAR(15) NOT NULL,\n" +
+                "    KEY_PREFIX CHAR(3) NOT NULL,\n" +
+                "    DATE1 DATE,\n" +
+                "    CONSTRAINT PK PRIMARY KEY (\n" +
+                "        ORG_ID,\n" +
+                "        KEY_PREFIX\n" +
+                "    )\n" +
+                ") VERSIONS=1, MULTI_TENANT=true, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, COLUMN_ENCODED_BYTES=0";
+        String globalViewDDL = "CREATE VIEW  " + viewName +  "(\n" +
+                "    DOUBLE1 DECIMAL(12, 3) NOT NULL,\n" +
+                "    INT1 BIGINT NOT NULL,\n" +
+                "    DATE_TIME1 DATE,\n" +
+                "    COS_ID CHAR(15),\n" +
+                "    TEXT1 VARCHAR,\n" +
+                "    CONSTRAINT PKVIEW PRIMARY KEY\n" +
+                "    (\n" +
+                "        DOUBLE1 DESC, INT1 DESC\n" +
+                "    )\n" +
+                ")\n" +
+                "AS SELECT * FROM " + tableName + " WHERE KEY_PREFIX = '08K'";
+        String tenantViewName = tableName + ".\"08K\"";
+        String tenantViewDDL = "CREATE VIEW " + tenantViewName + " AS SELECT * FROM " + viewName;
+        createTestTable(getUrl(), baseTableDDL);
+        createTestTable(getUrl(), globalViewDDL);
+
+        Properties tenantProps = new Properties();
+        tenantProps.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, "tenant1");
+        String upsertOne = "UPSERT INTO " + tenantViewName + " (DOUBLE1, INT1) VALUES (10.0,10)";
+        String upsertTwo = "UPSERT INTO " + tenantViewName + " (DOUBLE1, INT1) VALUES (20.0,20)";
+        String deletion = "DELETE FROM " + tenantViewName + " WHERE (DOUBLE1,INT1) IN ((10.0, 10),(20.0, 20))";
+        String deletionNotPkOrder = "DELETE FROM " + tenantViewName + " WHERE (INT1,DOUBLE1) IN ((10, 10.0),(20, 20.0))";
+
+        try (Connection tenantConn = DriverManager.getConnection(getUrl(), tenantProps)) {
+            tenantConn.createStatement().execute(tenantViewDDL);
+            tenantConn.createStatement().execute(upsertOne);
+            tenantConn.createStatement().execute(upsertTwo);
+            tenantConn.commit();
+
+            ResultSet rs = tenantConn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tenantViewName);
+            if (rs.next()) {
+                int count = rs.getInt(1);
+                assertEquals(2, count);
+            } else {
+                fail();
+            }
+            tenantConn.createStatement().execute(deletion);
+            tenantConn.commit();
+            rs = tenantConn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tenantViewName);
+            if (rs.next()) {
+                int count = rs.getInt(1);
+                assertEquals(0, count);
+            } else {
+                fail();
+            }
+
+            tenantConn.createStatement().execute(upsertOne);
+            tenantConn.createStatement().execute(upsertTwo);
+            tenantConn.commit();
+            tenantConn.createStatement().execute(deletionNotPkOrder);
+            tenantConn.commit();
+            rs = tenantConn.createStatement().executeQuery("SELECT COUNT(*) FROM " + tenantViewName);
+            if (rs.next()) {
+                int count = rs.getInt(1);
+                assertEquals(0, count);
+            } else {
+                fail();
+            }
+        }
+
+    }
+
+    @Test
     public void testKeyRangesContainsAllValues() throws Exception {
         String tableName = generateUniqueName();
         String ddl = "CREATE TABLE IF NOT EXISTS " + tableName + "(" +
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
index e0f5da3..5475c16 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
@@ -30,6 +30,7 @@ import java.util.NoSuchElementException;
 import java.util.Set;
 
 import org.apache.phoenix.expression.DelegateExpression;
+import org.apache.phoenix.schema.ValueSchema;
 import org.apache.phoenix.thirdparty.com.google.common.base.Optional;
 import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
@@ -227,12 +228,17 @@ public class WhereOptimizer {
             boolean stopExtracting = false;
             // Iterate through all spans of this slot
             boolean areAllSingleKey = KeyRange.areAllSingleKey(keyRanges);
+            boolean isInList = false;
+            if (keyPart.getExtractNodes() != null && keyPart.getExtractNodes().size() > 0
+                    && keyPart.getExtractNodes().get(0) instanceof InListExpression){
+                isInList = true;
+            }
             while (true) {
                 SortOrder sortOrder =
                         schema.getField(slot.getPKPosition() + slotOffset).getSortOrder();
                 if (prevSortOrder == null)  {
                     prevSortOrder = sortOrder;
-                } else if (prevSortOrder != sortOrder) {
+                } else if (prevSortOrder != sortOrder || (prevSortOrder == SortOrder.DESC && isInList)) {
                     //Consider the Universe of keys to be [0,7]+ on the leading column A
                     // and [0,7]+ on trailing column B, with a padbyte of 0 for ASC and 7 for DESC
                     //if our key range for ASC keys is leading [2,*] and trailing [3,*],
@@ -296,7 +302,7 @@ public class WhereOptimizer {
             } else {
                 if (schema.getField(
                        slot.getPKPosition() + slotOffset - 1).getSortOrder() == SortOrder.DESC) {
-                    keyRanges = invertKeyRanges(keyRanges);
+                   keyRanges = invertKeyRanges(keyRanges);
                 }
                 pkPos = slot.getPKPosition() + slotOffset;
                 slotSpanArray[cnf.size()] = clipLeftSpan-1;
@@ -2197,10 +2203,11 @@ public class WhereOptimizer {
                     // for the non-equality cases return actual sort order
                     //This work around should work
                     // but a more general approach can be taken.
-                    if(rvcElementOp == CompareOp.EQUAL ||
-                            rvcElementOp == CompareOp.NOT_EQUAL){
-                        return SortOrder.ASC;
-                    }
+                    //This optimization causes PHOENIX-6662 (when desc pk used with in clause)
+//                    if(rvcElementOp == CompareOp.EQUAL ||
+//                            rvcElementOp == CompareOp.NOT_EQUAL){
+//                        return SortOrder.ASC;
+//                    }
                     return childPart.getColumn().getSortOrder();
                 }
 
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java b/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
index 686c279..e154480 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/query/KeyRange.java
@@ -615,7 +615,7 @@ public class KeyRange implements Writable {
         }
         return result;
     }
-    
+
     public KeyRange invert() {
         // these special ranges do not get inverted because we
         // represent NULL in the same way for ASC and DESC.