Skip to content

[ss-159] Accept empty exclude columns for consistency with text columns#36922

Merged
peterdukelarsen merged 3 commits into
MaterializeInc:mainfrom
peterdukelarsen:pl/empty-exclude
Jun 8, 2026
Merged

[ss-159] Accept empty exclude columns for consistency with text columns#36922
peterdukelarsen merged 3 commits into
MaterializeInc:mainfrom
peterdukelarsen:pl/empty-exclude

Conversation

@peterdukelarsen

@peterdukelarsen peterdukelarsen commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Motivation

While working on #36890 I noticed that EXCLUDE COLUMNS has the same behavior as TEXT COLUMNS. For consistency's sake I've implemented the same fix.

Description

Removes panic/error log for empty exclude columns because we supported and want to support empty exclude columns.

Verification

Testdrive tests asserting legacy behavior aligns with expectations and new behavior matches it.

@peterdukelarsen peterdukelarsen requested a review from a team as a code owner June 5, 2026 19:15
@peterdukelarsen peterdukelarsen requested review from martykulma and removed request for a team and martykulma June 5, 2026 19:15
@peterdukelarsen peterdukelarsen marked this pull request as draft June 5, 2026 21:55
# `CREATE SOURCE .. FOR TABLES` (subsource-creating) syntax is treated as
# though the option were omitted and must succeed. This differs from the
# `CREATE TABLE .. FROM SOURCE` syntax, where an empty set is rejected with
# `invalid TEXT COLUMNS option value: cannot be empty`.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I caught some stale comments that slipped through on the old PR from when I was planning to make this an error. Generally not a fan of mixing changes, but it seemed related and trivial enough.

@peterdukelarsen peterdukelarsen marked this pull request as ready for review June 8, 2026 13:52

@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.

sweet - thanks @peterdukelarsen !

@peterdukelarsen peterdukelarsen merged commit 8d94c2a into MaterializeInc:main Jun 8, 2026
120 checks passed
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.

2 participants