fix(plan): preserve ENUM and SET types in views - #26604
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep-reviewed exact head bb144fc0c1c23c4b7439fc6f2a6359ee649b07c9. The earlier DISTINCT value-semantics blocker is fixed, but one systemic correctness issue remains.
P1: persisted ENUM/SET metadata is recovered from only one physical plan shape
At pkg/sql/plan/build_ddl.go:170-184, genViewTableDef derives the catalog type by applying mysqlSpecialTypeSourceType only to the final step's ProjectList. The helper at pkg/sql/plan/mysql_special_types.go:258-283 recognizes only the exact display-wrapper-over-ColRef shape. A transparent planner boundary turns that final expression into a VARCHAR ColRef, so the same source column silently gets a different persisted view type.
Minimal counterexample:
CREATE VIEW v_direct AS
SELECT priority, flags FROM nation;
CREATE VIEW v_order AS
SELECT priority, flags FROM nation ORDER BY priority, flags;Using the same ENUM/SET source definitions, I built both statements at this head and inspected CreateView.TableDef.Cols:
| view | MySQL 8.4.10 catalog | this head |
|---|---|---|
| direct projection | ENUM / SET | ENUM / SET |
| same projection + ORDER BY | ENUM / SET | VARCHAR / VARCHAR |
The same loss occurs for transparent GROUP BY, DISTINCT, derived-table, and CTE shapes. UNION ALL is the nearest useful negative control: MySQL exposes VARCHAR there, so unconditionally propagating the type through every boundary would also be wrong.
This violates the view-boundary invariant of the PR: persist the special type iff the visible output remains a transparent projection of the same ENUM/SET definition under MySQL semantics; semantic string expressions and set-operation outputs must clear it.
Please carry explicit, narrow source-type provenance through the planner boundaries that preserve it and consume that provenance when generating the view schema, rather than rediscovering it from only the final expression shape. Please also add public/catalog assertions (for example SHOW COLUMNS or information_schema.columns) for direct, ORDER BY, GROUP BY, DISTINCT, and derived-table projections, with UNION/mixed expressions as negative controls. The current DISTINCT regression checks visible row count, but not its persisted column metadata.
Validation performed:
- exact-head planner probe across direct/GROUP BY/DISTINCT/ORDER BY/derived/CTE/UNION shapes;
- MySQL 8.4.10 behavior used as the oracle for the same DDL;
- all seven PR-focused
pkg/sql/plantests pass locally; git diff --checkpasses.
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep-reviewed exact head 037e84666da1cd35f2e5ab7ad603acd7aa97c952. The previous catalog-shape blocker is only partially fixed; one end-to-end P1 remains.
P1: transparent derived/CTE view provenance is not restored on rebind, so CTAS still loses SET/ENUM
The new provenance path makes the persisted view column special, but appendViewMySQLSpecialTypeBoundary at pkg/sql/plan/query_builder.go:8903 and :8927 still recognizes only an exact display-wrapper expression. A transparent derived table or CTE ends in a VARCHAR ColRef with mysqlSpecialColumnTypes provenance, so view rebind does not restore the raw ENUM/SET type at the completed view boundary.
Minimal public-path counterexample:
CREATE VIEW v_derived AS
SELECT flags FROM (SELECT id, flags FROM t) d WHERE id = 2;
CREATE TABLE copied_derived AS SELECT flags FROM v_derived;At this head:
information_schema.columnsreportsv_derived.flagsasSET;copied_derived.flagsisVARCHAR, notSET;- the same failure occurs through a CTE view.
MySQL 8.4.10 produces SET for both the view and the CTAS table. This is also the downstream contract of issue #26226, not just a catalog-display difference.
Please consume the same narrow provenance when restoring the completed view boundary (or otherwise carry it into CTAS), while continuing to clear it for UNION and semantic string expressions. Add an end-to-end CTAS assertion for derived and CTE views, with UNION as the negative control; the current tests assert only the view catalog type for these shapes.
Validation performed:
- all seven PR-focused
pkg/sql/plantests pass; - exact-head embedded SQL regression fails with
expected SET, actual VARCHARforcopied_derived; - MySQL 8.4.10 used as the oracle for the same DDL;
git diff --checkpasses.
XuPeng-SH
left a comment
There was a problem hiding this comment.
Deep-reviewed exact head 630e72a31e0988b5bbce89319721df9fa56d3b49. The previous derived/CTE catalog and CTAS blockers are fixed, but one end-to-end P1 remains.
P1: an ORDER BY view corrupts a non-injective SET value at the new type-restoration boundary
bindView now calls appendMySQLSpecialTypeBoundary after the complete view plan (pkg/sql/plan/query_builder.go:8889). When the root is a SORT, canExposeRaw is false (:8915-8916), so the fallback takes the SQL-visible VARCHAR and calls funcCastForSetType to recreate a SET value (:8942-8964). That bitmap -> display string -> bitmap round trip is not reversible for legal SET definitions.
Public-path reproducer:
CREATE TABLE t (id INT PRIMARY KEY, flags SET('', 'a'));
INSERT INTO t VALUES (1, ''), (2, 1);
CREATE VIEW v_order AS SELECT id, flags FROM t ORDER BY flags;
SELECT CAST(flags AS UNSIGNED) FROM t WHERE id = 2; -- 1
SELECT CAST(flags AS UNSIGNED) FROM v_order WHERE id = 2; -- 0 on this headI reproduced the expected 0x1, actual 0x0 result in the PR embedded-cluster regression by adding only the v_order value assertion. The existing test already creates the same non-injective SET and an ORDER BY view, but checks only its catalog type, so this value corruption is currently missed. The unchanged PR test passes.
This is the same boundary invariant from the earlier DISTINCT fix viewed from the other side: semantic operators must consume the visible value, but a row-preserving operator must not force reconstruction of the stored ENUM/SET value from that visible value afterward. Please carry the original raw value through row-preserving boundaries (including the same derived/CTE shapes) instead of reverse-casting the display string, and add bitmap assertions for ORDER BY plus its transparent nested variants while retaining the DISTINCT visible-value regression.
Validation performed on the exact head:
- full
./pkg/sql/plantests pass; - the unchanged
TestIssue26226ViewDistinctUsesVisibleSetValuepasses; go list,go build, andgo vetfor the affected packages pass;git diff --checkpasses.
|
Deep-reviewed exact head P1: identical
|
|
Deep-reviewed exact head P1:
|
|
Deep-reviewed exact head One deterministic P1 remains in the newly added public regression itself. P1:
|
|
Deep-reviewed exact head P1: CTAS now leaks source defaults through derived/CTE boundaries and emits a replay-unsafe schema
Public reproducer: CREATE TABLE src (
id INT PRIMARY KEY,
e ENUM('low','medium','high') DEFAULT 'medium',
s SET('', 'a', 'b') DEFAULT 'a',
n INT DEFAULT 7
);
INSERT INTO src VALUES (1, 'low', '', 1);
CREATE TABLE c_derived AS
SELECT e,s,n FROM (SELECT e,s,n FROM src) d;
CREATE TABLE c_cte AS
WITH d AS (SELECT e,s,n FROM src) SELECT e,s,n FROM d;At this head, `e` enum('low','medium','high') DEFAULT 'medium',
`s` set('','a','b') DEFAULT 'a',
`n` int DEFAULT 7The same public test passes on clean base The head is internally inconsistent as well: despite the advertised defaults, Please separate special-type provenance from default inheritance, or clear only default provenance at derived/CTE boundaries while retaining the required ENUM/SET type metadata. The public regression should assert Validation on this exact head:
|
XuPeng-SH
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head. The prior CTAS default leak is fixed at the right abstraction boundary: source provenance remains available for ENUM/SET type restoration, while default inheritance is explicitly disabled when crossing derived-table/CTE query boundaries. Direct projections still inherit defaults, and semantic/set boundaries remain isolated. I also validated direct ORDER BY/GROUP BY/DISTINCT, nested derived tables/CTEs, a multi-reference CTE, and the public issue regression; all passed.
Merge Queue Status
This pull request spent 21 minutes 32 seconds in the queue, with no time running CI. ReasonPull request #26604 has been dequeued Queue conditions are not satisfied:
HintYou should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. Tick the box to put this pull request back in the merge queue (same as
|
What type of PR is this?
Which issue(s) this PR fixes:
issue #26226
What this PR does / why we need it:
Preserve the original ENUM and SET type metadata when a view directly projects those columns.
At view creation, persisted catalog columns retain their complete special-type definitions. When the saved view SQL is rebound, the planner restores the raw ENUM/SET index expression at the view output boundary. Outer queries therefore keep string display results while ORDER BY uses definition order and CTAS recreates ENUM/SET columns.
The shared source-expression validation also replaces the previous CTAS type assertion with a safe fallback for non-transparent or malformed expressions.
Tests cover persisted view metadata, full view rebind, ENUM ordering keys, CTAS from a view, ENUM, SET, explicit view column names, ordinary VARCHAR controls, and malformed expression fallback.