> On Oct. 23, 2014, 7:33 p.m., John Pullokkaran wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line 9981 > > <https://reviews.apache.org/r/25550/diff/7/?file=728978#file728978line9981> > > > > Shouldn't this check be for msg.isEmpty as opposed to null. Since in > > verbose mode msg will never be empty.
see structure of the "if". In case all conditions is satisfied, it will always return null. Verbose is only checked later. > On Oct. 23, 2014, 7:33 p.m., John Pullokkaran wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line > > 12419 > > <https://reviews.apache.org/r/25550/diff/7/?file=728978#file728978line12419> > > > > Why don't we remove this if block & remove verbose flag. > > > > Always return the reason for not running CBO. > > Caller based on log level can decide to log the reason. building the reason can be expensive... > On Oct. 23, 2014, 7:33 p.m., John Pullokkaran wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java, line > > 12437 > > <https://reviews.apache.org/r/25550/diff/7/?file=728978#file728978line12437> > > > > As pointed out last time, wouldn't this cause CBO to never run in Log > > level info. > > > > Caller code is > > msg = canHandleQuery(qb, true, LOG.isInfoEnabled()); > > runCBO = runCBO && (msg == null); no, verbose is only checked when CBO already cannot run. I'll make code more clear - Sergey ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25550/#review58065 ----------------------------------------------------------- On Oct. 22, 2014, 8:34 p.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25550/ > ----------------------------------------------------------- > > (Updated Oct. 22, 2014, 8:34 p.m.) > > > Review request for hive, Ashutosh Chauhan and John Pullokkaran. > > > Repository: hive-git > > > Description > ------- > > see JIRA > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/index/RewriteParseContextGenerator.java > dee7d7e > ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java > 37cbf7f > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 511103b > ql/src/test/queries/clientpositive/cbo_correctness.q 4d8f156 > ql/src/test/queries/clientpositive/ctas_colname.q 5322626 > ql/src/test/queries/clientpositive/decimal_serde.q cf3a86c > ql/src/test/queries/clientpositive/insert0.q PRE-CREATION > ql/src/test/results/clientpositive/ctas_colname.q.out 97dacf6 > ql/src/test/results/clientpositive/decimal_serde.q.out e461c2e > ql/src/test/results/clientpositive/insert0.q.out PRE-CREATION > > Diff: https://reviews.apache.org/r/25550/diff/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
