> On March 20, 2018, 12:08 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java > > Line 2501 (original), 2507 (patched) > > <https://reviews.apache.org/r/66069/diff/2/?file=1982591#file1982591line2507> > > > > I think lookup(op) is better than lookup(treeSig) since that way we can > > delay computation of signature. e.g, if I configure to run with > > EmptyStatsSource (to turn off this feature), this computation will be > > avoided.
the operator to signature calculation is done using a cache using the PlanMapper which is bound to the current plan; If I change the interface method to use Operator - I loose access to that signature cache - and the usage of the cache will probably look a bit out of place during the lookup process - I think this way it's more natural that the signature cache will go down along with the mapper. I think it would be probably safer this way in the long run - since in case of a statsSource which has a longer lifetime than a single query , it may by mistake keep some references to the previous query's objects thru some cache... I've added an conditional to the beginning of this method to disable these things altogether in case reexecution is disabled. > On March 20, 2018, 12:08 a.m., Ashutosh Chauhan wrote: > > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/PlanMapper.java > > Line 43 (original), 48 (patched) > > <https://reviews.apache.org/r/66069/diff/2/?file=1982597#file1982597line48> > > > > Add comments about this class. I've added some apidocs; and I've renamed it to EquivGroup - Zoltan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66069/#review199495 ----------------------------------------------------------- On March 19, 2018, 9:59 p.m., Zoltan Haindrich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66069/ > ----------------------------------------------------------- > > (Updated March 19, 2018, 9:59 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Bugs: HIVE-18926 > https://issues.apache.org/jira/browse/HIVE-18926 > > > Repository: hive-git > > > Description > ------- > > * match operators based on opsigs > * always calc operator sigs > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/ConvertJoinMapJoin.java > a235f3fbf4 > ql/src/java/org/apache/hadoop/hive/ql/optimizer/signature/OpSignature.java > 90b2fd3dad > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/signature/OpTreeSignature.java > c6d1a6aaca > ql/src/java/org/apache/hadoop/hive/ql/optimizer/signature/Signature.java > c228a8e4f5 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/signature/SignatureUtils.java > 2269322b69 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/stats/annotation/StatsRulesProcFactory.java > 22b052cd07 > ql/src/java/org/apache/hadoop/hive/ql/plan/CommonMergeJoinDesc.java > 5a81add706 > ql/src/java/org/apache/hadoop/hive/ql/plan/JoinCondDesc.java ea22131c56 > ql/src/java/org/apache/hadoop/hive/ql/plan/LateralViewJoinDesc.java > 85a4683491 > ql/src/java/org/apache/hadoop/hive/ql/plan/MapJoinDesc.java 91ea159d52 > ql/src/java/org/apache/hadoop/hive/ql/plan/ReduceSinkDesc.java f2955af25e > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/PlanMapper.java > 36d7e589a8 > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/PlanMapperProcess.java > 424dd7956b > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/RuntimeStatsSource.java > 21a0678153 > > ql/src/java/org/apache/hadoop/hive/ql/plan/mapper/SimpleRuntimeStatsSource.java > 6f340b8450 > ql/src/java/org/apache/hadoop/hive/ql/reexec/ReExecDriver.java 93031712dc > > ql/src/test/org/apache/hadoop/hive/ql/optimizer/signature/TestOperatorSignature.java > 8c899e7fef > ql/src/test/org/apache/hadoop/hive/ql/plan/mapping/TestOperatorCmp.java > cfb0ca38b2 > ql/src/test/results/clientpositive/llap/retry_failure_stat_changes.q.out > 9b58ce0e5d > > > Diff: https://reviews.apache.org/r/66069/diff/2/ > > > Testing > ------- > > > Thanks, > > Zoltan Haindrich > >
