Fixes #434: (ENG-3783) Deletes of specific range key incorrectly dele…

…ting all rows for that partition key

Summary:
When deciding whether the full primary key or only the partition key is set, we may encounter
partially set clustering (range) key. In that case we must make sure we pass the conditions to the
where clause for filtering, or it will be ignored (since the full primary key is not set).

Test Plan: TestDelete.java

Reviewers: robert

Reviewed By: robert

Subscribers: kannan, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D5327

java/yb-cql/src/test/java/org/yb/cql/TestDelete.java

@@ -144,7 +144,6 @@ public void testRangeDelete() throws Exception {
     //----------------------------------------------------------------------------------------------
     // Test Valid Statements
 
-
     // Test Delete entire hash.
     {
       session.execute("DELETE FROM " + tableName + " WHERE h1 = 0 and h2 = 'h0'");
@@ -207,21 +206,41 @@ public void testRangeDelete() throws Exception {
       assertFalse(rows.hasNext());
     }
 
+    // Test delete with partial range key (equality condition).
+    {
+      // Delete entry 3.
+      session.execute(
+          "DELETE FROM " + tableName + " WHERE h1 = 4 and h2 = 'h4' " +
+              "AND r1 = 3");
+
+      // Check Rows.
+      Iterator<Row> rows = session.execute(select.bind(new Integer(4), "h4")).iterator();
+      for (int r = 0; r < 3; r++) {
+        assertTrue(rows.hasNext());
+        assertEquals(r, rows.next().getInt("r1"));
+      }
+      for (int r = 4; r < 10; r++) {
+        assertTrue(rows.hasNext());
+        assertEquals(r, rows.next().getInt("r1"));
+      }
+      assertFalse(rows.hasNext());
+    }
+
     // Test static column when deleting all rows.
     {
       // Delete all entries.
       session.execute(
-          "DELETE FROM " + tableName + " WHERE h1 = 4 and h2 = 'h4' " +
+          "DELETE FROM " + tableName + " WHERE h1 = 5 and h2 = 'h5' " +
               "AND r1 >= 0 AND r1 <= 10");
 
       // Check Rows.
-      Iterator<Row> rows = session.execute(select.bind(new Integer(4), "h4")).iterator();
+      Iterator<Row> rows = session.execute(select.bind(new Integer(5), "h5")).iterator();
       // Omer: Changed this to true. Cassandra actually returns the static column if it
       // was not deleted
       assertTrue(rows.hasNext());
 
       // Check that static column is not removed.
-      rows = session.execute(static_select.bind(new Integer(4), "h4")).iterator();
+      rows = session.execute(static_select.bind(new Integer(5), "h5")).iterator();
       assertTrue(rows.hasNext());
       assertEquals(1, rows.next().getInt("s"));
       assertFalse(rows.hasNext());
@@ -231,11 +250,11 @@ public void testRangeDelete() throws Exception {
     {
       // Delete some entries but with old timestamp -- should do nothing.
       session.execute(
-          "DELETE FROM " + tableName + " USING TIMESTAMP 1 WHERE h1 = 5 and h2 = 'h5' " +
+          "DELETE FROM " + tableName + " USING TIMESTAMP 1 WHERE h1 = 6 and h2 = 'h6' " +
               "AND r1 >= 0 AND r1 <= 10");
 
       // Check Rows -- all should be there because timestamp was in the past.
-      Iterator<Row> rows = session.execute(select.bind(new Integer(5), "h5")).iterator();
+      Iterator<Row> rows = session.execute(select.bind(new Integer(6), "h6")).iterator();
       for (int r = 0; r < 10; r++) {
         assertTrue(rows.hasNext());
         assertEquals(r, rows.next().getInt("r1"));
@@ -249,10 +268,10 @@ public void testRangeDelete() throws Exception {
       // Delete entries 0,1,2,3,4,5,6 using max long as timestamp.
       session.execute(
           "DELETE FROM " + tableName + " USING TIMESTAMP " + Long.MAX_VALUE +
-              " WHERE h1 = 6 AND h2 = 'h6' AND r1 >= 0 AND r1 <= 6");
+              " WHERE h1 = 7 AND h2 = 'h7' AND r1 >= 0 AND r1 <= 6");
 
       // Check Rows -- delete should applied because timestamp was in the future.
-      Iterator<Row> rows = session.execute(select.bind(new Integer(6), "h6")).iterator();
+      Iterator<Row> rows = session.execute(select.bind(new Integer(7), "h7")).iterator();
       for (int r = 7; r < 10; r++) {
         assertTrue(rows.hasNext());
         assertEquals(r, rows.next().getInt("r1"));

src/yb/yql/cql/ql/ptree/pt_dml.cc

@@ -135,6 +135,11 @@ CHECKED_STATUS PTDmlStmt::AnalyzeWhereExpr(SemContext *sem_context, PTExpr *expr
       if (range_keys != num_key_columns_ - num_hash_key_columns_) {
         if (opcode() == TreeNodeOpcode::kPTDeleteStmt) {
           // Range expression in write requests are allowed for deletes only.
+          for (int idx = num_hash_key_columns_; idx < num_key_columns_; idx++) {
+            if (op_counters[idx].eq_count() != 0) {
+              where_ops_.push_front(key_where_ops_[idx]);
+            }
+          }
           key_where_ops_.resize(num_hash_key_columns_);
         } else {
           return sem_context->Error(expr, "Missing condition on key columns in WHERE clause",