kadirozde commented on code in PR #1701:
URL: https://github.com/apache/phoenix/pull/1701#discussion_r1375083564
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PartialIndexIT.java:
##########
@@ -0,0 +1,479 @@
+/*
Review Comment:
Done
##########
phoenix-core/src/main/java/org/apache/phoenix/compile/WhereCompiler.java:
##########
@@ -364,7 +401,495 @@ public Void visit(KeyValueColumnExpression expression) {
ScanUtil.andFilterAtBeginning(scan,
scanRanges.getSkipScanFilter());
}
}
-
+
+
+ public static Expression transformDNF(ParseNode where, StatementContext
statementContext)
+ throws SQLException {
+ if (where == null) {
+ return null;
+ }
+ StatementContext context = new StatementContext(statementContext);
+
context.setResolver(FromCompiler.getResolver(context.getCurrentTable()));
+ Expression expression = where.accept(new
WhereExpressionCompiler(context));
+ Expression dnf = expression.accept(new DNFExpressionRewriter());
+ return dnf;
+ }
+
+ /**
+ * Rewrites an expression in DNF (Disjunctive Normal Form). To do that
+ * (1) it transforms operators like RVC, IN, and BETWEEN to their AND/OR
equivalents,
+ * (2) eliminate double negations and apply DeMorgan rule, i.e.,
+ * NOT (A AND B) = NOT A OR NOT B and NOT (A OR B) = NOT A AND NOT
B, and
+ * (3) distributes AND over OR, i.e.,
+ * (A OR B) AND (C OR D) = (A AND C) OR (A AND D) OR (B AND C) OR (B
AND D).
+ */
+ public static class DNFExpressionRewriter extends
TraverseAllExpressionVisitor<Expression> {
+ /**
+ * Flattens nested AND expressions.
+ * For example A > 10 AND (B = 10 AND C > 0) is an AndExpression with
two children that are
+ * A > 10 and (B = 10 AND C > 0). Note the second child is another
AndExpression. This is
+ * flattened as an AndExpression ( A > 10 AND B = 10 AND C > 0) with
three
+ * children that are A > 10, B = 10, and C > 0.
+ *
+ */
+
+ private static AndExpression flattenAnd(List<Expression> l) {
+ for (Expression e : l) {
+ if (e instanceof AndExpression) {
+ List <Expression> flattenedList = new ArrayList<>(l.size()
+ + e.getChildren().size());
+ for (Expression child : l) {
+ if (child instanceof AndExpression) {
+ flattenedList.addAll(child.getChildren());
+ } else {
+ flattenedList.add(child);
+ }
+ }
+ return new AndExpression(flattenedList);
+ }
+ }
+ return new AndExpression(l);
+ }
+
+ /**
+ * Flattens nested OR expressions.
+ * For example A > 10 OR (B = 10 OR C > 0) is an OrExpression with two
children that are
+ * A > 10 and (B = 10 OR C > 0). Note the second child is another
OrExpression. This is
+ * flattened as an OrExpression ( A > 10 OR B = 10 OR C > 0) with
three
+ * children that are A > 10, B = 10, and C > 0.
+ *
+ */
+ private static OrExpression flattenOr(List<Expression> l) {
+ for (Expression e : l) {
+ if (e instanceof OrExpression) {
+ List <Expression> flattenedList = new ArrayList<>(l.size()
+ + e.getChildren().size());
+ for (Expression child : l) {
+ if (child instanceof OrExpression) {
+ flattenedList.addAll(child.getChildren());
+ } else {
+ flattenedList.add(child);
+ }
+ }
+ return new OrExpression(flattenedList);
+ }
+ }
+ return new OrExpression(l);
+ }
+
+ /**
+ * Flattens nested AND expressions and then distributes AND over OR.
+ *
+ */
+ @Override public Expression visitLeave(AndExpression node,
List<Expression> l) {
+ AndExpression andExpression = flattenAnd(l);
+
+ boolean foundOrChild = false;
+ int i;
+ Expression child = null;
+ List <Expression> andChildren = andExpression.getChildren();
+ for (i = 0; i < andChildren.size(); i++) {
+ child = andChildren.get(i);
+ if (child instanceof OrExpression) {
+ foundOrChild = true;
+ break;
+ }
+ }
+
+ if (foundOrChild) {
+ List <Expression> flattenedList = new
ArrayList<>(andChildren.size() - 1);
+ for (int j = 0; j < andChildren.size(); j++) {
+ if (i != j) {
+ flattenedList.add(andChildren.get(j));
+ }
+ }
+ List <Expression> orList = new
ArrayList<>(child.getChildren().size());
+ for (Expression grandChild : child.getChildren()) {
+ List <Expression> andList = new ArrayList<>(l.size());
+ andList.addAll(flattenedList);
+ andList.add(grandChild);
+ orList.add(visitLeave(new AndExpression(andList),
andList));
+ }
+ return visitLeave(new OrExpression(orList), orList);
+ }
+ return andExpression;
+ }
+ @Override public Expression visitLeave(OrExpression node,
List<Expression> l) {
+ return flattenOr(l);
+ }
+
+ @Override public Expression visitLeave(ScalarFunction node,
List<Expression> l) {
+ return node;
+ }
+
+ private static ComparisonExpression
createComparisonExpression(CompareOperator op, Expression lhs, Expression rhs) {
+ List <Expression> children = new ArrayList<>(2);
+ children.add(lhs);
+ children.add(rhs);
+ return new ComparisonExpression(children, op);
+ }
+ @Override public Expression visitLeave(ComparisonExpression node,
List<Expression> l) {
+ if (l == null || l.isEmpty()) {
+ return node;
+ }
+ Expression lhs = l.get(0);
+ Expression rhs = l.get(1);
+ if (!(lhs instanceof RowValueConstructorExpression) ||
+ !(rhs instanceof RowValueConstructorExpression)) {
+ return new ComparisonExpression(l, node.getFilterOp());
+ }
+
+ // Rewrite RVC in DNF (Disjunctive Normal Form)
+ // For example
+ // (A, B, C ) op (a, b, c) where op is == or != equals to
+ // (A != a and B != b and C != c)
+ // (A, B, C ) op (a, b, c) where op is <, <=, >, or >= is equals to
+ // (A == a and B == b and C op c) or (A == a and B op b) or A op c
+
+ int childCount = lhs.getChildren().size();
+ if (node.getFilterOp() == EQUAL ||
+ node.getFilterOp() == NOT_EQUAL) {
+ List <Expression> andList = new ArrayList<>(childCount);
+ for (int i = 0; i < childCount; i++) {
+ andList.add(createComparisonExpression(node.getFilterOp(),
lhs.getChildren().get(i),
+ rhs.getChildren().get(i)));
+ }
+ return new AndExpression(andList);
+ }
+ List <Expression> orList = new ArrayList<>(childCount);
+ for (int i = 0; i < childCount; i++) {
+ List <Expression> andList = new ArrayList<>(childCount);
+ int j;
+ for (j = 0; j < childCount - i - 1; j++) {
+ andList.add(createComparisonExpression(EQUAL,
lhs.getChildren().get(j),
+ rhs.getChildren().get(j)));
+ }
+ andList.add(createComparisonExpression(node.getFilterOp(),
lhs.getChildren().get(j),
+ rhs.getChildren().get(j)));
+ orList.add(new AndExpression(andList));
+ }
+ return new OrExpression(orList);
+ }
+
+ @Override public Expression visitLeave(LikeExpression node,
List<Expression> l) {
+ return node;
+ }
+
+ @Override public Expression visitLeave(SingleAggregateFunction node,
List<Expression> l) {
+ return node;
+ }
+
+ @Override public Expression visitLeave(CaseExpression node,
List<Expression> l) {
+ return node;
+ }
+
+ private static Expression negate(ComparisonExpression node) {
+ CompareOperator op = node.getFilterOp();
+ Expression lhs = node.getChildren().get(0);
+ Expression rhs = node.getChildren().get(1);
+ switch (op){
+ case LESS:
+ return createComparisonExpression(GREATER_OR_EQUAL, lhs, rhs);
+ case LESS_OR_EQUAL:
+ return createComparisonExpression(GREATER, lhs, rhs);
+ case EQUAL:
+ return createComparisonExpression(NOT_EQUAL, lhs, rhs);
+ case NOT_EQUAL:
+ return createComparisonExpression(EQUAL, lhs, rhs);
+ case GREATER_OR_EQUAL:
+ return createComparisonExpression(LESS, lhs, rhs);
+ case GREATER:
+ return createComparisonExpression(LESS_OR_EQUAL, lhs, rhs);
+ default:
+ throw new IllegalArgumentException("Unexpected CompareOp of "
+ op);
+ }
+ }
+ private static List<Expression> negateChildren(List<Expression>
children) {
+ List <Expression> list = new ArrayList<>(children.size());
+ for (Expression child : children) {
+ if (child instanceof ComparisonExpression) {
+ list.add(negate((ComparisonExpression) child));
+ } else if (child instanceof OrExpression) {
+ list.add(negate((OrExpression) child));
+ } else if (child instanceof AndExpression) {
+ list.add(negate((AndExpression) child));
+ } else if (child instanceof ColumnExpression) {
+ list.add(new NotExpression(child));
+ } else if (child instanceof NotExpression) {
+ list.add(child.getChildren().get(0));
+ } else {
+ throw new IllegalArgumentException("Unexpected Instance of
" + child);
+ }
+ }
+ return list;
+ }
+ private static Expression negate(OrExpression node) {
+ return new AndExpression(negateChildren(node.getChildren()));
+ }
+
+ private static Expression negate(AndExpression node) {
+ return new OrExpression(negateChildren(node.getChildren()));
+ }
+ @Override public Expression visitLeave(NotExpression node,
List<Expression> l) {
+ Expression child = l.get(0);
+ if (child instanceof OrExpression) {
+ return negate((OrExpression) child);
+ } else if (child instanceof AndExpression) {
+ return negate((AndExpression) child);
+ } else if (child instanceof ComparisonExpression) {
+ return negate((ComparisonExpression) child);
+ } else if (child instanceof NotExpression) {
+ return child.getChildren().get(0);
+ } else if (child instanceof IsNullExpression) {
+ return new
IsNullExpression(ImmutableList.of(l.get(0).getChildren().get(0)),
+ !((IsNullExpression) child).isNegate());
+ } else {
+ return new NotExpression(child);
+ }
+ }
+
+ private Expression transformInList(InListExpression node, boolean
negate, List<Expression> l) {
+ List<Expression> list = new
ArrayList<>(node.getKeyExpressions().size());
+ for (Expression element : node.getKeyExpressions()) {
+ if (negate) {
+ list.add(createComparisonExpression(NOT_EQUAL, l.get(0),
element));
Review Comment:
It does handle. Please see the new test for this in WhereComplierTest.
##########
phoenix-core/src/main/java/org/apache/phoenix/hbase/index/IndexRegionObserver.java:
##########
@@ -917,7 +954,8 @@ private void prepareIndexMutations(BatchMutateContext
context, List<IndexMaintai
for (Pair<IndexMaintainer, HTableInterfaceReference> pair :
indexTables) {
IndexMaintainer indexMaintainer = pair.getFirst();
HTableInterfaceReference hTableInterfaceReference =
pair.getSecond();
- if (nextDataRowState != null) {
+ if (nextDataRowState != null &&
Review Comment:
Done
##########
phoenix-core/src/main/java/org/apache/phoenix/expression/ComparisonExpression.java:
##########
@@ -352,7 +356,139 @@ public boolean evaluate(Tuple tuple,
ImmutableBytesWritable ptr) {
ptr.set(ByteUtil.compare(op, comparisonResult) ? PDataType.TRUE_BYTES
: PDataType.FALSE_BYTES);
return true;
}
-
+
+ @Override
+ public boolean contains (Expression other) {
+ if (!(other instanceof ComparisonExpression || other instanceof
IsNullExpression)) {
+ return false;
+ }
+ if (other instanceof IsNullExpression) {
+ return !((IsNullExpression) other).isNegate();
+ }
+
+ BaseTerminalExpression lhsA =
WhereCompiler.getBaseTerminalExpression(this.getChildren().get(0));
+ BaseTerminalExpression lhsB =
WhereCompiler.getBaseTerminalExpression(other.getChildren().get(0));
+ if (!lhsA.equals(lhsB)) {
+ return false;
+ }
+ CompareOperator opA = this.getFilterOp();
+ CompareOperator opB = ((ComparisonExpression) other).getFilterOp();
+ BaseTerminalExpression rhs = WhereCompiler.getBaseTerminalExpression(
+ this.getChildren().get(1));
+ if (rhs instanceof ColumnExpression) {
+ BaseTerminalExpression rhsB =
WhereCompiler.getBaseTerminalExpression(
+ other.getChildren().get(1));
+ if (!rhs.equals(rhsB)) {
+ return false;
+ }
+ switch (opA) {
Review Comment:
Done
##########
phoenix-core/src/test/java/org/apache/phoenix/compile/WhereCompilerTest.java:
##########
@@ -1026,4 +1029,78 @@ public void testScanCaching_CustomFetchSizeOnStatement()
throws SQLException {
assertEquals(FETCH_SIZE, pstmt.getFetchSize());
assertEquals(FETCH_SIZE, scan.getCaching());
}
+ private Expression getDNF(PhoenixConnection pconn, String query) throws
SQLException {
+ //SQLParser parser = new SQLParser("where ID = 'i1' or (ID = 'i2' and
A > 1)");
+ // ParseNode where = parser.parseWhereClause()
+ PhoenixPreparedStatement pstmt = newPreparedStatement(pconn, query);
+ QueryPlan plan = pstmt.compileQuery();
+ ParseNode where = plan.getStatement().getWhere();
+
+ return transformDNF(where, plan.getContext());
+ }
+ @Test
+ public void testWhereInclusion() throws SQLException {
+ PhoenixConnection pconn = DriverManager.getConnection(getUrl(),
+
PropertiesUtil.deepCopy(TEST_PROPERTIES)).unwrap(PhoenixConnection.class);
+ String ddl = "create table myTable(ID varchar primary key, A integer,
B varchar, " +
+ "C date, D double, E integer)";
+ pconn.createStatement().execute(ddl);
+ ddl = "create table myTableDesc(ID varchar primary key DESC, A
integer, B varchar, " +
+ "C date, D double, E integer)";
+ pconn.createStatement().execute(ddl);
+
+ final int NUM = 12;
+ String[] containingQueries = new String[NUM];
+ String[] containedQueries = new String[NUM];
+
+ containingQueries[0] = "select * from myTable where ID = 'i1' or (ID =
'i2' and A > 1)";
+ containedQueries[0] = "select * from myTableDesc where ID = 'i1' or
(ID = 'i2' and " +
+ "A > 2 + 2)";
+
+ containingQueries[1] = "select * from myTable where ID > 'i3' and A >
1";
+ containedQueries[1] = "select * from myTableDesc where (ID > 'i7' or
ID = 'i4') and " +
+ "A > 2 * 10";
+
+ containingQueries[2] = "select * from myTable where ID IN ('i3', 'i7',
'i1') and A < 10";
+ containedQueries[2] = "select * from myTableDesc where ID IN ('i1',
'i7') and A < 10 / 2";
Review Comment:
Done
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/PartialIndexIT.java:
##########
@@ -0,0 +1,564 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.end2end.index;
+
+import static org.apache.phoenix.compile.WhereCompiler.transformDNF;
+import static
org.apache.phoenix.end2end.index.GlobalIndexCheckerIT.assertExplainPlan;
+import static
org.apache.phoenix.end2end.index.GlobalIndexCheckerIT.assertExplainPlanWithLimit;
+import static org.apache.phoenix.mapreduce.index.PhoenixIndexToolJobCounters.*;
+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.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Timestamp;
+import java.util.Arrays;
+import java.util.Calendar;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.apache.hadoop.hbase.TableName;
+import org.apache.hadoop.hbase.client.Scan;
+import org.apache.hadoop.hbase.coprocessor.ObserverContext;
+import org.apache.hadoop.hbase.coprocessor.RegionCoprocessorEnvironment;
+import org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver;
+import org.apache.hadoop.mapreduce.CounterGroup;
+import org.apache.phoenix.compile.FromCompiler;
+import org.apache.phoenix.compile.QueryPlan;
+import org.apache.phoenix.end2end.IndexToolIT;
+import org.apache.phoenix.exception.PhoenixParserException;
+import org.apache.phoenix.expression.Expression;
+import org.apache.phoenix.filter.SkipScanFilter;
+import org.apache.phoenix.hbase.index.IndexRegionObserver;
+import org.apache.phoenix.jdbc.PhoenixConnection;
+import org.apache.phoenix.jdbc.PhoenixPreparedStatement;
+import org.apache.phoenix.jdbc.PhoenixResultSet;
+import org.apache.phoenix.mapreduce.index.IndexTool;
+import org.apache.phoenix.parse.ParseNode;
+import org.apache.phoenix.query.KeyRange;
+import org.apache.phoenix.schema.ColumnFamilyNotFoundException;
+import org.apache.phoenix.schema.ColumnNotFoundException;
+import org.apache.phoenix.schema.PTable;
+import org.apache.phoenix.thirdparty.com.google.common.collect.Maps;
+import org.apache.phoenix.end2end.NeedsOwnMiniClusterTest;
+import org.apache.phoenix.query.BaseTest;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.util.*;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+@Category(NeedsOwnMiniClusterTest.class)
+@RunWith(Parameterized.class)
+public class PartialIndexIT extends BaseTest {
+ private final boolean local;
+ private final boolean uncovered;
+ private final boolean salted;
+
+ public PartialIndexIT (boolean local, boolean uncovered, boolean salted) {
+ this.local = local;
+ this.uncovered = uncovered;
+ this.salted = salted;
+ }
+ @BeforeClass
+ public static synchronized void doSetup() throws Exception {
+ Map<String, String> props = Maps.newHashMapWithExpectedSize(1);
+
props.put(QueryServices.GLOBAL_INDEX_ROW_AGE_THRESHOLD_TO_DELETE_MS_ATTRIB,
Long.toString(0));
+ setUpTestDriver(new ReadOnlyProps(props.entrySet().iterator()));
+ }
+
+ @After
+ public void unsetFailForTesting() throws Exception {
+ boolean refCountLeaked = isAnyStoreRefCountLeaked();
+ assertFalse("refCount leaked", refCountLeaked);
+ }
+ @Parameterized.Parameters(
+ name = "local={0}, uncovered={1}, salted={2}")
+ public static synchronized Collection<Boolean[]> data() {
+ return Arrays.asList(new Boolean[][] {
+ // Partial local indexes are not supported currently.
+ {false, false, true},
+ {false, false, false},
+ {false, true, false},
+ {false, true, true}
+ });
+ }
+
+ public static void assertPlan(PhoenixResultSet rs, String schemaName,
String tableName) {
+ PTable table = rs.getContext().getCurrentTable().getTable();
+ assertTrue(table.getSchemaName().getString().equals(schemaName) &&
+ table.getTableName().getString().equals(tableName));
+ }
+
+ private static void verifyIndex(String dataTableName, String
indexTableName) throws Exception {
+ IndexTool indexTool = IndexToolIT.runIndexTool(false, "",
dataTableName,
+ indexTableName, null, 0, IndexTool.IndexVerifyType.ONLY);
+
+ assertEquals(0, indexTool.getJob().getCounters().
+ findCounter(REBUILT_INDEX_ROW_COUNT).getValue());
+ assertEquals(0, indexTool.getJob().getCounters().
+
findCounter(BEFORE_REBUILD_INVALID_INDEX_ROW_COUNT).getValue());
+ assertEquals(0, indexTool.getJob().getCounters().
+
findCounter(BEFORE_REBUILD_MISSING_INDEX_ROW_COUNT).getValue());
+ assertEquals(0, indexTool.getJob().getCounters().
+
findCounter(BEFORE_REBUILD_BEYOND_MAXLOOKBACK_MISSING_INDEX_ROW_COUNT).getValue());
+ assertEquals(0, indexTool.getJob().getCounters().
+
findCounter(BEFORE_REBUILD_BEYOND_MAXLOOKBACK_INVALID_INDEX_ROW_COUNT).getValue());
+ assertEquals(0, indexTool.getJob().getCounters().
+ findCounter(BEFORE_REBUILD_OLD_INDEX_ROW_COUNT).getValue());
+ assertEquals(0, indexTool.getJob().getCounters().
+
findCounter(BEFORE_REBUILD_UNKNOWN_INDEX_ROW_COUNT).getValue());
+
+ IndexToolIT.runIndexTool(false, "", dataTableName,
+ indexTableName, null, 0, IndexTool.IndexVerifyType.ONLY,
"-fi");
+ CounterGroup mrJobCounters = IndexToolIT.getMRJobCounters(indexTool);
+ assertEquals(0,
+
mrJobCounters.findCounter(BEFORE_REBUILD_INVALID_INDEX_ROW_COUNT.name()).getValue());
+ assertEquals(0,
+
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_VERIFIED_INDEX_ROW_COUNT.name()).getValue());
+ assertEquals(0,
+
mrJobCounters.findCounter(BEFORE_REPAIR_EXTRA_UNVERIFIED_INDEX_ROW_COUNT.name()).getValue());
+ }
+
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
