Skip to content

Commit

Permalink
Merge #25184 #25473
Browse files Browse the repository at this point in the history 
25184: cluster: remove legacy version upgrade tests for auto upgrade merge r=windchan7 a=windchan7

Legacy tests such as `TestClusterVersionUpgrade1_0To2_0`,
`TestDockerReadWriteBidirectionalReferenceVersion`, and
`TestDockerReadWriteForwardReferenceVersion` no longer applies.

Therefore, we remove and they will be replaced with new roachtests
when the auto upgrade PR #24987 merges.

Release note: None

25473: opt/optbuilder: Fix bug with order by, aggregate alias, and having. r=rytaft a=rytaft

Previously, queries such as:
`SELECT SUM(a) AS a2 FROM abcd GROUP BY c HAVING SUM(a)=10
ORDER BY a2`

were incorrectly causing an error:
`error: column name "a2" not found`

This commit fixes the error by ensuring that a column alias for an
aggregate function is applied even when the same aggregate appears
multiple times in the same query (e.g., in the HAVING clause).

Release note: None

Co-authored-by: Victor Chen <victor@cockroachlabs.com>
Co-authored-by: Rebecca Taft <becca@cockroachlabs.com>
  • Loading branch information
@windchan7 @rytaft
3 people committed May 14, 2018
3 parents 38d2a55 + 05290b9 + 0402f4e commit 937e4e4
Show file tree
Hide file tree
Showing 4 changed files with 228 additions and 148 deletions.
148 changes: 0 additions & 148 deletions pkg/acceptance/reference_test.go
View file

This file was deleted.

5 changes: 5 additions & 0 deletions pkg/sql/opt/optbuilder/groupby.go
View file
Expand Up @@ -377,6 +377,11 @@ func (b *Builder) buildAggregateFunction(
// Undo the adding of the args.
// TODO(radu): is there a cleaner way to do this?
aggInScope.cols = aggInScope.cols[:aggInScopeColsBefore]

// Update the column name if a label is provided.
if label != "" {
col.name = tree.Name(label)
}
}

// Replace the function call with a reference to the column.
Expand Down
75 changes: 75 additions & 0 deletions pkg/sql/opt/optbuilder/testdata/aggregate
View file
Expand Up @@ -2747,3 +2747,78 @@ sort
│ └── variable: kv.k [type=int]
└── projections
└── variable: mk [type=int]

build
SELECT MAX(k) AS mk1, MAX(k) AS mk2 FROM kv GROUP BY v ORDER BY mk1
----
sort
├── columns: mk1:5(int) mk2:5(int)
├── ordering: +5
└── project
├── columns: mk1:5(int)
├── group-by
│ ├── columns: kv.v:2(int) mk1:5(int)
│ ├── grouping columns: kv.v:2(int)
│ ├── project
│ │ ├── columns: kv.v:2(int) kv.k:1(int!null)
│ │ ├── scan kv
│ │ │ └── columns: kv.k:1(int!null) kv.v:2(int) kv.w:3(int) kv.s:4(string)
│ │ └── projections
│ │ ├── variable: kv.v [type=int]
│ │ └── variable: kv.k [type=int]
│ └── aggregations
│ └── max [type=int]
│ └── variable: kv.k [type=int]
└── projections
└── variable: mk1 [type=int]

build
SELECT MAX(k) AS mk1, MAX(k) AS mk2 FROM kv GROUP BY v ORDER BY mk2
----
sort
├── columns: mk1:5(int) mk2:5(int)
├── ordering: +5
└── project
├── columns: mk1:5(int)
├── group-by
│ ├── columns: kv.v:2(int) mk1:5(int)
│ ├── grouping columns: kv.v:2(int)
│ ├── project
│ │ ├── columns: kv.v:2(int) kv.k:1(int!null)
│ │ ├── scan kv
│ │ │ └── columns: kv.k:1(int!null) kv.v:2(int) kv.w:3(int) kv.s:4(string)
│ │ └── projections
│ │ ├── variable: kv.v [type=int]
│ │ └── variable: kv.k [type=int]
│ └── aggregations
│ └── max [type=int]
│ └── variable: kv.k [type=int]
└── projections
└── variable: mk1 [type=int]

