cockroachdb / cockroachdb/cockroach

DROP USER fails when null_ordered_last is on

Open
#168,317 2 comments 0 reactions 0 assignees View on GitHub
C-bug O-support P-3 T-sql-queries
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.