opt: change InterestingOrderings to use OrderingChoice#64501
opt: change InterestingOrderings to use OrderingChoice#64501craig[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
This commit moves OrderingChoice from pkg/sql/opt/props/physical to pkg/sql/opt/props in preparation for using it in pkg/sql/opt/props. This will be necessary to avoid an import cycle. Release note: None
|
pkg/sql/opt/xform/testdata/physprops/ordering, line 1495 at r3 (raw file):
This is the one place where we have a worse plan. I think it has something to do with the interesting orderings not aligning with the provided ordering, but I'm having trouble figuring it out. We shouldn't need any sort here at all. |
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @mgartner and @rytaft)
pkg/sql/opt/xform/testdata/physprops/ordering, line 1495 at r3 (raw file):
Previously, rytaft (Rebecca Taft) wrote…
This is the one place where we have a worse plan. I think it has something to do with the interesting orderings not aligning with the provided ordering, but I'm having trouble figuring it out. We shouldn't need any sort here at all.
Provided orderings (the actual) are born in a post-processing step after the final plan is chosen, they can't affect the plan. Not sure if you meant those though.
The only place where interesting orderings play a role in this query is when we try segmented sort. Something stopped working in that logic, probably because what used to be +1,+2,+3 is now +(1|2),+3.
As for why we sort at all, it's the +(1|2),+3 which can only get pushed down under the select as +1,+3 or +2,+3 (although +1,+2,+3 would also be correct). This can be improved by using interesting orderings to choose what to push down; tracked by issue #33023
|
pkg/sql/opt/xform/testdata/physprops/ordering, line 1495 at r3 (raw file): Previously, RaduBerinde wrote…
Oh nice! Thanks for the hint. I've fixed this in #64593. |
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @mgartner, @RaduBerinde, and @rytaft)
pkg/sql/opt/xform/join_funcs.go, line 101 at r3 (raw file):
for _, ord := range orders { o := ord.ToOrderingLong()
[nit] ToOrderingLong is a bit questionable as a general primitive. I wonder if we shouldn't do it "inline" and while addressing the TODO below as well. We'd do the loop below directly on the OrderingChoice and then loop through the remaining columns and add them as well.
pkg/sql/opt/props/ordering_choice.go, line 729 at r3 (raw file):
// set. If a group does not contain any columns in the given set, it removes that // group and all following groups. func (oc *OrderingChoice) ProjectColsNoError(cols opt.ColSet) {
[nit] I'd name it RestrictToCols
6d365fa to
e466748
Compare
rytaft
left a comment
There was a problem hiding this comment.
TFTR!
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @mgartner)
pkg/sql/opt/xform/join_funcs.go, line 101 at r3 (raw file):
Previously, RaduBerinde wrote…
[nit] ToOrderingLong is a bit questionable as a general primitive. I wonder if we shouldn't do it "inline" and while addressing the TODO below as well. We'd do the loop below directly on the OrderingChoice and then loop through the remaining columns and add them as well.
Good idea! Done.
pkg/sql/opt/props/ordering_choice.go, line 729 at r3 (raw file):
Previously, RaduBerinde wrote…
[nit] I'd name it
RestrictToCols
Done.
This commit changes the InterestingOrderings logical property to use a set of OrderingChoices rather than a set of Orderings. This allows the optimizer to take advantage of a larger number of interesting orderings when some columns are constant or there are equalities between columns. Informs cockroachdb#56201 Release note (performance improvement): increased the intelligence of the optimizer around orderings that can be provided by certain relational expressions when some columns are constant or there are equalities between columns. This can allow the optimizer to plan merge joins, streaming group bys, and streaming set operations in more cases, resulting in improved performance.
|
bors r+ |
|
Build succeeded: |
opt: move
OrderingChoicefromphysicaltopropsThis commit moves
OrderingChoicefrompkg/sql/opt/props/physicaltopkg/sql/opt/propsin preparation for using it inpkg/sql/opt/props.This will be necessary to avoid an import cycle.
opt: change
InterestingOrderingsto useOrderingChoiceThis commit changes the
InterestingOrderingslogical property touse a set of
OrderingChoices rather than a set ofOrderings. Thisallows the optimizer to take advantage of a larger number of
interesting orderings when some columns are constant or there
are equalities between columns.
Informs #56201
Release note (performance improvement): increased the intelligence
of the optimizer around orderings that can be provided by certain
relational expressions when some columns are constant or there
are equalities between columns. This can allow the optimizer to
plan merge joins, streaming group bys, and streaming set operations
in more cases, resulting in improved performance.