diff --git a/pkg/sql/opt/props/ordering_choice.go b/pkg/sql/opt/props/ordering_choice.go index f199f6c08d7d..256747f4fd79 100644 --- a/pkg/sql/opt/props/ordering_choice.go +++ b/pkg/sql/opt/props/ordering_choice.go @@ -215,16 +215,21 @@ func (oc *OrderingChoice) FromOrdering(ord opt.Ordering) { // and with the given optional columns. Any optional columns in the given // ordering are ignored. func (oc *OrderingChoice) FromOrderingWithOptCols(ord opt.Ordering, optCols opt.ColSet) { - oc.Optional = optCols.Copy() oc.Columns = make([]OrderingColumnChoice, 0, len(ord)) for i := range ord { - if !oc.Optional.Contains(ord[i].ID()) { + if !optCols.Contains(ord[i].ID()) { oc.Columns = append(oc.Columns, OrderingColumnChoice{ Group: opt.MakeColSet(ord[i].ID()), Descending: ord[i].Descending(), }) } } + // If Columns is empty, then Optional must be as well. + if oc.Any() { + oc.Optional = opt.ColSet{} + } else { + oc.Optional = optCols.Copy() + } } // ToOrdering returns an opt.Ordering instance composed of the shortest possible @@ -430,8 +435,14 @@ func (oc *OrderingChoice) Intersection(other *OrderingChoice) OrderingChoice { for ; right < len(other.Columns); right++ { result = append(result, other.Columns[right]) } + var optional opt.ColSet + // If Columns is empty, then Optional must be as well. len(result) should + // never be zero here, but check anyway in case the logic changes. + if len(result) != 0 { + optional = oc.Optional.Intersection(other.Optional) + } return OrderingChoice{ - Optional: oc.Optional.Intersection(other.Optional), + Optional: optional, Columns: result, } } @@ -494,8 +505,13 @@ func (oc *OrderingChoice) CommonPrefix(other *OrderingChoice) OrderingChoice { right++ default: + var optional opt.ColSet + // If Columns is empty, then Optional must be as well. + if len(result) != 0 { + optional = oc.Optional.Intersection(other.Optional) + } return OrderingChoice{ - Optional: oc.Optional.Intersection(other.Optional), + Optional: optional, Columns: result, } } @@ -512,8 +528,13 @@ func (oc *OrderingChoice) CommonPrefix(other *OrderingChoice) OrderingChoice { Descending: rightCol.Descending, }) } + var optional opt.ColSet + // If Columns is empty, then Optional must be as well. + if len(result) != 0 { + optional = oc.Optional.Intersection(other.Optional) + } return OrderingChoice{ - Optional: oc.Optional.Intersection(other.Optional), + Optional: optional, Columns: result, } } @@ -805,6 +826,10 @@ func (oc *OrderingChoice) RestrictToCols(cols opt.ColSet) { } } } + // Normalize when OrderingChoice is Any. + if oc.Any() { + oc.Optional = opt.ColSet{} + } } // PrefixIntersection computes an OrderingChoice which: diff --git a/pkg/sql/opt/props/ordering_choice_test.go b/pkg/sql/opt/props/ordering_choice_test.go index 45bdedc3e17f..44728dea79e1 100644 --- a/pkg/sql/opt/props/ordering_choice_test.go +++ b/pkg/sql/opt/props/ordering_choice_test.go @@ -28,6 +28,11 @@ func TestOrderingChoice_FromOrdering(t *testing.T) { if exp, actual := "-2,+4 opt(1,3,5)", oc.String(); exp != actual { t.Errorf("expected %s, got %s", exp, actual) } + + oc.FromOrderingWithOptCols(opt.Ordering{1, 2, 3}, opt.MakeColSet(1, 2, 3)) + if exp, actual := "", oc.String(); exp != actual { + t.Errorf("expected %s, got %s", exp, actual) + } } func TestOrderingChoice_ToOrdering(t *testing.T) { @@ -155,6 +160,9 @@ func TestOrderingChoice_Intersection(t *testing.T) { {left: "+1", right: "+2 opt(2)", expected: "NO"}, {left: "+1", right: "-1 opt(2)", expected: "NO"}, {left: "+(1|2),+(3|4)", right: "+(2|5),+(6|7)", expected: "NO"}, + {left: "+1 opt(3)", right: "+2", expected: "NO"}, + {left: "+1", right: "+2 opt(3)", expected: "NO"}, + {left: "+1 opt(3)", right: "+2 opt(3)", expected: "NO"}, // Non-commutative cases. { @@ -236,6 +244,9 @@ func TestOrderingChoice_CommonPrefix(t *testing.T) { {left: "+1", right: "", expected: ""}, {left: "+1 opt(2)", right: "", expected: ""}, {left: "+1", right: "+1", expected: "+1"}, + {left: "+1 opt(3)", right: "+2", expected: ""}, + {left: "+1", right: "+2 opt(3)", expected: ""}, + {left: "+1 opt(3)", right: "+2 opt(3)", expected: ""}, {left: "+1,-2", right: "+1", expected: "+1"}, {left: "+1,-2", right: "+1,-2", expected: "+1,-2"}, {left: "+1", right: "+1 opt(2)", expected: "+1"}, @@ -614,7 +625,8 @@ func TestOrderingChoice_RestrictToCols(t *testing.T) { {s: "+1,+(2|3),-4 opt(5,6)", cols: []opt.ColumnID{1, 2, 4}, expected: "+1,+2,-4"}, {s: "+1,+(2|3),-4 opt(5,6)", cols: []opt.ColumnID{1, 4, 5, 6}, expected: "+1 opt(5,6)"}, {s: "+1,+(2|3),-4 opt(5,6)", cols: []opt.ColumnID{1, 3, 5}, expected: "+1,+3 opt(5)"}, - {s: "+1,+(2|3),-4 opt(5,6)", cols: []opt.ColumnID{2, 4, 5}, expected: "opt(5)"}, + {s: "+1,+(2|3),-4 opt(5,6)", cols: []opt.ColumnID{2, 4, 5}, expected: ""}, + {s: "+1,+(2|3),-4 opt(5,6)", cols: []opt.ColumnID{5, 6}, expected: ""}, } for _, tc := range testcases { diff --git a/pkg/sql/opt/xform/testdata/physprops/ordering b/pkg/sql/opt/xform/testdata/physprops/ordering index 88b9e31d8fb0..180e217d1562 100644 --- a/pkg/sql/opt/xform/testdata/physprops/ordering +++ b/pkg/sql/opt/xform/testdata/physprops/ordering @@ -1473,6 +1473,97 @@ distinct-on └── first-agg [as=b:2, outer=(2)] └── b:2 +# Regression test for #79644 +exec-ddl +CREATE TABLE t1 ( + id INT8 PRIMARY KEY, + col2 INT8 NOT NULL, + name STRING(1024), + UNIQUE (col2, name) +) +---- + +exec-ddl +CREATE TABLE t2 ( + col1 INT8 NOT NULL REFERENCES t1, + col2 INT4 NOT NULL, + PRIMARY KEY (col1, col2) +) +---- + +exec-ddl +CREATE TABLE t3 ( + col1 INT8 NOT NULL, + col2 INT4 NOT NULL, + col3 INT8 NOT NULL, + PRIMARY KEY (col1, col2, col3), + CONSTRAINT fk_t2 FOREIGN KEY (col1, col2) REFERENCES t2 (col1, col2) +) +---- + +# DISTINCT ON + GROUP BY + ORDER BY should not use the GROUP BY's empty +# OrderingChoice to try and fulfill the required ordering. +opt format=hide-stats +SELECT +DISTINCT ON (t2.col1) +t1.col2, t2.col1, array_agg(t3.col3) FROM t2 +INNER JOIN t1 ON t1.id = t2.col1 +INNER JOIN t3 ON t2.col2 = t3.col2 +GROUP BY (t2.col1, t1.col2, t2.col2) +ORDER BY (t2.col1, t2.col2) +---- +distinct-on + ├── columns: col2:6!null col1:1!null array_agg:15!null + ├── grouping columns: t2.col1:1!null + ├── internal-ordering: +2 opt(1,6) + ├── key: (1) + ├── fd: (1)-->(6,15) + ├── ordering: +1 + ├── sort + │ ├── columns: t2.col1:1!null t2.col2:2!null t1.col2:6!null array_agg:15!null + │ ├── key: (1,2) + │ ├── fd: (1)-->(6), (1,2)-->(6,15) + │ ├── ordering: +1,+2 + │ └── group-by (hash) + │ ├── columns: t2.col1:1!null t2.col2:2!null t1.col2:6!null array_agg:15!null + │ ├── grouping columns: t2.col1:1!null t2.col2:2!null + │ ├── key: (1,2) + │ ├── fd: (1)-->(6), (1,2)-->(6,15) + │ ├── inner-join (hash) + │ │ ├── columns: t2.col1:1!null t2.col2:2!null id:5!null t1.col2:6!null t3.col2:11!null col3:12!null + │ │ ├── multiplicity: left-rows(zero-or-more), right-rows(one-or-more) + │ │ ├── fd: (5)-->(6), (1)==(5), (5)==(1), (2)==(11), (11)==(2) + │ │ ├── inner-join (merge) + │ │ │ ├── columns: t2.col1:1!null t2.col2:2!null id:5!null t1.col2:6!null + │ │ │ ├── left ordering: +1 + │ │ │ ├── right ordering: +5 + │ │ │ ├── key: (2,5) + │ │ │ ├── fd: (5)-->(6), (1)==(5), (5)==(1) + │ │ │ ├── scan t2 + │ │ │ │ ├── columns: t2.col1:1!null t2.col2:2!null + │ │ │ │ ├── key: (1,2) + │ │ │ │ └── ordering: +1 + │ │ │ ├── scan t1 + │ │ │ │ ├── columns: id:5!null t1.col2:6!null + │ │ │ │ ├── key: (5) + │ │ │ │ ├── fd: (5)-->(6) + │ │ │ │ └── ordering: +5 + │ │ │ └── filters (true) + │ │ ├── scan t3 + │ │ │ └── columns: t3.col2:11!null col3:12!null + │ │ └── filters + │ │ └── t2.col2:2 = t3.col2:11 [outer=(2,11), fd=(2)==(11), (11)==(2)] + │ └── aggregations + │ ├── array-agg [as=array_agg:15, outer=(12)] + │ │ └── col3:12 + │ └── const-agg [as=t1.col2:6, outer=(6)] + │ └── t1.col2:6 + └── aggregations + ├── first-agg [as=t1.col2:6, outer=(6)] + │ └── t1.col2:6 + └── first-agg [as=array_agg:15, outer=(15)] + └── array_agg:15 + # -------------------------------------------------- # Set Operations. # --------------------------------------------------