diff --git ql/src/java/org/apache/hadoop/hive/ql/io/sarg/SearchArgumentImpl.java ql/src/java/org/apache/hadoop/hive/ql/io/sarg/SearchArgumentImpl.java index e692ec5186..09bb8412a3 100644 --- ql/src/java/org/apache/hadoop/hive/ql/io/sarg/SearchArgumentImpl.java +++ ql/src/java/org/apache/hadoop/hive/ql/io/sarg/SearchArgumentImpl.java @@ -778,7 +778,10 @@ private static void generateAllCombinations(List result, for(ExpressionTree kid: kids) { for(ExpressionTree or: work) { ExpressionTree copy = new ExpressionTree(or); - copy.children.add(kid); + ExpressionTree kidCopy = new ExpressionTree(kid); + // If we don't use a copy, a non-leaf node will be modified + // more than once in `buildLeafList`, which results in a wrong SARG + copy.children.add(kidCopy); result.add(copy); } } diff --git ql/src/test/org/apache/hadoop/hive/ql/io/sarg/TestSearchArgumentImpl.java ql/src/test/org/apache/hadoop/hive/ql/io/sarg/TestSearchArgumentImpl.java index 576e677092..9c45f78cfa 100644 --- ql/src/test/org/apache/hadoop/hive/ql/io/sarg/TestSearchArgumentImpl.java +++ ql/src/test/org/apache/hadoop/hive/ql/io/sarg/TestSearchArgumentImpl.java @@ -2900,6 +2900,271 @@ public void testExpression10() throws Exception { assertEquals(TruthValue.YES_NO_NULL, sarg.evaluate(values(TruthValue.YES_NO_NULL))); } + @Test + public void testExpression11() throws Exception { + /* ((rfr_page = 'search') and uid is not null) or ((rfr_page = 'index') and uid is not null) */ + String exprStr = " \n" + + "\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " rfr_page\n" + + " \n" + + " \n" + + " orc_people\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " string\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " search\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " boolean\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " uid\n" + + " \n" + + " \n" + + " orc_people\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " string\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " boolean\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " boolean\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " rfr_page\n" + + " \n" + + " \n" + + " orc_people\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " string\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " index\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " boolean\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " uid\n" + + " \n" + + " \n" + + " orc_people\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " string\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " boolean\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " boolean\n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + " \n" + + ""; + + SearchArgumentImpl sarg = + (SearchArgumentImpl) SearchArgumentFactory.create(getFuncDesc(exprStr)); + List leaves = sarg.getLeaves(); + assertEquals(3, leaves.size()); + + FilterPredicate p = sarg.toFilterPredicate(); + String[] conditions = new String[]{ + "eq(rfr_page, Binary{\"search\"})", /* rfr_page = 'search' */ + "eq(rfr_page, Binary{\"index\"})", /* rfr_page = 'index' */ + "not(eq(uid, null))", /* uid is not null */ + }; + String expected = String + .format("and(and(and(or(%1$s, %2$s), or(%3$s, %2$s)), or(%1$s, %3$s)), or(%3$s, %3$s))", conditions); + assertEquals(expected, p.toString()); + + + assertEquals(PredicateLeaf.Type.STRING, leaves.get(0).getType()); + assertEquals(PredicateLeaf.Operator.EQUALS, + leaves.get(0).getOperator()); + assertEquals("rfr_page", leaves.get(0).getColumnName()); + assertEquals("search", leaves.get(0).getLiteral()); + + assertEquals(PredicateLeaf.Type.STRING, leaves.get(1).getType()); + assertEquals(PredicateLeaf.Operator.EQUALS, + leaves.get(1).getOperator()); + assertEquals("rfr_page", leaves.get(1).getColumnName()); + assertEquals("index", leaves.get(1).getLiteral()); + + assertEquals(PredicateLeaf.Type.STRING, leaves.get(2).getType()); + assertEquals(PredicateLeaf.Operator.IS_NULL, + leaves.get(2).getOperator()); + assertEquals("uid", leaves.get(2).getColumnName()); + + assertEquals("(and (or leaf-0 leaf-1) (or (not leaf-2) leaf-1) " + + "(or leaf-0 (not leaf-2)) (or (not leaf-2) (not leaf-2)))", + sarg.getExpression().toString()); + assertNoSharedNodes(sarg.getExpression(), + Sets.newIdentityHashSet()); + } + private static TruthValue[] values(TruthValue... vals) { return vals; }