opt: create library that determines how joins affect input rows

[DrewKimball]

Previously, there was no simple way to determine whether all rows from
a join input will be included in its output, nor whether input rows will
be duplicated by the join.

This patch adds a library that constructs a Multiplicity struct for join
operators. The Multiplicity can be queried for information about how a
join will affect its input rows (e.g. duplicated, filtered and/or
null-extended). The existing SimplifyLeftJoinWithFilters rule has been
refactored to use this library. The Multiplicity library will also be
useful for future join elimination and limit pushdown rules.

Release note: None

[blathers-crl]

Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[cockroach-teamcity]

This change is 

[DrewKimball]

I also plan on adding random testing in a future PR.

[RaduBerinde]

Nice work! I have some comments about the new properties, it's worth spending some time to make them as clean and easy to use as possible.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball)

---

pkg/sql/opt/memo/multiplicity_builder.go, line 1 at r1 (raw file):

// Copyright 2018 The Cockroach Authors.

[nit] 2020

---

pkg/sql/opt/memo/multiplicity_builder.go, line 83 at r1 (raw file):

	joinOp opt.Operator, left, right RelExpr, filters FiltersExpr,
) props.Multiplicity {
	isLeftOuter := false

[nit] shorter to just write isLeftOuter := joinOp == opt.LeftJoinOp || joinOp == opt.FullJoinOp

We could also add some "NullExtendsLeft / NulExtendsRight" tags to the relevant join operators.

---

pkg/sql/opt/memo/multiplicity_builder.go, line 104 at r1 (raw file):

	rightMultiplicity := getJoinLeftMultiplicityVal(right, left, filters, isRightOuter)
	if isLeftOuter {
		rightMultiplicity |= props.NullExtendedMultiplicityVal

If this flag is directly derived from the join type, it's not useful (we can always look at the join type)..

---

pkg/sql/opt/memo/testdata/logprops/join, line 45 at r1 (raw file):

 ├── prune: (2-4,6,7)
 ├── interesting orderings: (+1) (-3,+4,+1) (+7)
 ├── multiplicity: left-rows(can-dup, can-discard), right-rows(no-dup, can-discard)

I think we should hide this when it's basically saying "anything goes"

---

pkg/sql/opt/props/multiplicity.go, line 1 at r1 (raw file):

// Copyright 2018 The Cockroach Authors.

[nit] 2020

---

pkg/sql/opt/props/multiplicity.go, line 19 at r1 (raw file):

)

// MultiplicityValue is an enum that describes whether a join filters,

s/enum/bitfield

Also, consider naming these things JoinMultiplicity instead of just Multiplicity because it sounds like a more general concept.

---

pkg/sql/opt/props/multiplicity.go, line 20 at r1 (raw file):

// MultiplicityValue is an enum that describes whether a join filters,
// duplicates and/or null-extends rows from its input.

"from a particular (left or right) input"

---

pkg/sql/opt/props/multiplicity.go, line 24 at r1 (raw file):

const (
	// UnchangedMultiplicityVal indicates that every input row will be included in

[nit] MultiplicityUnchanged, MultiplicityDuplicated

---

pkg/sql/opt/props/multiplicity.go, line 26 at r1 (raw file):

	// UnchangedMultiplicityVal indicates that every input row will be included in
	// the join output exactly once.
	UnchangedMultiplicityVal MultiplicityValue = 0

It would be more natural if the zero value would mean no guarantees (as opposed to the strongest guarantee). For that we could flip the meaning of the flags.

---

pkg/sql/opt/props/multiplicity.go, line 34 at r1 (raw file):

	// FilteredMultiplicityVal indicates that the join may not include all input
	// rows in its output.
	FilteredMultiplicityVal

What is the relation between this flag and NullExtended? Does a row that is null-extended (because it had no join matches) count as being "filtered"?

---

pkg/sql/opt/props/multiplicity.go, line 38 at r1 (raw file):

	// NullExtendedMultiplicityVal indicates that the join may null-extend the
	// input.
	NullExtendedMultiplicityVal

This doesn't seem used in this PR, do we envision uses for it? Because it doesn't fit very neatly with the others (e.g. we don't even display it). We can consider having the multiplicity be a property of the conceptual inner-join, and whoever uses this property can use it in conjunction with the join type to deduce whatever they need (we can provide some helpers for that).

---

pkg/sql/opt/props/multiplicity.go, line 69 at r1 (raw file):

// be considered immutable.
type Multiplicity struct {
	// UnfilteredCols contains all columns from the operator's input(s) that are

Feels strange to have UnfilteredCols be here, since this property conceptually only applies to joins (whereas UnfilteredCols applies to anything).

---

pkg/sql/opt/props/multiplicity.go, line 78 at r1 (raw file):

	// initializing the other fields. Other callers should only use the property
	// methods (e.g. JoinFiltersMatchAllLeftRows).
	UnfilteredCols opt.ColSet

This is an idea for future improvements but a case we care about is when there is a filter on the equality column. That filter gets pushed down to both sides (so both sides are not "unfiltered") but the "non-filtered" condition would still hold.

I think we can use constraints to figure out this case: if the right side is just a Select on top of "unfiltered cols" with tight constraints that only include the equality columns, and the left side has constraints at least as restrictive as the ones on the right side, then we can prove that the left side will be "non-filtered" by the join.

---

pkg/sql/opt/props/multiplicity.go, line 142 at r1 (raw file):

// interesting.
func (mp *Multiplicity) String() string {
	var buf bytes.Buffer

It would be useful to have simpler descriptions for some common cases, e.g. "one-to-one".

[blathers-crl]

Thank you for updating your pull request.

My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI.

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan.}

[DrewKimball]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RaduBerinde)

---

pkg/sql/opt/memo/multiplicity_builder.go, line 1 at r1 (raw file):

Previously, RaduBerinde wrote…

[nit] 2020

Done.

---

pkg/sql/opt/memo/multiplicity_builder.go, line 83 at r1 (raw file):

Previously, RaduBerinde wrote…

[nit] shorter to just write isLeftOuter := joinOp == opt.LeftJoinOp || joinOp == opt.FullJoinOp

We could also add some "NullExtendsLeft / NulExtendsRight" tags to the relevant join operators.

Yes, this is much nicer. Done.

---

pkg/sql/opt/memo/multiplicity_builder.go, line 104 at r1 (raw file):

Previously, RaduBerinde wrote…

If this flag is directly derived from the join type, it's not useful (we can always look at the join type)..

I'll remove the NullExtended flag.

---

pkg/sql/opt/memo/testdata/logprops/join, line 45 at r1 (raw file):

Previously, RaduBerinde wrote…

I think we should hide this when it's basically saying "anything goes"

Done.

---

pkg/sql/opt/props/multiplicity.go, line 1 at r1 (raw file):

Previously, RaduBerinde wrote…

[nit] 2020

Done.

---

pkg/sql/opt/props/multiplicity.go, line 19 at r1 (raw file):

Previously, RaduBerinde wrote…

s/enum/bitfield

Also, consider naming these things JoinMultiplicity instead of just Multiplicity because it sounds like a more general concept.

Done.

---

pkg/sql/opt/props/multiplicity.go, line 20 at r1 (raw file):

Previously, RaduBerinde wrote…

"from a particular (left or right) input"

Done.

---

pkg/sql/opt/props/multiplicity.go, line 24 at r1 (raw file):

Previously, RaduBerinde wrote…

[nit] MultiplicityUnchanged, MultiplicityDuplicated

Done.

---

pkg/sql/opt/props/multiplicity.go, line 26 at r1 (raw file):

Previously, RaduBerinde wrote…

It would be more natural if the zero value would mean no guarantees (as opposed to the strongest guarantee). For that we could flip the meaning of the flags.

Done.

---

pkg/sql/opt/props/multiplicity.go, line 34 at r1 (raw file):

Previously, RaduBerinde wrote…

What is the relation between this flag and NullExtended? Does a row that is null-extended (because it had no join matches) count as being "filtered"?

I originally intended NullExtended to mean that null rows would be added to columns from that input. In that sense, an input could be null-extended without being filtered. However, I think I'll take your advice and simply remove the NullExtended field altogether.

---

pkg/sql/opt/props/multiplicity.go, line 38 at r1 (raw file):

Previously, RaduBerinde wrote…

This doesn't seem used in this PR, do we envision uses for it? Because it doesn't fit very neatly with the others (e.g. we don't even display it). We can consider having the multiplicity be a property of the conceptual inner-join, and whoever uses this property can use it in conjunction with the join type to deduce whatever they need (we can provide some helpers for that).

I'm anticipating using something like this in a join elimination rule, but it would be just as easy to check the type of join.

The issue with having the multiplicity describe the conceptual inner join is that we won't be able to take shortcuts when we encounter an outer join (which we can do because outer joins automatically add back filtered rows). We'll have to call filtersMatchAllLeftRows for left and full joins when we could just look at the type of join. I do like the idea of keeping track of the inner multiplicity though (and in fact I did this in an earlier draft). If you don't think being able to skip filtersMatchAllLeftRows for outer joins is a
big deal, I'll happily change it.

---

pkg/sql/opt/props/multiplicity.go, line 69 at r1 (raw file):

Previously, RaduBerinde wrote…

Feels strange to have UnfilteredCols be here, since this property conceptually only applies to joins (whereas UnfilteredCols applies to anything).

I think I'll push back on this one a bit. UnfilteredCols requires a JoinMultiplicity in order to be populated in the case when there's a join in the tree, and of course JoinMultiplicity requires the UnfilteredCols field. The two are pretty interdependent, and UnfilteredCols isn't used anywhere else.

---

pkg/sql/opt/props/multiplicity.go, line 78 at r1 (raw file):

Previously, RaduBerinde wrote…

This is an idea for future improvements but a case we care about is when there is a filter on the equality column. That filter gets pushed down to both sides (so both sides are not "unfiltered") but the "non-filtered" condition would still hold.

I think we can use constraints to figure out this case: if the right side is just a Select on top of "unfiltered cols" with tight constraints that only include the equality columns, and the left side has constraints at least as restrictive as the ones on the right side, then we can prove that the left side will be "non-filtered" by the join.

This is an interesting case. Are you envisioning that constraints would be bubbled up, or that that there would be a ruleprop like UnfilteredCols that gives information about column constraints?

---

pkg/sql/opt/props/multiplicity.go, line 142 at r1 (raw file):

Previously, RaduBerinde wrote…

It would be useful to have simpler descriptions for some common cases, e.g. "one-to-one".

I'm going with "zero-or-more, "one-or-zero", "one-or-more", and "exactly-one", since it's hard to follow the "one-to-one" pattern without the flags getting long ("one-to-one-or-more"). Do you think this is clear enough? Another option would be to use something like this:

multiplicity: left-rows [ 1 - 1 ], right-rows [ 0 - ]

[RaduBerinde]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball and @RaduBerinde)

---

pkg/sql/opt/props/multiplicity.go, line 38 at r1 (raw file):

Previously, DrewKimball (Andrew Kimball) wrote…

I'm anticipating using something like this in a join elimination rule, but it would be just as easy to check the type of join.

The issue with having the multiplicity describe the conceptual inner join is that we won't be able to take shortcuts when we encounter an outer join (which we can do because outer joins automatically add back filtered rows). We'll have to call filtersMatchAllLeftRows for left and full joins when we could just look at the type of join. I do like the idea of keeping track of the inner multiplicity though (and in fact I did this in an earlier draft). If you don't think being able to skip filtersMatchAllLeftRows for outer joins is a
big deal, I'll happily change it.

Makes sense, I think your opinion is more informed at this point. I was thinking we could use it for the leftjoin->innerjoin rule but I see now that we don't yet have the property calculated there anyway (and we can just calculate it for innerjoin).

---

pkg/sql/opt/props/multiplicity.go, line 69 at r1 (raw file):

Previously, DrewKimball (Andrew Kimball) wrote…

I think I'll push back on this one a bit. UnfilteredCols requires a JoinMultiplicity in order to be populated in the case when there's a join in the tree, and of course JoinMultiplicity requires the UnfilteredCols field. The two are pretty interdependent, and UnfilteredCols isn't used anywhere else.

I see, sounds good. I would just mention explicitly in the comment that the property struct pertains to any operator (because of UnfilteredCols).

[DrewKimball]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RaduBerinde)

---

pkg/sql/opt/props/multiplicity.go, line 38 at r1 (raw file):

Previously, RaduBerinde wrote…

Makes sense, I think your opinion is more informed at this point. I was thinking we could use it for the leftjoin->innerjoin rule but I see now that we don't yet have the property calculated there anyway (and we can just calculate it for innerjoin).

Exactly.

---

pkg/sql/opt/props/multiplicity.go, line 69 at r1 (raw file):

Previously, RaduBerinde wrote…

I see, sounds good. I would just mention explicitly in the comment that the property struct pertains to any operator (because of UnfilteredCols).

Done.

[rytaft]

This is great! This is going to be really helpful to improve our join stats too.

My main question is regarding the assumptions you've made about foreign keys and whether they apply in all cases (see below). Otherwise just some nits.

Reviewed 8 of 25 files at r1, 18 of 18 files at r2.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @DrewKimball and @RaduBerinde)

---

pkg/sql/opt/memo/multiplicity_builder.go, line 23 at r2 (raw file):

// join operator will affect the rows of its left and right inputs (e.g.
// duplicated and/or filtered). When the function is called on an operator that
// other than an InnerJoin, a LeftJoin, or a FullJoin, it simply populates the

[nit] that other than -> other than

---

pkg/sql/opt/memo/multiplicity_builder.go, line 85 at r2 (raw file):

	switch joinOp {

[nit] empty line

---

pkg/sql/opt/memo/multiplicity_builder.go, line 189 at r2 (raw file):

// 2. If this is not a cross join, every filter falls under one of these two
//    cases:
//   a. The self-join case: an equality between meta columns that come from the

what are meta columns?

---

pkg/sql/opt/memo/multiplicity_builder.go, line 209 at r2 (raw file):

//    the referenced column. Therefore, if the foreign key has at least one
//    not-null column, the referenced columns must have at least one row
//    whenever the foreign key columns do.

Is this true in the case of match simple? I think it's only match full....

---

pkg/sql/opt/memo/multiplicity_builder.go, line 258 at r2 (raw file):

		}
		if !leftColIDs.Contains(leftColID) || !rightColIDs.Contains(rightColID) {
			// Columns don't come from both sides of join, left column is nullable or

[nit] I'd say ".. left column is nullable, or right column is filtered"

---

pkg/sql/opt/memo/multiplicity_builder.go, line 322 at r2 (raw file):

		}
		if fkTableID == lastSeen {
			// We have already encountered this TableID.

[nit] maybe add a note explaining that all columns from the same table will be clustered together

---

pkg/sql/opt/props/logical.go, line 341 at r2 (raw file):

		// MultiplicityProps is a struct that describes how rows from the input of
		// a join are affected by the join. Rows from the left or right input are
		// described as being duplicated, filtered or null-extended.

null-extended isn't included here anymore

---

pkg/sql/opt/props/multiplicity.go, line 37 at r2 (raw file):

)

// JoinMultiplicity answers queries about how a join will affect the rows from

Need to update this comment based on the fact that null-extended is no longer included above

---

pkg/sql/opt/props/multiplicity.go, line 62 at r2 (raw file):

// runtime, the duplicated and filtered flags must be set.
//
// When it is stored in the Relational of an operator other than a join,

[nit] "the Relational" sounds kind of weird -- maybe "the Relational properties"?

---

pkg/sql/opt/props/multiplicity.go, line 89 at r2 (raw file):

	//  SELECT * FROM xy FULL JOIN uv ON x=u;
	//
	// Duplicated: neither LeftMultiplicity nor RightMultiplicity would set the

[nit] make these names match the names above

---

pkg/sql/opt/props/multiplicity.go, line 96 at r2 (raw file):

	// will add back any rows that don't match on the filter conditions.
	//
	// Null-extended: since the FullJoin will add back null-extended rows on both

This can be removed

---

pkg/sql/opt/props/multiplicity.go, line 105 at r2 (raw file):

// JoinWillNotDuplicateLeftRows returns true when rows from the left input will
// not be included in the join output more than once.
func (mp *JoinMultiplicity) JoinWillNotDuplicateLeftRows() bool {

[nit] it's a bit odd to use the future tense -- I'd name this "JoinDoesNotDuplicateLeftRows"

---

pkg/sql/opt/props/multiplicity.go, line 115 at r2 (raw file):

}

// JoinPreservesLeftRows returns true when rows from the left input are

[nit] "when rows -> "when all rows" (to match the comment below)

---

pkg/sql/opt/props/multiplicity.go, line 152 at r2 (raw file):

	}

	if mp.JoinWillNotDuplicateLeftRows() {

[nit] I'd put this in a helper function (can be defined inside this function...) so you don't need to repeat the logic below.

---

pkg/sql/opt/props/multiplicity_test.go, line 19 at r2 (raw file):

)

// Example sql tables:

All of these examples are great, but I don't think they add a lot of value for this set of tests. Here it seems like you should just test that all the different combinations of multiplicity flags produce the correct output for the JoinWillNotDuplicate and JoinPreserves APIs.

It would be better to use these examples as data driven tests and make sure you get the expected multiplicity flags (if you haven't already tested something similar).

---

pkg/sql/opt/props/multiplicity_test.go, line 60 at r2 (raw file):

//  	LEFT JOIN xy AS xy1
//  	On xy.y = xy1.y
var leftSameColFiltered = JoinMultiplicity{

This is a duplicate test case.

---

pkg/sql/opt/props/multiplicity_test.go, line 68 at r2 (raw file):

// times. Rows that are not matched are added back.
//  SELECT * FROM xy FULL JOIN uv ON x = u
var fullPrimaryKey = JoinMultiplicity{

ditto

[DrewKimball]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @RaduBerinde and @rytaft)

---

pkg/sql/opt/memo/multiplicity_builder.go, line 23 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] that other than -> other than

Done.

---

pkg/sql/opt/memo/multiplicity_builder.go, line 85 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] empty line

Done.

---

pkg/sql/opt/memo/multiplicity_builder.go, line 189 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

what are meta columns?

I was trying to distinguish "query" columns and the underlying catalogue columns they come from. Now that I think about it, it might be better to just say ColumnIDs instead.

