Skip to content

opt: change InterestingOrderings to use OrderingChoice#64501

Merged
craig[bot] merged 2 commits intocockroachdb:masterfrom
rytaft:interesting-ord
May 4, 2021
Merged

opt: change InterestingOrderings to use OrderingChoice#64501
craig[bot] merged 2 commits intocockroachdb:masterfrom
rytaft:interesting-ord

Conversation

@rytaft
Copy link
Copy Markdown
Collaborator

@rytaft rytaft commented Apr 30, 2021

opt: move OrderingChoice from physical to props
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.

opt: change InterestingOrderings to use OrderingChoice
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 #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.

@rytaft rytaft requested review from a team, RaduBerinde and mgartner April 30, 2021 23:07
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

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
@rytaft rytaft force-pushed the interesting-ord branch from 7c12123 to 73c6b2c Compare May 1, 2021 00:02
@rytaft
Copy link
Copy Markdown
Collaborator Author

rytaft commented May 1, 2021


pkg/sql/opt/xform/testdata/physprops/ordering, line 1495 at r3 (raw file):

 ├── fd: (9)==(10), (10)==(9)
 ├── ordering: +(9|10) [actual: +9]
 ├── sort (segmented)

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.

Copy link
Copy Markdown
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: 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

@rytaft
Copy link
Copy Markdown
Collaborator Author

rytaft commented May 3, 2021


pkg/sql/opt/xform/testdata/physprops/ordering, line 1495 at r3 (raw file):

Previously, RaduBerinde wrote…

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

Oh nice! Thanks for the hint. I've fixed this in #64593.

Copy link
Copy Markdown
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm: Nice improvement!

Reviewable status: :shipit: 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

@rytaft rytaft force-pushed the interesting-ord branch 2 times, most recently from 6d365fa to e466748 Compare May 3, 2021 22:09
Copy link
Copy Markdown
Collaborator Author

@rytaft rytaft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR!

Reviewable status: :shipit: 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.
@rytaft rytaft force-pushed the interesting-ord branch from e466748 to fc101dc Compare May 4, 2021 11:39
@rytaft
Copy link
Copy Markdown
Collaborator Author

rytaft commented May 4, 2021

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented May 4, 2021

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants