PHOENIX-2194 order by should not require all PK fields with = constraint

phoenix-core/src/it/java/org/apache/phoenix/end2end/RowValueConstructorIT.java

@@ -1522,4 +1522,73 @@ public void testCountDistinct4() throws Exception {
         assertTrue(rs.next());
         assertEquals(3, rs.getInt(1));
     }
+
+    @Test
+    public void testRVCRequiringExtractNodeClear() throws Exception {
+        Connection conn = nextConnection(getUrl());
+        String tableName = "testRVCWithTrailingGT";
+        String ddl = "CREATE TABLE  " + tableName + "  (k1 VARCHAR, k2 VARCHAR, k3 VARCHAR, k4 VARCHAR, CONSTRAINT pk PRIMARY KEY (k1,k2,k3,k4))";
+        conn.createStatement().execute(ddl);
+
+        conn = nextConnection(getUrl());
+        PreparedStatement stmt = conn.prepareStatement("UPSERT INTO  " + tableName + " VALUES('a','b','c','d')");
+        stmt.execute();
+        stmt = conn.prepareStatement("UPSERT INTO  " + tableName + " VALUES('b', 'b', 'c', 'e')");
+        stmt.execute();
+        stmt = conn.prepareStatement("UPSERT INTO  " + tableName + " VALUES('c', 'b','c','f')");
+        stmt.execute();
+        conn.commit();
+
+        conn = nextConnection(getUrl());
+        ResultSet rs;
+        rs = conn.createStatement().executeQuery("SELECT k1 from  " + tableName + " WHERE k1 IN ('a','c') AND (k2,k3) IN (('b','c'),('f','g')) AND k4 > 'c'");
+        assertTrue(rs.next());
+        assertEquals("a", rs.getString(1));
+        assertTrue(rs.next());
+        assertEquals("c", rs.getString(1));
+        assertFalse(rs.next());
+    }
+
+    @Test
+    public void testRVCRequiringNoSkipScan() throws Exception {
+        Connection conn = nextConnection(getUrl());
+        String tableName = "testRVCWithTrailingGT";
+        String ddl = "CREATE TABLE  " + tableName + "  (k1 VARCHAR, k2 VARCHAR, k3 VARCHAR, k4 VARCHAR, CONSTRAINT pk PRIMARY KEY (k1,k2,k3,k4))";
+        conn.createStatement().execute(ddl);
+
+        conn = nextConnection(getUrl());
+        PreparedStatement stmt = conn.prepareStatement("UPSERT INTO  " + tableName + " VALUES('','','a')");
+        stmt.execute();
+        stmt = conn.prepareStatement("UPSERT INTO  " + tableName + " VALUES('', '', 'a', 'a')");
+        stmt.execute();
+        stmt = conn.prepareStatement("UPSERT INTO  " + tableName + " VALUES('', '','b')");
+        stmt.execute();
+        stmt = conn.prepareStatement("UPSERT INTO  " + tableName + " VALUES('', '','b','a')");
+        stmt.execute();
+        stmt = conn.prepareStatement("UPSERT INTO  " + tableName + " VALUES('a', '','c')");
+        stmt.execute();
+        stmt = conn.prepareStatement("UPSERT INTO  " + tableName + " VALUES('a', '','c', 'a')");
+        stmt.execute();
+        conn.commit();
+
+        conn = nextConnection(getUrl());
+        ResultSet rs;
+        rs = conn.createStatement().executeQuery("SELECT k1,k3,k4 from  " + tableName + " WHERE (k1,k2,k3) IN (('','','a'),('','','b'),('a','','c')) AND k4 is not null");
+        assertTrue(rs.next());
+        assertEquals(null, rs.getString(1));
+        assertEquals("a", rs.getString(2));
+        assertEquals("a", rs.getString(3));
+
+        assertTrue(rs.next());
+        assertEquals(null, rs.getString(1));
+        assertEquals("b", rs.getString(2));
+        assertEquals("a", rs.getString(3));
+
+        assertTrue(rs.next());
+        assertEquals("a", rs.getString(1));
+        assertEquals("c", rs.getString(2));
+        assertEquals("a", rs.getString(3));
+
+        assertFalse(rs.next());
+    }
 }

