[ https://issues.apache.org/jira/browse/PIG-1038?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12776729#action_12776729 ]
Pradeep Kamath commented on PIG-1038: ------------------------------------- In JobControlCompiler: ====================== {code} 583 jobConf.set("pig.secondarySortOrder", 584 ObjectSerializer.serialize(mro.getSecondarySortOrder())); 585 } {code} Looks like above code should be set in the case of non order by Mro which uses secondary key {code} 638 valuea = ((Tuple)wa.getValueAsPigType()).get(0); {code} We should put a comment explaining that we extract the first field out since that represents the actual group by key. In SecondaryKeyOptimizer: ======================== {code} } else if (mapLeaf instanceof POUnion || mapLeaf instanceof POSplit) { {code} The above should not contain POSplit since POSplit would only occur after multi query optimization which happens later. {code} 94 } else if (plan.getRoots().size() != 1) { 95 // POLocalRearrange plan contains more than 1 root. 96 // Probably there is an Expression operator in the local 97 // rearrangement plan, 98 // skip secondary key optimizing 99 return null; {code} Should we do continue nextPlan instead of return null here since this is similar to udf or constant in local rearrange case {code} 105. columnChainInfo.insert(false, columns, DataType.TUPLE); {code} It would useful to put a comment explaining this is put into the ColumnChainInfo only for comparing that different components of SortKeyInfo are all coming from the same input index. Also should the datatype be BAG? {code} 118 log.debug(node + " have more than 1 predecessor"); {code} predecessor should change to successor. {code} 217 if (currentNode instanceof POPackage 218 || currentNode instanceof POFilter 219 || currentNode instanceof POLimit) { {code} In line 217 we should ensure, we don't optimize when we encounter POJoinPackage using something like {code} if ((currentNode instanceof POPackage && !(currentNode instanceof POJoinPackage)) {code} {code} 307. int errorCode = 1000; 327 int errorCode = 1000; 526. int errorCode = 1000; {code} This error code is already in use {code} 336 } else if (mapLeaf instanceof POUnion || mapLeaf instanceof POSplit) { 337 List<PhysicalOperator> preds = mr.mapPlan 338 .getPredecessors(mapLeaf); 339 for (PhysicalOperator pred : preds) { 340 POLocalRearrange rearrange = (POLocalRearrange) pred; 341 rearrange.setUseSecondaryKey(true); 342 if (rearrange.getIndex() == indexOfRearrangeToChange) // Try 343 // to 344 // find 345 // the 346 // POLocalRearrange 347 // for 348 // the 349 // secondary 350 // key 351 setSecondaryPlan(mr.mapPlan, rearrange, 352 secondarySortKeyInfo); 353 } 354 } {code} The above should not contain POSplit since POSplit would only occur after multi query optimization which happens later. Also in the if statement on line 342, what if the condition evaluates to false - should we throw an Exception like earlier in the same method? {code} 530 if (r) 531 sawInvalidPhysicalOper = true; .. 557 if (r) // if we saw physical operator other than project in sort 558 // plan 559 return; {code} At line 559 should we be setting sawInvalidPhysicalOper? General comments: ================= A comment on ColumnChainInfo and SortKeyInfo explaining how it tracks to POProjects in the plan would be useful POMultiQueryPackage should not change since SecondaryKeyOptimizer runs before MultiQueryOptimizer. > Optimize nested distinct/sort to use secondary key > -------------------------------------------------- > > Key: PIG-1038 > URL: https://issues.apache.org/jira/browse/PIG-1038 > Project: Pig > Issue Type: Improvement > Components: impl > Affects Versions: 0.4.0 > Reporter: Olga Natkovich > Assignee: Daniel Dai > Fix For: 0.6.0 > > Attachments: PIG-1038-1.patch, PIG-1038-2.patch, PIG-1038-3.patch > > > If nested foreach plan contains sort/distinct, it is possible to use hadoop > secondary sort instead of SortedDataBag and DistinctDataBag to optimize the > query. > Eg1: > A = load 'mydata'; > B = group A by $0; > C = foreach B { > D = order A by $1; > generate group, D; > } > store C into 'myresult'; > We can specify a secondary sort on A.$1, and drop "order A by $1". > Eg2: > A = load 'mydata'; > B = group A by $0; > C = foreach B { > D = A.$1; > E = distinct D; > generate group, E; > } > store C into 'myresult'; > We can specify a secondary sort key on A.$1, and simplify "D=A.$1; E=distinct > D" to a special version of distinct, which does not do the sorting. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.