planner: ExtractFD misuses null-reject proofs for not-null inference
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Bug Report
### 1. Minimal reproduce step (Required)
`ExtractFD` currently uses `IsNullRejected` in several places where the caller actually needs a different not-null proof. There are at least two distinct caller semantics:
1. Filter-derived column not-null inference: whether `condition = TRUE` implies a specific column is non-NULL.
2. Projection/Aggregation scalar output nullability: whether a scalar expression result is guaranteed to be non-NULL.
`IsNullRejected(ctx, schema, predicate)` proves a different property: after every column in `schema` is replaced with SQL NULL, `predicate` cannot evaluate to TRUE. That is correct for outer-join simplification against the null-producing side, but it is not directly a general not-null proof.
#### Case 1: filter not-null inference incorrectly uses whole-schema null-reject
Reproduction:
```sql
set @@session.tidb_enable_new_only_full_group_by_check = 'on';
create table x3(
a int not null primary key,
b int not null,
c int default null,
d int not null,
unique key I_b_c (b,c)
);
select a from x3 where c = 1 or d = 1 group by b,c;
```
The planner currently accepts the query. A local planner FD check on current master shows that `ExtractFD` upgrades the nullable unique key `(b,c)` into a strict FD:
```text
Plan: DataScan(x3)->Aggr(firstrow(test.x3.a))->Projection
FD: {(1)-->(2-5), (2,3)~~>(1,4,5)}
>>> {(1)-->(2-5), (2,3)-->(1,4,5)}
>>> {(1)-->(2,3), (2,3)-->(1)}
```
This comes from `ExtractNotNullFromConds`: it calls `IsNullRejected(p.SCtx(), p.Schema(), condition)` once for the whole schema and then marks every column referenced by the condition as not-null.
However, whole-schema null-reject does not imply each referenced column must be non-NULL when the predicate is TRUE. For `c = 1 OR d = 1`, replacing both `c` and `d` with NULL is null-rejected, but `c` itself is not required to be non-NULL because the predicate is TRUE for rows where `c IS NULL AND d = 1`.
Concrete data shape:
```sql
insert into x3 values (1, 10, null, 1), (2, 10, null, 1);
```
Both rows satisfy `c = 1 OR d = 1`, and `unique(b,c)` permits multiple rows with `c IS NULL`. Therefore `(b,c)` does not functionally determine `a` after filtering.
For this caller, the intended property should be checked per column, for example by proving null-rejection against a single-column schema for each referenced column.
#### Case 2: Projection/Aggregation scalar output nullability uses predicate null-reject
`LogicalProjection.ExtractFD` and `LogicalAggregation.ExtractFD` also call `IsNullRejected` to decide whether a scalar expression's output column should be marked not-null:
```go
notnull := util.IsNullRejected(p.SCtx(), p.Schema(), x)
if notnull || determinants.SubsetOf(fds.NotNullCols) {
notnullColsUniqueIDs.Insert(scalarUniqueID)
}
```
and similarly in `LogicalAggregation.ExtractFD`.
This is not the same property. For example, as a filter predicate, `a = 1` is null-rejected with respect to `a`: if `a` is NULL, the predicate cannot be TRUE. But as a projected scalar expression, `a = 1` can evaluate to NULL when `a` is NULL, so the output column is not guaranteed to be non-NULL.
Those callers need an expression-result nullability proof instead of a predicate null-reject proof. A conservative fix would avoid marking scalar outputs not-null from `IsNullRejected` and only use a helper that proves the expression result itself cannot be NULL.
### 2. What did you expect to see? (Required)
For the filter-derived FD case, the query should be rejected by `ONLY_FULL_GROUP_BY`, because `a` is not functionally dependent on `GROUP BY b,c` after the filter.
For Projection/Aggregation, scalar expression output columns should only be marked not-null when the expression result itself is guaranteed to be non-NULL. Predicate null-rejection should not be used as a substitute for output nullability.
### 3. What did you see instead (Required)
The filter case is accepted. `ExtractFD` treats `(b,c)` as a strict FD determinant and allows `a` to be selected with `GROUP BY b,c`.
Projection/Aggregation can also mark scalar expression outputs as not-null based on predicate null-rejection semantics, which is stronger than what those callers can safely conclude.
### 4. What is your TiDB version? (Required)
Observed on current master at:
```text
3c8816c0143f80a93a98c8b4f9d13a3f898cb0dc
```
Suggested labels: `type/bug`, `sig/planner`, `severity/major`, `affects-9.0`.
Contributor guide
Assessment
This issue has not been assessed yet.