----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55553/#review162239 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HivePlannerContext.java (line 30) <https://reviews.apache.org/r/55553/#comment233552> Please add a comment here about purpose of this variable. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java (line 234) <https://reviews.apache.org/r/55553/#comment233553> Any reason for disabling this rule? ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSubQueryRemoveRule.java (lines 95 - 98) <https://reviews.apache.org/r/55553/#comment233554> Can be simplified as isCorrScalarQuery = corrScalarQueries.contains(e.rel); ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSubQueryRemoveRule.java (line 143) <https://reviews.apache.org/r/55553/#comment233572> Comment: Transformation Outer Query Left Join (inner query) on correlated predicate and preserve rows only from left side. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSubQueryRemoveRule.java (line 166) <https://reviews.apache.org/r/55553/#comment233576> Comment: Transformation is to left join for correlated predicates and inner join otherwise, but do a count on inner side before that to make sure it generates exactly 1 row. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSubQueryRemoveRule.java (line 183) <https://reviews.apache.org/r/55553/#comment233577> Can remove this line. ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/RexNodeConverter.java (line 214) <https://reviews.apache.org/r/55553/#comment233555> Add subQueryDesc.getType() in error message? ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java (line 419) <https://reviews.apache.org/r/55553/#comment233556> Add a comment we dont want to retry subqueries on non-cbo path because... ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java (line 2301) <https://reviews.apache.org/r/55553/#comment233578> can remove this. ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java (lines 2302 - 2303) <https://reviews.apache.org/r/55553/#comment233579> Instead of arrays, you may use Boolean & Integer objects, which is better for readability. ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java (line 2326) <https://reviews.apache.org/r/55553/#comment233580> Boolean object can be used here. ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java (line 573) <https://reviews.apache.org/r/55553/#comment233581> I guess you also want to make sure predicate is equality only, ie. conjunctAST.getType == HiveParser.EQUAL ? ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSQCountCheck.java (lines 65 - 67) <https://reviews.apache.org/r/55553/#comment233584> assert valObject != null ? ql/src/test/queries/clientpositive/subquery_in.q <https://reviews.apache.org/r/55553/#comment233585> Instead of removing you may add group by in subquery, unless this query pattern is covered elsewhere in tests. ql/src/test/queries/clientpositive/subquery_in_having.q <https://reviews.apache.org/r/55553/#comment233586> This satisifies all conditions: correlated and no implied gby. Will this still wont work? ql/src/test/queries/clientpositive/subquery_in_having.q (line 120) <https://reviews.apache.org/r/55553/#comment233587> Dont we allow correlated predicates here? ql/src/test/queries/clientpositive/subquery_notin.q <https://reviews.apache.org/r/55553/#comment233588> add gby ? - Ashutosh Chauhan On Jan. 19, 2017, 2:53 a.m., Vineet Garg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55553/ > ----------------------------------------------------------- > > (Updated Jan. 19, 2017, 2:53 a.m.) > > > Review request for hive, Ashutosh Chauhan and Jesús Camacho Rodríguez. > > > Bugs: HIVE-15544 > https://issues.apache.org/jira/browse/HIVE-15544 > > > Repository: hive-git > > > Description > ------- > > This patch provides support for scalar subqueries in filter clause. Note that > this patch disables the following: > * IN/NOT IN correlated subqueries containing aggregates (HIVE checks for such > queries and throw an exception) > * SCALAR correlated subqueries containing aggregates with non-equi join > predicates on correlated columns (HIVE throws an exception for such queries). > > > Diffs > ----- > > itests/src/test/resources/testconfiguration.properties be5a747 > ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 6f01da0 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/CalciteSubquerySemanticException.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/HivePlannerContext.java > 8beb0dd > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRelDecorrelator.java > a373cdd > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveSubQueryRemoveRule.java > f1e8ebd > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/RexNodeConverter.java > 8d2e535 > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 9f1b9d5 > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g cd9adfc > ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java 24381b9 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java f275f6a > ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java > 6c30efd > ql/src/java/org/apache/hadoop/hive/ql/plan/ExprNodeSubQueryDesc.java > aec331b > > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSQCountCheck.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/TestCBORuleFiredOnlyOnce.java > 44e157b > ql/src/test/queries/clientnegative/subquery_exists_implicit_gby.q 9013df6 > ql/src/test/queries/clientnegative/subquery_in_implicit_gby.q PRE-CREATION > ql/src/test/queries/clientnegative/subquery_notexists_implicit_gby.q > 852b295 > ql/src/test/queries/clientnegative/subquery_scalar_implicit_gby.q > PRE-CREATION > ql/src/test/queries/clientnegative/subquery_scalar_multi_columns.q > PRE-CREATION > ql/src/test/queries/clientnegative/subquery_scalar_multi_rows.q > PRE-CREATION > ql/src/test/queries/clientnegative/subquery_with_or_cond.q c2c3221 > ql/src/test/queries/clientpositive/perf/query1.q PRE-CREATION > ql/src/test/queries/clientpositive/perf/query23.q e8ebd86 > ql/src/test/queries/clientpositive/perf/query30.q PRE-CREATION > ql/src/test/queries/clientpositive/perf/query6.q PRE-CREATION > ql/src/test/queries/clientpositive/perf/query81.q PRE-CREATION > ql/src/test/queries/clientpositive/subquery_exists.q cac8e1b > ql/src/test/queries/clientpositive/subquery_in.q fe0c9c8 > ql/src/test/queries/clientpositive/subquery_in_having.q 40b7e32 > ql/src/test/queries/clientpositive/subquery_notin.q f9b5405 > ql/src/test/queries/clientpositive/subquery_scalar.q PRE-CREATION > ql/src/test/queries/clientpositive/subquery_unqualcolumnrefs.q bdfa648 > ql/src/test/results/clientnegative/subquery_exists_implicit_gby.q.out > f7251e3 > ql/src/test/results/clientnegative/subquery_in_implicit_gby.q.out > PRE-CREATION > ql/src/test/results/clientnegative/subquery_multiple_cols_in_select.q.out > 7a16bae > ql/src/test/results/clientnegative/subquery_notexists_implicit_gby.q.out > da38f5f > ql/src/test/results/clientnegative/subquery_scalar_implicit_gby.q.out > PRE-CREATION > ql/src/test/results/clientnegative/subquery_scalar_multi_columns.q.out > PRE-CREATION > ql/src/test/results/clientnegative/subquery_scalar_multi_rows.q.out > PRE-CREATION > ql/src/test/results/clientnegative/subquery_windowing_corr.q.out dcd3026 > ql/src/test/results/clientnegative/subquery_with_or_cond.q.out d2d743d > ql/src/test/results/clientpositive/llap/subquery_exists.q.out 3d8251f > ql/src/test/results/clientpositive/llap/subquery_in.q.out 887a27e > ql/src/test/results/clientpositive/llap/subquery_multi.q.out 7765221 > ql/src/test/results/clientpositive/llap/subquery_notin.q.out eb99650 > ql/src/test/results/clientpositive/llap/subquery_scalar.q.out PRE-CREATION > ql/src/test/results/clientpositive/perf/query1.q.out PRE-CREATION > ql/src/test/results/clientpositive/perf/query23.q.out 6d4cfca > ql/src/test/results/clientpositive/perf/query30.q.out PRE-CREATION > ql/src/test/results/clientpositive/perf/query6.q.out PRE-CREATION > ql/src/test/results/clientpositive/perf/query81.q.out PRE-CREATION > ql/src/test/results/clientpositive/subquery_exists.q.out c3a1b8a > ql/src/test/results/clientpositive/subquery_in_having.q.out 125187a > ql/src/test/results/clientpositive/subquery_unqualcolumnrefs.q.out 6e1cc1a > > Diff: https://reviews.apache.org/r/55553/diff/ > > > Testing > ------- > > Added q tests > > > Thanks, > > Vineet Garg > >
