PHOENIX-2559 Keep filter for OR when only some expressions are extracted to stop/stop key
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/a7788daf Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/a7788daf Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/a7788daf Branch: refs/heads/4.x-HBase-1.0 Commit: a7788dafac19cd1ce0bb2d50b972dd9079673b51 Parents: c027e0b Author: James Taylor <[email protected]> Authored: Mon Jan 4 17:29:40 2016 -0800 Committer: James Taylor <[email protected]> Committed: Mon Jan 4 17:32:39 2016 -0800 ---------------------------------------------------------------------- .../apache/phoenix/end2end/SkipScanQueryIT.java | 20 ++++++++ .../apache/phoenix/compile/WhereOptimizer.java | 52 +++++++++++++++---- .../phoenix/compile/WhereCompilerTest.java | 54 +++++++++++++++++++- 3 files changed, 114 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/a7788daf/phoenix-core/src/it/java/org/apache/phoenix/end2end/SkipScanQueryIT.java ---------------------------------------------------------------------- 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 2ade0a4..9f73550 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 @@ -414,4 +414,24 @@ public class SkipScanQueryIT extends BaseHBaseManagedTimeIT { conn.close(); } } + + @Test + public void testOrPKWithAndNonPK() throws Exception { + Connection conn = DriverManager.getConnection(getUrl()); + try { + conn.createStatement().execute("create table bugTable(ID varchar primary key,company varchar)"); + conn.setAutoCommit(true); + conn.createStatement().execute("upsert into bugTable values('i1','c1')"); + conn.createStatement().execute("upsert into bugTable values('i2','c2')"); + conn.createStatement().execute("upsert into bugTable values('i3','c3')"); + ResultSet rs = conn.createStatement().executeQuery("select * from bugTable where ID = 'i1' or (ID = 'i2' and company = 'c3')"); + assertTrue(rs.next()); + assertEquals("i1", rs.getString(1)); + assertEquals("c1", rs.getString(2)); + assertFalse(rs.next()); + + } finally { + conn.close(); + } + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/a7788daf/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 99986a2..0122712 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 @@ -456,6 +456,11 @@ public class WhereOptimizer { public KeyRange getMinMaxRange() { return null; } + + @Override + public boolean isPartialExtraction() { + return false; + } }; private static boolean isDegenerate(List<KeyRange> keyRanges) { @@ -643,10 +648,12 @@ public class WhereOptimizer { KeyRange minMaxRange = KeyRange.EVERYTHING_RANGE; List<Expression> minMaxExtractNodes = Lists.<Expression>newArrayList(); int initPosition = (table.getBucketNum() ==null ? 0 : 1) + (this.context.getConnection().getTenantId() != null && table.isMultiTenant() ? 1 : 0) + (table.getViewIndexId() == null ? 0 : 1); + boolean partialExtraction = andExpression.getChildren().size() != childSlots.size(); for (KeySlots childSlot : childSlots) { if (childSlot == EMPTY_KEY_SLOTS) { return EMPTY_KEY_SLOTS; } + partialExtraction |= childSlot.isPartialExtraction(); // FIXME: get rid of this special-cased min/max range now that a key range can span multiple columns if (childSlot.getMinMaxRange() != null) { // Only set if in initial pk position // TODO: fix intersectSlots so that it works with RVCs. We'd just need to fill in the leading parts @@ -688,7 +695,7 @@ public class WhereOptimizer { // If we have a salt column, skip that slot because // they'll never be an expression contained by it. keySlots = keySlots.subList(initPosition, keySlots.size()); - return new MultiKeySlot(keySlots, minMaxRange == KeyRange.EVERYTHING_RANGE ? null : minMaxRange); + return new MultiKeySlot(keySlots, minMaxRange == KeyRange.EVERYTHING_RANGE ? null : minMaxRange, partialExtraction); } private KeySlots orKeySlots(OrExpression orExpression, List<KeySlots> childSlots) { @@ -708,7 +715,7 @@ public class WhereOptimizer { KeySlot theSlot = null; List<Expression> slotExtractNodes = Lists.<Expression>newArrayList(); int thePosition = -1; - boolean extractAll = true; + boolean partialExtraction = false; // TODO: Have separate list for single span versus multi span // For multi-span, we only need to keep a single range. List<KeyRange> slotRanges = Lists.newArrayList(); @@ -718,6 +725,10 @@ public class WhereOptimizer { // TODO: can this ever happen and can we safely filter the expression tree? continue; } + // When we OR together expressions, we can only extract the entire OR expression + // if all sub-expressions have been completely extracted. Otherwise, we must + // leave the OR as a post filter. + partialExtraction |= childSlot.isPartialExtraction(); if (childSlot.getMinMaxRange() != null) { if (!slotRanges.isEmpty() && thePosition != initialPos) { // ORing together rvc in initial slot with other slots return null; @@ -726,9 +737,7 @@ public class WhereOptimizer { thePosition = initialPos; for (KeySlot slot : childSlot) { if (slot != null) { - List<Expression> extractNodes = slot.getKeyPart().getExtractNodes(); - extractAll &= !extractNodes.isEmpty(); - slotExtractNodes.addAll(extractNodes); + slotExtractNodes.addAll(slot.getKeyPart().getExtractNodes()); } } } else { @@ -757,9 +766,7 @@ public class WhereOptimizer { } else if (thePosition != slot.getPKPosition()) { return null; } - List<Expression> extractNodes = slot.getKeyPart().getExtractNodes(); - extractAll &= !extractNodes.isEmpty(); - slotExtractNodes.addAll(extractNodes); + slotExtractNodes.addAll(slot.getKeyPart().getExtractNodes()); slotRanges.addAll(slot.getKeyRanges()); } } @@ -790,7 +797,7 @@ public class WhereOptimizer { minMaxRange = minMaxRange.union(range); } if (clearExtracts) { - extractAll = false; + partialExtraction = true; slotExtractNodes = Collections.emptyList(); } slotRanges = Collections.emptyList(); @@ -802,7 +809,7 @@ public class WhereOptimizer { } return newKeyParts( theSlot, - extractAll ? Collections.<Expression>singletonList(orExpression) : slotExtractNodes, + partialExtraction ? slotExtractNodes : Collections.<Expression>singletonList(orExpression), slotRanges.isEmpty() ? EVERYTHING_RANGES : KeyRange.coalesce(slotRanges), minMaxRange == KeyRange.EMPTY_RANGE ? null : minMaxRange); } @@ -1029,6 +1036,17 @@ public class WhereOptimizer { private static interface KeySlots extends Iterable<KeySlot> { @Override public Iterator<KeySlot> iterator(); public KeyRange getMinMaxRange(); + /** + * Tracks whether or not the contained KeySlot(s) contain + * a slot that includes only a partial extraction of the + * involved expressions. For example: (A AND B) in the case + * of A being a PK column and B being a KV column, the + * KeySlots representing the AND would return true for + * isPartialExtraction. + * @return true if a partial expression extraction was + * done and false otherwise. + */ + public boolean isPartialExtraction(); } private final class KeySlot { @@ -1154,10 +1172,12 @@ public class WhereOptimizer { private static class MultiKeySlot implements KeySlots { private final List<KeySlot> childSlots; private final KeyRange minMaxRange; + private final boolean partialExtraction; - private MultiKeySlot(List<KeySlot> childSlots, KeyRange minMaxRange) { + private MultiKeySlot(List<KeySlot> childSlots, KeyRange minMaxRange, boolean partialExtraction) { this.childSlots = childSlots; this.minMaxRange = minMaxRange; + this.partialExtraction = partialExtraction; } @Override @@ -1169,6 +1189,11 @@ public class WhereOptimizer { public KeyRange getMinMaxRange() { return minMaxRange; } + + @Override + public boolean isPartialExtraction() { + return partialExtraction; + } } private class SingleKeySlot implements KeySlots { @@ -1205,6 +1230,11 @@ public class WhereOptimizer { public KeyRange getMinMaxRange() { return minMaxRange; } + + @Override + public boolean isPartialExtraction() { + return this.slot.getKeyPart().getExtractNodes().isEmpty(); + } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/a7788daf/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereCompilerTest.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereCompilerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereCompilerTest.java index a1d1440..7315ad6 100644 --- a/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereCompilerTest.java +++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/WhereCompilerTest.java @@ -50,8 +50,11 @@ import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp; import org.apache.hadoop.hbase.filter.Filter; +import org.apache.hadoop.hbase.filter.FilterList; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.phoenix.expression.ColumnExpression; import org.apache.phoenix.expression.Expression; +import org.apache.phoenix.expression.KeyValueColumnExpression; import org.apache.phoenix.expression.LiteralExpression; import org.apache.phoenix.expression.RowKeyColumnExpression; import org.apache.phoenix.expression.function.SubstrFunction; @@ -62,7 +65,8 @@ import org.apache.phoenix.jdbc.PhoenixPreparedStatement; import org.apache.phoenix.query.BaseConnectionlessQueryTest; import org.apache.phoenix.query.KeyRange; import org.apache.phoenix.query.QueryConstants; -import org.apache.phoenix.query.QueryServicesOptions; +import org.apache.phoenix.schema.ColumnRef; +import org.apache.phoenix.schema.PColumn; import org.apache.phoenix.schema.RowKeyValueAccessor; import org.apache.phoenix.schema.SaltingUtil; import org.apache.phoenix.schema.types.PChar; @@ -106,6 +110,54 @@ public class WhereCompilerTest extends BaseConnectionlessQueryTest { } @Test + public void testOrPKWithAndPKAndNotPK() throws SQLException { + String query = "select * from bugTable where ID = 'i1' or (ID = 'i2' and company = 'c3')"; + PhoenixConnection pconn = DriverManager.getConnection(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES)).unwrap(PhoenixConnection.class); + pconn.createStatement().execute("create table bugTable(ID varchar primary key,company varchar)"); + PhoenixPreparedStatement pstmt = newPreparedStatement(pconn, query); + QueryPlan plan = pstmt.optimizeQuery(); + Scan scan = plan.getContext().getScan(); + Filter filter = scan.getFilter(); + ColumnExpression idExpression = new ColumnRef(plan.getTableRef(), plan.getTableRef().getTable().getColumn("ID").getPosition()).newColumnExpression(); + Expression id = new RowKeyColumnExpression(idExpression,new RowKeyValueAccessor(plan.getTableRef().getTable().getPKColumns(),0)); + Expression company = new KeyValueColumnExpression(plan.getTableRef().getTable().getColumn("COMPANY")); + // FilterList has no equals implementation + assertTrue(filter instanceof FilterList); + FilterList filterList = (FilterList)filter; + assertEquals(FilterList.Operator.MUST_PASS_ALL, filterList.getOperator()); + assertEquals( + Arrays.asList( + new SkipScanFilter( + ImmutableList.of(Arrays.asList( + pointRange("i1"), + pointRange("i2"))), + SchemaUtil.VAR_BINARY_SCHEMA), + singleKVFilter( + or(constantComparison(CompareOp.EQUAL,id,"i1"), + and(constantComparison(CompareOp.EQUAL,id,"i2"), + constantComparison(CompareOp.EQUAL,company,"c3"))))), + filterList.getFilters()); + } + + @Test + public void testAndPKAndNotPK() throws SQLException { + String query = "select * from bugTable where ID = 'i2' and company = 'c3'"; + PhoenixConnection pconn = DriverManager.getConnection(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES)).unwrap(PhoenixConnection.class); + pconn.createStatement().execute("create table bugTable(ID varchar primary key,company varchar)"); + PhoenixPreparedStatement pstmt = newPreparedStatement(pconn, query); + QueryPlan plan = pstmt.optimizeQuery(); + Scan scan = plan.getContext().getScan(); + Filter filter = scan.getFilter(); + PColumn column = plan.getTableRef().getTable().getColumn("COMPANY"); + assertEquals( + singleKVFilter(constantComparison( + CompareOp.EQUAL, + new KeyValueColumnExpression(column), + "c3")), + filter); + } + + @Test public void testSingleFixedFullPkSalted() throws SQLException { PhoenixConnection pconn = DriverManager.getConnection(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES)).unwrap(PhoenixConnection.class); pconn.createStatement().execute("CREATE TABLE t (k bigint not null primary key, v varchar) SALT_BUCKETS=20");