phoenix-core/src/it/java/org/apache/phoenix/end2end/SortOrderIT.java

@@ -402,6 +402,33 @@ public void descVarLengthAscPKLTE() throws Exception {
                 new WhereCondition("k2", "<=", "'bb'"), null, null);
     }
 
+    @Test
+    public void varLengthAscLT() throws Exception {
+        String ddl = "CREATE TABLE " + TABLE + " (k1 VARCHAR NOT NULL, k2 VARCHAR, CONSTRAINT pk PRIMARY KEY (k1, k2))";
+        Object[][] insertedRows = new Object[][]{{"a", ""}, {"b",""}, {"b","a"}};
+        Object[][] expectedRows = new Object[][]{{"a"}};
+        runQueryTest(ddl, upsert("k1", "k2"), select("k1"), insertedRows, expectedRows,
+                new WhereCondition("k1", "<", "'b'"), null, null);
+    }
+
+    @Test
+    public void varLengthDescLT() throws Exception {
+        String ddl = "CREATE TABLE " + TABLE + " (k1 VARCHAR NOT NULL, k2 VARCHAR, CONSTRAINT pk PRIMARY KEY (k1 desc, k2))";
+        Object[][] insertedRows = new Object[][]{{"a", ""}, {"b",""}, {"b","a"}};
+        Object[][] expectedRows = new Object[][]{{"a"}};
+        runQueryTest(ddl, upsert("k1", "k2"), select("k1"), insertedRows, expectedRows,
+                new WhereCondition("k1", "<", "'b'"), null, null);
+    }
+
+    @Test
+    public void varLengthDescGT() throws Exception {
+        String ddl = "CREATE TABLE " + TABLE + " (k1 VARCHAR NOT NULL, k2 VARCHAR, CONSTRAINT pk PRIMARY KEY (k1 desc, k2))";
+        Object[][] insertedRows = new Object[][]{{"a", ""}, {"b",""}, {"b","a"}, {"ba","a"}};
+        Object[][] expectedRows = new Object[][]{{"ba"}};
+        runQueryTest(ddl, upsert("k1", "k2"), select("k1"), insertedRows, expectedRows,
+                new WhereCondition("k1", ">", "'b'"), null, null);
+    }
+
    @Test
     public void testNonPKCompare() throws Exception {
         List<Integer> expectedResults = Lists.newArrayList(2,3,4);

phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java

@@ -30,15 +30,18 @@
 
 import java.sql.Connection;
 import java.sql.DriverManager;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.List;
 
+import org.apache.phoenix.compile.QueryPlan;
 import org.apache.phoenix.exception.SQLExceptionCode;
 import org.apache.phoenix.query.KeyRange;
 import org.apache.phoenix.schema.ColumnAlreadyExistsException;
 import org.apache.phoenix.schema.ReadOnlyTableException;
 import org.apache.phoenix.schema.TableNotFoundException;
+import org.apache.phoenix.util.PhoenixRuntime;
 import org.apache.phoenix.util.QueryUtil;
 import org.junit.Test;
 
@@ -572,6 +575,27 @@ public void testViewAddsNotNullPKColumn() throws Exception {
         }
     }
 
