cockroachdb / cockroachdb/cockroach
DROP USER fails when null_ordered_last is on
- Dominant language
- Go
- Stars
- 32.5k
- Forks
- 4.1k
- PR merge metrics
- PR metrics pending
Description
**Describe the problem**
`DROP USER` (and any query using `SELECT DISTINCT ... ORDER BY`) fails with the following error when the session setting `null_ordered_last` is set to on:
`ERROR: drop-role-get-system-privileges: for SELECT DISTINCT, ORDER BY expressions must appear in select list`
In the optbuilder, when `null_ordered_last` = on, the `buildOrderBy` code synthesizes extra IS NULL columns to implement non-default null ordering. These synthesized columns are added to `inScope.ordering`, but `constructDistinct` rejects any ordering column whose ID is not in the visible select list (GroupingCols). The `DISTINCT ON` code path already handles this case (Case 3 in `buildDistinctOn`), but `constructDistinct` for plain `SELECT DISTINCT` does not.
This was reported on 25.2.13 (LTS) but is reproducible against master.
**To Reproduce**
```
SET null_ordered_last = on;
CREATE USER IF NOT EXISTS testuser;
DROP USER testuser;
```
See: `ERROR: drop-role-get-system-privileges: for SELECT DISTINCT, ORDER BY expressions must appear in select list`
**Expected behavior**
`DROP USER` (or any other `SELECT DISTINCT ... ORDER BY` query) should succeed regardless of the `null_ordered_last` setting.
**Additional data / screenshots**
The internal query triggering the error during `DROP USER` is in drop_role.go:
```
SELECT DISTINCT username, path, privileges
FROM system.privileges
WHERE username = ANY($1)
ORDER BY 1, 2
```
Exec flow:
1. When `null_ordered_last` = on, `hasDefaultNullsOrder` returns false for ascending columns with no explicit NULLS clause.
2. `buildOrderBy` synthesizes IS NULL columns prepended before each non-default-nulls ORDER BY column (e.g., ORDER BY 1, 2 becomes effectively ORDER BY (col1 IS NULL), col1, (col2 IS NULL), col2).
3. `constructDistinct` checks that every column in `inScope.ordering` is in `GroupingCols` (visible select-list columns). The synthesized IS NULL columns are not, which hits the error.
Potential fix:
I tested the following change in distinct.go to add similar logic to `constructDistinct` that's already used in Case 3 of `buildDistinctOn` (support using NULLS LAST with DISTINCT ON) and this appeared to fix the issue:
Before:
```
for _, col := range inScope.ordering {
if !private.GroupingCols.Contains(col.ID()) {
panic(pgerror.Newf(
pgcode.InvalidColumnReference,
"for SELECT DISTINCT, ORDER BY expressions must appear in select list",
))
}
}
```
After:
```
for _, col := range inScope.ordering {
if !private.GroupingCols.Contains(col.ID()) {
colIsValid := false
scopeCol := inScope.getColumn(col.ID())
if scopeCol != nil {
if isExpr, ok := scopeCol.scalar.(*memo.IsExpr); ok {
if _, ok := isExpr.Right.(*memo.NullExpr); ok {
if v, ok := isExpr.Left.(*memo.VariableExpr); ok {
if private.GroupingCols.Contains(v.Col) {
private.GroupingCols.Add(col.ID())
colIsValid = true
}
}
}
}
}
if !colIsValid {
panic(pgerror.Newf(
pgcode.InvalidColumnReference,
"for SELECT DISTINCT, ORDER BY expressions must appear in select list",
))
}
}
}
```
Jira issue: CRDB-62900
Contributor guide
Assessment
This issue has not been assessed yet.