You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ja...@apache.org on 2016/10/25 23:42:55 UTC

[3/4] phoenix git commit: PHOENIX-3328 Optimize ORed leading pk column range comparisions to SkipScan (William Yang)

PHOENIX-3328 Optimize ORed leading pk column range comparisions to SkipScan (William Yang)


Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/6027ee8e
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/6027ee8e
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/6027ee8e

Branch: refs/heads/4.x-HBase-0.98
Commit: 6027ee8efe53658ba3fd3e234876e2e6f89b8468
Parents: c40fa01
Author: James Taylor <ja...@apache.org>
Authored: Sat Oct 15 15:19:47 2016 -0700
Committer: James Taylor <ja...@apache.org>
Committed: Tue Oct 25 16:41:01 2016 -0700

----------------------------------------------------------------------
 .../apache/phoenix/compile/WhereOptimizer.java  | 23 ++++++++-
 .../phoenix/compile/WhereOptimizerTest.java     | 53 ++++++++++++++++++++
 .../java/org/apache/phoenix/util/TestUtil.java  |  7 +++
 3 files changed, 81 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/6027ee8e/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java b/phoenix-core/src/main/java/org/apache/phoenix/compile/WhereOptimizer.java
index f49aa52..f15a251 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
@@ -750,12 +750,31 @@ public class WhereOptimizer {
                     	}
                     }
                 } else {
+                    boolean hasFirstSlot = true;
+                    boolean prevIsNull = false;
                     // TODO: Do the same optimization that we do for IN if the childSlots specify a fully qualified row key
                     for (KeySlot slot : childSlot) {
-                        // We have a nested OR with nothing for this slot, so continue
+                        if (hasFirstSlot) {
+                            // if the first slot is null, return null immediately
+                            if (slot == null) {
+                                return null;
+                            }
+                            // mark that we've handled the first slot
+                            hasFirstSlot = false;
+                        }
+
+                        // now if current slot is the first one, it must not be null
+                        // if not the first, then it might be null, so check if all the rest are null
                         if (slot == null) {
-                            return null; //If one childSlot does not have the PK columns, let Phoenix scan all the key ranges of the table. 
+                            prevIsNull = true;
+                            continue;
+                        } else {
+                            // current slot is not null but prev one is null, cannot OR these together (PHOENIX-3328)
+                            if (prevIsNull) {
+                                return null;
+                            }
                         }
+
                         /*
                          * If we see a different PK column than before, we can't
                          * optimize it because our SkipScanFilter only handles

http://git-wip-us.apache.org/repos/asf/phoenix/blob/6027ee8e/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java
index 8b668a1..34beb59 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereOptimizerTest.java
@@ -19,6 +19,7 @@ package org.apache.phoenix.compile;
 
 import static org.apache.phoenix.query.QueryConstants.MILLIS_IN_DAY;
 import static org.apache.phoenix.util.TestUtil.BINARY_NAME;
+import static org.apache.phoenix.util.TestUtil.BTABLE_NAME;
 import static org.apache.phoenix.util.TestUtil.TEST_PROPERTIES;
 import static org.apache.phoenix.util.TestUtil.assertDegenerate;
 import static org.apache.phoenix.util.TestUtil.assertEmptyScanKey;
@@ -28,6 +29,7 @@ import static org.apache.phoenix.util.TestUtil.rowKeyFilter;
 import static org.apache.phoenix.util.TestUtil.substr;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
@@ -38,6 +40,7 @@ import java.sql.Connection;
 import java.sql.Date;
 import java.sql.DriverManager;
 import java.sql.SQLException;
+import java.sql.Statement;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Iterator;
@@ -70,6 +73,7 @@ import org.apache.phoenix.util.ByteUtil;
 import org.apache.phoenix.util.DateUtil;
 import org.apache.phoenix.util.PhoenixRuntime;
 import org.apache.phoenix.util.PropertiesUtil;
+import org.apache.phoenix.util.ScanUtil;
 import org.apache.phoenix.util.StringUtil;
 import org.apache.phoenix.util.TestUtil;
 import org.junit.Test;
@@ -1216,6 +1220,55 @@ public class WhereOptimizerTest extends BaseConnectionlessQueryTest {
                         StringUtil.padChar(ByteUtil.nextKey(PChar.INSTANCE.toBytes("foo")),15), false)));
         assertEquals(expectedRanges, ranges);
     }
+
+    @Test
+    public void testOrPKRanges() throws SQLException {
+        Connection conn = DriverManager.getConnection(getUrl());
+        ensureTableCreated(getUrl(), TestUtil.BTABLE_NAME);
+        Statement stmt = conn.createStatement();
+        // BTABLE has 5 PK columns
+        String query = "select * from " + BTABLE_NAME +
+                       " where (a_string > '1' and a_string < '5') or (a_string > '6' and a_string < '9')";
+        StatementContext context = compileStatement(query);
+        Filter filter = context.getScan().getFilter();
+
+        assertNotNull(filter);
+        assertTrue(filter instanceof SkipScanFilter);
+        ScanRanges scanRanges = context.getScanRanges();
+        assertNotNull(scanRanges);
+        List<List<KeyRange>> ranges = scanRanges.getRanges();
+        assertEquals(1, ranges.size());
+        List<List<KeyRange>> expectedRanges = Collections.singletonList(Arrays.asList(
+                KeyRange.getKeyRange(Bytes.toBytes("1"), false, Bytes.toBytes("5"), false),
+                KeyRange.getKeyRange(Bytes.toBytes("6"), false, Bytes.toBytes("9"), false)));
+        assertEquals(expectedRanges, ranges);
+
+        stmt.close();
+        conn.close();
+    }
+    
+    @Test
+    public void testOrPKRangesNotOptimized() throws SQLException {
+        Connection conn = DriverManager.getConnection(getUrl());
+        ensureTableCreated(getUrl(), TestUtil.BTABLE_NAME);
+        Statement stmt = conn.createStatement();
+        // BTABLE has 5 PK columns
+        String[] queries = {
+                "select * from " + BTABLE_NAME + " where (a_string > '1' and a_string < '5') or (a_string > '6' and a_string < '9' and a_id = 'foo')",
+                "select * from " + BTABLE_NAME + " where (a_id > 'aaa' and a_id < 'ccc') or (a_id > 'jjj' and a_id < 'mmm')",
+                };
+        for (String query : queries) {
+            StatementContext context = compileStatement(query);
+            Iterator<Filter> it = ScanUtil.getFilterIterator(context.getScan());
+            while (it.hasNext()) {
+                assertFalse(it.next() instanceof SkipScanFilter);
+            }
+            TestUtil.assertNotDegenerate(context.getScan());
+        }
+
+        stmt.close();
+        conn.close();
+    }
     
     @Test
     public void testForceSkipScanOnSaltedTable() throws SQLException {

http://git-wip-us.apache.org/repos/asf/phoenix/blob/6027ee8e/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java b/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java
index 03469e2..5feedb1 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/util/TestUtil.java
@@ -28,6 +28,7 @@ import static org.apache.phoenix.util.PhoenixRuntime.JDBC_PROTOCOL_TERMINATOR;
 import static org.apache.phoenix.util.PhoenixRuntime.PHOENIX_TEST_DRIVER_URL_PARAM;
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -397,6 +398,12 @@ public class TestUtil {
         assertEquals(null,scan.getFilter());
     }
 
+    public static void assertNotDegenerate(Scan scan) {
+        assertFalse(
+                Bytes.compareTo(KeyRange.EMPTY_RANGE.getLowerRange(), scan.getStartRow()) == 0 &&
+                Bytes.compareTo(KeyRange.EMPTY_RANGE.getLowerRange(), scan.getStopRow()) == 0);
+    }
+
     public static void assertEmptyScanKey(Scan scan) {
         assertNull(scan.getFilter());
         assertArrayEquals(ByteUtil.EMPTY_BYTE_ARRAY, scan.getStartRow());