---

pkg/sql/opt/memo/multiplicity_builder.go, line 209 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

Is this true in the case of match simple? I think it's only match full....

That's a nice catch, you're right. I'll add a check to ensure that all foreign key columns must be not-null when the match method isn't match full. I'll also add some logic tests. This actually wasn't handled in the original code, either, so I'll open an issue for it.

---

pkg/sql/opt/memo/multiplicity_builder.go, line 258 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] I'd say ".. left column is nullable, or right column is filtered"

Done.

---

pkg/sql/opt/memo/multiplicity_builder.go, line 322 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] maybe add a note explaining that all columns from the same table will be clustered together

Done.

---

pkg/sql/opt/props/logical.go, line 341 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

null-extended isn't included here anymore

Done.

---

pkg/sql/opt/props/multiplicity.go, line 37 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

Need to update this comment based on the fact that null-extended is no longer included above

Done.

---

pkg/sql/opt/props/multiplicity.go, line 62 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] "the Relational" sounds kind of weird -- maybe "the Relational properties"?

Done.

---

pkg/sql/opt/props/multiplicity.go, line 89 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] make these names match the names above

Done.

---

pkg/sql/opt/props/multiplicity.go, line 96 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

This can be removed

Done.

---

pkg/sql/opt/props/multiplicity.go, line 105 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] it's a bit odd to use the future tense -- I'd name this "JoinDoesNotDuplicateLeftRows"

