Skip to content

SS-496 add option ALL COLUMNS NULLABLE to postgres CTFS - #39680

Merged
patrickwwbutler merged 6 commits into
mainfrom
patrick/pg-nullable
Oct 14, 2026
Merged

patrickwwbutler merged 6 commits into
mainfrom
patrick/pg-nullable

Conversation

@patrickwwbutler

Copy link
Copy Markdown
Contributor

Adds a new WITH option ALL COLUMNS NULLABLE to the Postgres CREATE TABLE FROM SOURCE syntax, allowing users to specify that any NOT NULL constraints should be ignored, are droppable, and the corresponding Materialize columns are nullable.

Adds sql-parser ddl test cases, and adds to the pg-cdc/exclude-constraints.td testdrive file with cases covering this logic.

@patrickwwbutler
patrickwwbutler requested a review from a team October 8, 2026 18:13
@patrickwwbutler
patrickwwbutler requested a review from a team as a code owner October 8, 2026 18:13
@linear-code

linear-code Bot commented Oct 8, 2026

Copy link
Copy Markdown

SS-496

@def-

def- commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. MEDIUM -- ALL COLUMNS NULLABLE is silently accepted and ignored for MySQL, SQL Server, Kafka and load-generator sources

src/sql/src/pure.rs:1931

purify_create_table_from_source only passes all_columns_nullable to the Postgres branch and never rejects it for other source types, so CREATE TABLE ... FROM SOURCE <mysql_source> (...) WITH (ALL COLUMNS NULLABLE) succeeds, keeps every upstream NOT NULL, and SHOW CREATE TABLE still shows the option. A user who opted in to survive an upstream DROP NOT NULL gets the permanent table stall the option is meant to prevent, and only finds out when it happens.

Details

The sibling options are already guarded: EXCLUDE CONSTRAINTS / EXCLUDE ALL CONSTRAINTS bail with "EXCLUDE CONSTRAINTS is not supported for {} sources" for anything except Postgres and MySQL (src/sql/src/pure.rs:1972-1983). The new flag has no matching check, the MySQL / SQL Server / Kafka / load-generator branches never read it, and plan_create_table_from_source drops it (all_columns_nullable: _, src/sql/src/plan/statement/ddl.rs:1789). humanize_sql_for_show_create keeps it (src/sql/src/plan/statement/show.rs:1172), so the stored and displayed SQL claim it is in effect.

For MySQL this has a direct consequence: MySqlColumnDesc::get_incompatible_schema_change returns NotNullDropped when a stored non-nullable column becomes nullable upstream (src/mysql-util/src/desc.rs:330). The table errors permanently, and the recovery hint then says to recreate it with EXCLUDE ALL CONSTRAINTS (src/source-schema-change/src/lib.rs:98). enable_exclude_constraints_option defaults to true (src/sql/src/session/vars/definitions.rs:2276), so the option can be used without changing any flag.

Fix: reject the option for non-Postgres connections next to the existing EXCLUDE CONSTRAINTS guard (or implement it in mysql::purify_source_exports, which already handles FilterConstraints::ExcludeAll the same way), and add a negative testdrive case for a non-Postgres source.

Comment thread src/sql/src/pure/postgres.rs Outdated
Comment thread src/sql/src/pure/postgres.rs
@def-

def- commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

QA LLM Review

1. HIGH -- The break 'c to continue 'c change re-lands the reverted #39598 for source exports, so ingestion replicas crash-loop after upgrade

src/sql/src/plan/statement/ddl.rs:1557

Existing subsources and tables-from-source are re-planned from their stored create_sql at boot. Any export whose constraints had a nullable non-NND UNIQUE ahead of an all-NOT-NULL UNIQUE now gets an extra key. That RelationDesc no longer equals the schema registered on its persist shard, so after an upgrade the replica that ingests it panics and restarts in a loop. This is the same change #39660 reverted for exactly this crash.

Details

Persist stores the whole ProtoRelationDesc as the shard schema, keys included (src/storage-types/src/sources.rs:1147). It finds a writer's schema by equality (src/persist-client/src/internal/state.rs:1622) and returns None once a different schema is registered. Source exports are not schema-evolved at bootstrap. evolve_nullability_for_bootstrap only covers compute collections (src/adapter/src/coord.rs:3486). persist_sink opens its writer with the re-planned desc (src/storage/src/render/persist_sink.rs:534), and the append then hits assert!(..., "fetching a schema id should only fail when the shard is tombstoned") at src/persist-client/src/write.rs:746.

A common upstream shape is enough. Take CREATE TABLE s (id int PRIMARY KEY, a int UNIQUE, b int NOT NULL UNIQUE) with an existing CREATE TABLE s FROM SOURCE pg (REFERENCE s). Constraints are emitted PK first, then in OID order (src/sql/src/pure/postgres.rs:317), so keys change from [[id]] to [[id],[b]]. MySQL and SQL Server exports and legacy FOR TABLES subsources take the same path (ddl.rs:1628). In 0dt the failure only shows after promotion, because read-only sources do not append.

The new option does not need this change. Postgres always puts the PK first, so break 'c cannot hide it, and the testdrive expectations are the same either way. Fix: revert the ddl.rs hunk and keep break 'c. Fixing the dropped constraints needs a catalog migration or a per-export flag that keeps existing descs stable, as the #39660 description says.

Comment thread test/pg-cdc/exclude-constraints.td Outdated

@martykulma martykulma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have a SHOW CREATE TABLE round trip test as well.

Otherwise, lgtm!

Comment thread src/sql/src/pure.rs
@patrickwwbutler
patrickwwbutler merged commit 012cf0a into main Oct 14, 2026
82 checks passed
@patrickwwbutler
patrickwwbutler deleted the patrick/pg-nullable branch October 14, 2026 22:36
peterdukelarsen added a commit that referenced this pull request Oct 16, 2026
### Motivation

#39680 added `EXCLUDE ALL NOT NULL CONSTRAINTS` to `CREATE TABLE .. FROM
SOURCE`. The parser, option plumbing, and feature flag gate are
source-agnostic, but only the Postgres purifier applied it, so MySQL
tables silently kept their `NOT NULL` constraints.

### Description

Thread the flag through the MySQL purifier and mark every column
nullable, matching the Postgres behavior. The unsupported-source guard
in `purify_create_table_from_source` now also covers this option, so
source types without support reject it instead of ignoring it.

Columns of a retained primary key stay NOT NULL. The planner declares
them NOT NULL regardless of the purified desc, so marking them nullable
disabled the decoder's NULL check for a column persist treats as
non-nullable. MySQL schema verification compares keys by name and
columns, so a key dropped and restored while the source was behind is
not reported, and a NULL written in between panicked clusterd. Postgres
compares keys by constraint OID, so a recreated key is reported before
any such row is decoded, and its purifier is unchanged.

### Verification

Extends `test/mysql-cdc/exclude-constraints.td` with the same cases
#39680 added for Postgres: `SHOW CREATE` roundtrip, index key, column
nullability, the combination with `EXCLUDE CONSTRAINTS ('PRIMARY')`,
`DROP NOT NULL` as a non-event, and PRIMARY KEY and UNIQUE drops still
erroring with the hint. Also adds the drop-and-restore-while-paused
scenario from the review, which panicked clusterd before the second
commit.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants