ENG-2503 patch

9 views
Skip to first unread message

Michael Alexeev

unread,
Apr 9, 2012, 8:21:42 PM4/9/12
to voltdb-dev
Hi All,

I prepared the patch for ENG-2503 - push down Distinct node to a partition for a distributed access plan. All changes related to inlining the Distinct node are reverted back.

Please let me know your comments.

Thanks,
Mike



0001-ENG-2503-push-down-distinct-node.patch

Paul Martel

unread,
Apr 10, 2012, 8:53:35 PM4/10/12
to voltd...@googlegroups.com
Mike,

Thanks for the patch. It looks good to go. I can check it directly into the github master tomorrow, unless you want to post a formal pull request tonight.

In my review of the patch, I discovered a few opportunities for follow-up work.

1. The pseudo-handling of multi-column distinct via multiple distinct nodes (currently guarded by hard-coded restrictions) needs to be eliminated - it couldn't really work.

2. The existing distinct nodes should be generalized to take a list of expressions rather than a single column reference.
This expansion of functionality will eventually need to make its way down to the distinct executor and into some unit tests.
Multi-expression GROUP BY could provide a model for this -- or actually a replacement (more on this below).

3. There are opportunities for further optimization specific to "distinct" processing of special cases: if the distinct expression (or any element of a distinct expression list) is a reference to a partitioning column, all the work can be done by the distributed distinct node and the final distinct node can be left off -- actually this also applies to GROUP BY but I don't recall seeing this optimization in the "Aggregation" push-down either.

4. The placement of a projection node above the distinct node -- if one is ever needed -- seems like a missed optimization -- the only column values that can usefully flow into a distinct node are the exact same ones that flow out of it. It seems like we could be needlessly sending/receiving overly wide rows if a required projection is not pushed below the distributed distinct.

Actually, items 2 and 3 make me want to re-write distinct as a special case of GROUP BY and possibly eliminate the redundancy of separate support for a distinct executor -- it's hard for me to imagine that there's a lot to be gained at execution time from hard-coding the special-case constraints of distinct vs. group by (no extra columns -- just the same set coming in, getting grouped, and going out).

I'll log issues for all of these tomorrow.

I'll send another message shortly with some ideas about pull-based execution.

--paul

Michael Alexeev

unread,
Apr 10, 2012, 9:43:09 PM4/10/12
to voltd...@googlegroups.com
Hi Paul,

On Tue, Apr 10, 2012 at 8:53 PM, Paul Martel <pma...@voltdb.com> wrote:
Mike,

Thanks for the patch. It looks good to go. I can check it directly into the github master tomorrow, unless you want to post a formal pull request tonight.

Please go ahead with the patch. 

In my review of the patch, I discovered a few opportunities for follow-up work.

1. The pseudo-handling of multi-column distinct via multiple distinct nodes (currently guarded by hard-coded restrictions) needs to be eliminated - it couldn't really work.

2. The existing distinct nodes should be generalized to take a list of expressions rather than a single column reference.
This expansion of functionality will eventually need to make its way down to the distinct executor and into some unit tests.
Multi-expression GROUP BY could provide a model for this -- or actually a replacement (more on this below).

It's funny, I was about to write an e-mail about eliminating this single column restriction. John and I discussed it briefly few weeks ago. The idea was instead of stacking multiple distinct nodes for each column to have a single distinct node with expression chaining TVEs into a single distinct expression. The executor changes than would be limited to replacing set of unique NValue objects with a set of TableTuple and providing less operator for them ( can be implemented in-terms of already existing compare method)  


3. There are opportunities for further optimization specific to "distinct" processing of special cases: if the distinct expression (or any element of a distinct expression list) is a reference to a partitioning column, all the work can be done by the distributed distinct node and the final distinct node can be left off -- actually this also applies to GROUP BY but I don't recall seeing this optimization in the "Aggregation" push-down either.

It sounds very familiar. ENG-496 uses the same optimization in case of multi-table join on partition column. 

4. The placement of a projection node above the distinct node -- if one is ever needed -- seems like a missed optimization -- the only column values that can usefully flow into a distinct node are the exact same ones that flow out of it. It seems like we could be needlessly sending/receiving overly wide rows if a required projection is not pushed below the distributed distinct.

Actually, items 2 and 3 make me want to re-write distinct as a special case of GROUP BY and possibly eliminate the redundancy of separate support for a distinct executor -- it's hard for me to imagine that there's a lot to be gained at execution time from hard-coding the special-case constraints of distinct vs. group by (no extra columns -- just the same set coming in, getting grouped, and going out).

I would be more than happy to help you there.

Mike 
Reply all
Reply to author
Forward
0 new messages