> On Aug. 31, 2016, 6:31 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java, > > line 375 > > <https://reviews.apache.org/r/51542/diff/1/?file=1488888#file1488888line375> > > > > WalkState.CONSTANT or WalkState.UNKNOWN? (I think in this case it > > should be the latest) > > Sergey Shelukhin wrote: > this is existing code moved as is :)
OK, it is CONSTANT because it is only called in handleDeterministicUDF, after other checks have been done. I honestly would not create its own method for this logic, as it is rather misleading : it does fold the expressions, but it makes some assumptions on the nature of that expression. > On Aug. 31, 2016, 6:31 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java, > > line 286 > > <https://reviews.apache.org/r/51542/diff/1/?file=1488888#file1488888line286> > > > > Will STRUCT with all constants inside e.g. (x, y) in (1,2), be handled > > correctly by this code block? In that case, (1, 2) should be considered a > > constant, thus we could remove the IN. However, I think currently we will > > end up with UNKNOWN state and we will never remove it. I think that was a > > case that we were supporting previously, thus it would be a regression. > > Could you double check? > > Sergey Shelukhin wrote: > Those are handled elsewhere. See pointlookup tests where they are removed I do not understand what you mean. Removal of pcr is handled here. I have been checking : pointlookup tests do not contain any test that removes an IN clause with STRUCT. Consider the following (based on pointlookup4): create table pcr_t1 (key int, value string) partitioned by (ds1 string, ds2 string); insert overwrite table pcr_t1 partition (ds1='2000-04-08', ds2='2001-04-08') select * from src where key < 20 order by key; insert overwrite table pcr_t1 partition (ds1='2000-04-09', ds2='2001-04-09') select * from src where key < 20 order by key; select select key, value, ds1, ds2 from pcr_t1 where (struct(ds1,ds2)) IN (struct('2000-04-08','2001-04-08'), struct('2000-04-09','2001-04-09')); For the query above, once we have executed partitioned prunning, the filter predicate can be removed. I am afraid that with the new change in the code, we will identify the CONST STRUCT e.g. struct('2000-04-08','2001-04-08') as UNKNOWN, and thus we will not consider that predicate for removal. In fact, the extension should be as easy as checking whether the STRUCT is a CONST STRUCT or not, and create the WalkState accordingly. I think it is important to deal with this case, as I guess this kind of predicates were the objective when support for IN-STRUCT clauses was added to PCR in the first place. - Jesús ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51542/#review147456 ----------------------------------------------------------- On Aug. 31, 2016, 6:42 p.m., Sergey Shelukhin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51542/ > ----------------------------------------------------------- > > (Updated Aug. 31, 2016, 6:42 p.m.) > > > Review request for hive and Jesús Camacho Rodríguez. > > > Bugs: HIVE-14652 > https://issues.apache.org/jira/browse/HIVE-14652 > > > Repository: hive-git > > > Description > ------- > > see jira > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/pcr/PcrExprProcFactory.java > f9388e2 > ql/src/java/org/apache/hadoop/hive/ql/parse/ParseContext.java b2125ca > ql/src/test/queries/clientpositive/partition_condition_remover.q > PRE-CREATION > ql/src/test/results/clientpositive/partition_condition_remover.q.out > PRE-CREATION > ql/src/test/results/clientpositive/pcs.q.out 0045c1c > > Diff: https://reviews.apache.org/r/51542/diff/ > > > Testing > ------- > > > Thanks, > > Sergey Shelukhin > >
