Re: Review Request 20737: HIVE-6031: explain subquery rewrite for where clause predicates
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20737/#review41607 --- Few comments. Major one is to drop subquery from explain subquery rewrite syntax. ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java https://reviews.apache.org/r/20737/#comment75143 will be good to add a comment here for this replacement. ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java https://reviews.apache.org/r/20737/#comment75133 Is this needed ? Looks redundant code. If not, good to put some comments here. ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g https://reviews.apache.org/r/20737/#comment75132 I think we should not have explain subquery rewrite .. rather explain rewrite .. because later on we may want to extend this explain functionality beyond just subquery rewrites. ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java https://reviews.apache.org/r/20737/#comment75146 Doesn't make lot of difference, but QBSQRewrite extends QBSQRewriteNoop seems more natural. ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java https://reviews.apache.org/r/20737/#comment75147 Also, good to add a note here saying its non-explain path. - Ashutosh Chauhan On April 25, 2014, 10:57 p.m., Harish Butani wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20737/ --- (Updated April 25, 2014, 10:57 p.m.) Review request for hive and Ashutosh Chauhan. Bugs: HIVE-6031 https://issues.apache.org/jira/browse/HIVE-6031 Repository: hive-git Description --- explain subquery rewrite for where clause predicates Diffs - ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 679c6ec ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 3e673ca ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13bbf0a ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 864e692 ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java a8b436e ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java b7c9e65 ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 3b33dc2 ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java e7d0359 ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java 07d32ed ql/src/java/org/apache/hadoop/hive/ql/plan/ExplainSQRewriteWork.java PRE-CREATION ql/src/test/queries/clientpositive/subquery_exists_explain_rewrite.q PRE-CREATION ql/src/test/queries/clientpositive/subquery_in_explain_rewrite.q PRE-CREATION ql/src/test/results/clientpositive/subquery_exists_explain_rewrite.q.out PRE-CREATION ql/src/test/results/clientpositive/subquery_in_explain_rewrite.q.out PRE-CREATION Diff: https://reviews.apache.org/r/20737/diff/ Testing --- new tests added Thanks, Harish Butani
Re: Review Request 20737: HIVE-6031: explain subquery rewrite for where clause predicates
On April 28, 2014, 6:28 p.m., Ashutosh Chauhan wrote: ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java, line 137 https://reviews.apache.org/r/20737/diff/1/?file=568879#file568879line137 will be good to add a comment here for this replacement. done On April 28, 2014, 6:28 p.m., Ashutosh Chauhan wrote: ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java, lines 53-56 https://reviews.apache.org/r/20737/diff/1/?file=568881#file568881line53 Is this needed ? Looks redundant code. If not, good to put some comments here. removed On April 28, 2014, 6:28 p.m., Ashutosh Chauhan wrote: ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g, line 297 https://reviews.apache.org/r/20737/diff/1/?file=568882#file568882line297 I think we should not have explain subquery rewrite .. rather explain rewrite .. because later on we may want to extend this explain functionality beyond just subquery rewrites. done On April 28, 2014, 6:28 p.m., Ashutosh Chauhan wrote: ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java, line 207 https://reviews.apache.org/r/20737/diff/1/?file=568889#file568889line207 Also, good to add a note here saying its non-explain path. done - Harish --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20737/#review41607 --- On April 29, 2014, 12:43 a.m., Harish Butani wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20737/ --- (Updated April 29, 2014, 12:43 a.m.) Review request for hive and Ashutosh Chauhan. Bugs: HIVE-6031 https://issues.apache.org/jira/browse/HIVE-6031 Repository: hive-git Description --- explain subquery rewrite for where clause predicates Diffs - ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 679c6ec ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 3e673ca ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13bbf0a ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 864e692 ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java a8b436e ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java b7c9e65 ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0e0395e ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java f6b70d8 ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java 07d32ed ql/src/java/org/apache/hadoop/hive/ql/plan/ExplainSQRewriteWork.java PRE-CREATION ql/src/test/queries/clientpositive/subquery_exists_explain_rewrite.q PRE-CREATION ql/src/test/queries/clientpositive/subquery_in_explain_rewrite.q PRE-CREATION ql/src/test/results/clientpositive/subquery_exists_explain_rewrite.q.out PRE-CREATION ql/src/test/results/clientpositive/subquery_in_explain_rewrite.q.out PRE-CREATION Diff: https://reviews.apache.org/r/20737/diff/ Testing --- new tests added Thanks, Harish Butani
Re: Review Request 20737: HIVE-6031: explain subquery rewrite for where clause predicates
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/20737/ --- (Updated April 29, 2014, 12:43 a.m.) Review request for hive and Ashutosh Chauhan. Changes --- address review comments Bugs: HIVE-6031 https://issues.apache.org/jira/browse/HIVE-6031 Repository: hive-git Description --- explain subquery rewrite for where clause predicates Diffs (updated) - ql/src/java/org/apache/hadoop/hive/ql/exec/ExplainSQRewriteTask.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/exec/TaskFactory.java 679c6ec ql/src/java/org/apache/hadoop/hive/ql/parse/ExplainSQRewriteSemanticAnalyzer.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/parse/HiveLexer.g 3e673ca ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 13bbf0a ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 864e692 ql/src/java/org/apache/hadoop/hive/ql/parse/QB.java a8b436e ql/src/java/org/apache/hadoop/hive/ql/parse/QBSubQuery.java b7c9e65 ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 0e0395e ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzerFactory.java f6b70d8 ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryDiagnostic.java PRE-CREATION ql/src/java/org/apache/hadoop/hive/ql/parse/SubQueryUtils.java 07d32ed ql/src/java/org/apache/hadoop/hive/ql/plan/ExplainSQRewriteWork.java PRE-CREATION ql/src/test/queries/clientpositive/subquery_exists_explain_rewrite.q PRE-CREATION ql/src/test/queries/clientpositive/subquery_in_explain_rewrite.q PRE-CREATION ql/src/test/results/clientpositive/subquery_exists_explain_rewrite.q.out PRE-CREATION ql/src/test/results/clientpositive/subquery_in_explain_rewrite.q.out PRE-CREATION Diff: https://reviews.apache.org/r/20737/diff/ Testing --- new tests added Thanks, Harish Butani