Done.

---

pkg/sql/opt/props/multiplicity.go, line 115 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] "when rows -> "when all rows" (to match the comment below)

Done.

---

pkg/sql/opt/props/multiplicity.go, line 152 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

[nit] I'd put this in a helper function (can be defined inside this function...) so you don't need to repeat the logic below.

Done.

---

pkg/sql/opt/props/multiplicity_test.go, line 19 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

All of these examples are great, but I don't think they add a lot of value for this set of tests. Here it seems like you should just test that all the different combinations of multiplicity flags produce the correct output for the JoinWillNotDuplicate and JoinPreserves APIs.

It would be better to use these examples as data driven tests and make sure you get the expected multiplicity flags (if you haven't already tested something similar).

Done.

---

pkg/sql/opt/props/multiplicity_test.go, line 60 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

This is a duplicate test case.

Done.

---

pkg/sql/opt/props/multiplicity_test.go, line 68 at r2 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

ditto

Done.

[rytaft]

Nice work!

Reviewed 9 of 9 files at r3.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @DrewKimball and @RaduBerinde)

---

pkg/sql/opt/memo/multiplicity_builder.go, line 209 at r2 (raw file):

Previously, DrewKimball (Andrew Kimball) wrote…

That's a nice catch, you're right. I'll add a check to ensure that all foreign key columns must be not-null when the match method isn't match full. I'll also add some logic tests. This actually wasn't handled in the original code, either, so I'll open an issue for it.

Great - thanks!

---

pkg/sql/opt/memo/multiplicity_builder.go, line 381 at r3 (raw file):

				// Add any valid foreign key relations to the mapping.
				if rightCols == nil {
					break

Shouldn't rightCols be the same length as leftCols? Do you need this break?

---

pkg/sql/opt/props/multiplicity.go, line 87 at r3 (raw file):

	//  SELECT * FROM xy FULL JOIN uv ON x=u;
	//
	// MultiplicityNotDuplicatedVal: both LeftMultiplicity nor RightMultiplicity

nor -> and

[DrewKimball]

Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @RaduBerinde and @rytaft)

---

pkg/sql/opt/memo/multiplicity_builder.go, line 381 at r3 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

Shouldn't rightCols be the same length as leftCols? Do you need this break?

It should, I just had that there to stop an annoying warning. I'll remove it.

---

pkg/sql/opt/props/multiplicity.go, line 87 at r3 (raw file):

Previously, rytaft (Rebecca Taft) wrote…

nor -> and

Whoops, done.

[rytaft]

Reviewed 3 of 3 files at r4.
Reviewable status: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @RaduBerinde)

[andy-kimball]

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)