build
SELECT MAX(k) AS mk1, MAX(k)/5 AS mk2 FROM kv GROUP BY v ORDER BY mk2
----
sort
├── columns: mk1:5(int) mk2:6(decimal)
├── ordering: +6
└── project
├── columns: mk1:5(int) mk2:6(decimal)
├── group-by
│ ├── columns: kv.v:2(int) mk1:5(int)
│ ├── grouping columns: kv.v:2(int)
│ ├── project
│ │ ├── columns: kv.v:2(int) kv.k:1(int!null)
│ │ ├── scan kv
│ │ │ └── columns: kv.k:1(int!null) kv.v:2(int) kv.w:3(int) kv.s:4(string)
│ │ └── projections
│ │ ├── variable: kv.v [type=int]
│ │ └── variable: kv.k [type=int]
│ └── aggregations
│ └── max [type=int]
│ └── variable: kv.k [type=int]
└── projections
├── variable: mk1 [type=int]
└── div [type=decimal]
├── variable: mk1 [type=int]
└── const: 5 [type=int]
148 changes: 148 additions & 0 deletions pkg/sql/opt/optbuilder/testdata/having
View file
Expand Up @@ -314,3 +314,151 @@ select
└── gt [type=bool]
├── variable: column6 [type=int]
└── const: 1 [type=int]

# Check that ordering by an alias of an aggregate works when HAVING is present.
build
SELECT SUM(k) AS mk FROM kv GROUP BY v HAVING SUM(k)=10 ORDER BY mk
----
sort
├── columns: mk:5(decimal)
├── ordering: +5
└── project
├── columns: column5:5(decimal)
├── select
│ ├── columns: kv.v:2(int) column5:5(decimal)
│ ├── group-by
│ │ ├── columns: kv.v:2(int) column5:5(decimal)
│ │ ├── grouping columns: kv.v:2(int)
│ │ ├── project
│ │ │ ├── columns: kv.v:2(int) kv.k:1(int!null)
│ │ │ ├── scan kv
│ │ │ │ └── columns: kv.k:1(int!null) kv.v:2(int) kv.w:3(int) kv.s:4(string)
│ │ │ └── projections
│ │ │ ├── variable: kv.v [type=int]
│ │ │ └── variable: kv.k [type=int]
│ │ └── aggregations
│ │ └── sum [type=decimal]
│ │ └── variable: kv.k [type=int]
│ └── eq [type=bool]
│ ├── variable: column5 [type=decimal]
│ └── const: 10 [type=decimal]
└── projections
└── variable: column5 [type=decimal]

build
SELECT SUM(k) AS mk FROM kv GROUP BY v HAVING MAX(k) > 10 ORDER BY mk
----
sort
├── columns: mk:6(decimal)
├── ordering: +6
└── project
├── columns: mk:6(decimal)
├── select
│ ├── columns: kv.v:2(int) column5:5(int) mk:6(decimal)
│ ├── group-by
│ │ ├── columns: kv.v:2(int) column5:5(int) mk:6(decimal)
│ │ ├── grouping columns: kv.v:2(int)
│ │ ├── project
│ │ │ ├── columns: kv.v:2(int) kv.k:1(int!null)
│ │ │ ├── scan kv
│ │ │ │ └── columns: kv.k:1(int!null) kv.v:2(int) kv.w:3(int) kv.s:4(string)
│ │ │ └── projections
│ │ │ ├── variable: kv.v [type=int]
│ │ │ └── variable: kv.k [type=int]
│ │ └── aggregations
│ │ ├── max [type=int]
│ │ │ └── variable: kv.k [type=int]
│ │ └── sum [type=decimal]
│ │ └── variable: kv.k [type=int]
│ └── gt [type=bool]
│ ├── variable: column5 [type=int]
│ └── const: 10 [type=int]
└── projections
└── variable: mk [type=decimal]