+    @Test
+    public void testQueryViewStatementOptimization() throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        String sql = "CREATE TABLE tp (k1 INTEGER NOT NULL, k2 INTEGER NOT NULL, v1 DECIMAL, CONSTRAINT pk PRIMARY KEY (k1, k2))";
+        conn.createStatement().execute(sql);
+        sql = "CREATE VIEW v1  AS SELECT * FROM tp";
+        conn.createStatement().execute(sql);
+        sql = "CREATE VIEW v2  AS SELECT * FROM tp WHERE k1 = 1.0";
+        conn.createStatement().execute(sql);
+
+        sql = "SELECT * FROM v1 order by k1, k2";
+        PreparedStatement stmt = conn.prepareStatement(sql);
+        QueryPlan plan = PhoenixRuntime.getOptimizedQueryPlan(stmt);
+        assertEquals(0, plan.getOrderBy().getOrderByExpressions().size());
+
+        sql = "SELECT * FROM v2 order by k1, k2";
+        stmt = conn.prepareStatement(sql);
+        plan = PhoenixRuntime.getOptimizedQueryPlan(stmt);
+        assertEquals(0, plan.getOrderBy().getOrderByExpressions().size());
+    }
+
     private void assertPKs(ResultSet rs, String[] expectedPKs) throws SQLException {
         List<String> pkCols = newArrayListWithExpectedSize(expectedPKs.length);
         while (rs.next()) {

phoenix-core/src/main/java/org/apache/phoenix/compile/OrderPreservingTracker.java

@@ -65,6 +65,7 @@ public Info(Info info, int slotSpan, OrderPreserving orderPreserving) {
             this.orderPreserving = orderPreserving;
         }
     }
+    private final StatementContext context;
     private final TrackOrderPreservingExpressionVisitor visitor;
     private final GroupBy groupBy;
     private final Ordering ordering;
@@ -78,6 +79,7 @@ public OrderPreservingTracker(StatementContext context, GroupBy groupBy, Orderin
     }
 
     public OrderPreservingTracker(StatementContext context, GroupBy groupBy, Ordering ordering, int nNodes, TupleProjector projector) {
+        this.context = context;
         int pkPositionOffset = 0;
         PTable table = context.getResolver().getTables().get(0).getTable();
         isOrderPreserving = table.rowKeyOrderOptimizable();
@@ -156,7 +158,12 @@ public boolean isOrderPreserving() {
             Collections.sort(orderPreservingInfos, new Comparator<Info>() {
                 @Override
                 public int compare(Info o1, Info o2) {
-                    return o1.pkPosition-o2.pkPosition;
+                    int cmp = o1.pkPosition-o2.pkPosition;
+                    if (cmp != 0) return cmp;
+                    // After pk position, sort on reverse OrderPreserving ordinal: NO, YES_IF_LAST, YES
+                    // In this way, if we have an ORDER BY over a YES_IF_LAST followed by a YES, we'll
+                    // allow subsequent columns to be ordered.
+                    return o2.orderPreserving.ordinal() - o1.orderPreserving.ordinal();
                 }
             });
         }
@@ -169,14 +176,24 @@ public int compare(Info o1, Info o2) {
         for (int i = 0; i < orderPreservingInfos.size() && isOrderPreserving; i++) {
             Info entry = orderPreservingInfos.get(i);
             int pos = entry.pkPosition;
-            isOrderPreserving &= (entry.orderPreserving != OrderPreserving.NO) && (pos == prevPos || ((pos - prevSlotSpan == prevPos) && (prevOrderPreserving == OrderPreserving.YES)));
+            isOrderPreserving &= entry.orderPreserving != OrderPreserving.NO && prevOrderPreserving == OrderPreserving.YES && (pos == prevPos || pos - prevSlotSpan == prevPos  || hasEqualityConstraints(prevPos+prevSlotSpan, pos));
             prevPos = pos;
             prevSlotSpan = entry.slotSpan;
             prevOrderPreserving = entry.orderPreserving;
         }
         return isOrderPreserving;
     }
 
+    private boolean hasEqualityConstraints(int startPos, int endPos) {
+        ScanRanges ranges = context.getScanRanges();
+        for (int pos = startPos; pos < endPos; pos++) {
+            if (!ranges.hasEqualityConstraint(pos)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     public boolean isReverse() {
         return Boolean.TRUE.equals(isReverse);
     }