build
SELECT SUM(k) AS mk FROM kv GROUP BY v HAVING v > 10 ORDER BY mk
----
sort
├── columns: mk:5(decimal)
├── ordering: +5
└── project
├── columns: mk:5(decimal)
├── select
│ ├── columns: kv.v:2(int) mk:5(decimal)
│ ├── group-by
│ │ ├── columns: kv.v:2(int) mk:5(decimal)
│ │ ├── grouping columns: kv.v:2(int)
│ │ ├── project
│ │ │ ├── columns: kv.v:2(int) kv.k:1(int!null)
│ │ │ ├── scan kv
│ │ │ │ └── columns: kv.k:1(int!null) kv.v:2(int) kv.w:3(int) kv.s:4(string)
│ │ │ └── projections
│ │ │ ├── variable: kv.v [type=int]
│ │ │ └── variable: kv.k [type=int]
│ │ └── aggregations
│ │ └── sum [type=decimal]
│ │ └── variable: kv.k [type=int]
│ └── gt [type=bool]
│ ├── variable: kv.v [type=int]
│ └── const: 10 [type=int]
└── projections
└── variable: mk [type=decimal]

build
SELECT MAX(k) AS mk1, MAX(k) AS mk2 FROM kv GROUP BY v HAVING MAX(k) > 10 ORDER BY mk1
----
sort
├── columns: mk1:5(int) mk2:5(int)
├── ordering: +5
└── project
├── columns: column5:5(int)
├── select
│ ├── columns: kv.v:2(int) column5:5(int)
│ ├── group-by
│ │ ├── columns: kv.v:2(int) column5:5(int)
│ │ ├── grouping columns: kv.v:2(int)
│ │ ├── project
│ │ │ ├── columns: kv.v:2(int) kv.k:1(int!null)
│ │ │ ├── scan kv
│ │ │ │ └── columns: kv.k:1(int!null) kv.v:2(int) kv.w:3(int) kv.s:4(string)
│ │ │ └── projections
│ │ │ ├── variable: kv.v [type=int]
│ │ │ └── variable: kv.k [type=int]
│ │ └── aggregations
│ │ └── max [type=int]
│ │ └── variable: kv.k [type=int]
│ └── gt [type=bool]
│ ├── variable: column5 [type=int]
│ └── const: 10 [type=int]
└── projections
└── variable: column5 [type=int]

build
SELECT MAX(k) AS mk1, MAX(k) AS mk2 FROM kv GROUP BY v HAVING MAX(k) > 10 ORDER BY mk2
----
sort
├── columns: mk1:5(int) mk2:5(int)
├── ordering: +5
└── project
├── columns: column5:5(int)
├── select
│ ├── columns: kv.v:2(int) column5:5(int)
│ ├── group-by
│ │ ├── columns: kv.v:2(int) column5:5(int)
│ │ ├── grouping columns: kv.v:2(int)
│ │ ├── project
│ │ │ ├── columns: kv.v:2(int) kv.k:1(int!null)
│ │ │ ├── scan kv
│ │ │ │ └── columns: kv.k:1(int!null) kv.v:2(int) kv.w:3(int) kv.s:4(string)
│ │ │ └── projections
│ │ │ ├── variable: kv.v [type=int]
│ │ │ └── variable: kv.k [type=int]
│ │ └── aggregations
│ │ └── max [type=int]
│ │ └── variable: kv.k [type=int]
│ └── gt [type=bool]
│ ├── variable: column5 [type=int]
│ └── const: 10 [type=int]
└── projections
└── variable: column5 [type=int]

0 comments on commit 937e4e4

Please sign in to comment.