Skip to content

repeat_row: Make subquery guard aware of negative diffs#36185

Merged
ggevay merged 1 commit intoMaterializeInc:mainfrom
ggevay:repeat_row-4-GuardSubquerySize
Apr 21, 2026
Merged

repeat_row: Make subquery guard aware of negative diffs#36185
ggevay merged 1 commit intoMaterializeInc:mainfrom
ggevay:repeat_row-4-GuardSubquerySize

Conversation

@ggevay
Copy link
Copy Markdown
Contributor

@ggevay ggevay commented Apr 21, 2026

This is the 4. PR in the repeat_row productionization work stream. Resolve SQL-164.

This resolves #32978 (comment). It was not important originally, but now that we are productionizing repeat_row, the original error message would become confusing: it would say that "more than one record produced in subquery", when in fact there was a negative number of rows.

I didn't add a test, because I wasn't able to find a query where this error deterministically happens. If I just naively put a repeat_row(-1) inside a subquery, then sometimes this error happens, but sometimes a Distinct gives the negative accumulation error message.

@ggevay ggevay changed the title Make subquery guard aware of negative diffs repeat_row: Make subquery guard aware of negative diffs Apr 21, 2026
@ggevay ggevay force-pushed the repeat_row-4-GuardSubquerySize branch from b1f7261 to 88ad442 Compare April 21, 2026 14:59
@ggevay ggevay force-pushed the repeat_row-4-GuardSubquerySize branch from 88ad442 to ac14239 Compare April 21, 2026 15:08
@ggevay ggevay marked this pull request as ready for review April 21, 2026 15:10
@ggevay ggevay requested a review from a team as a code owner April 21, 2026 15:10
@ggevay ggevay requested a review from frankmcsherry April 21, 2026 15:23
@ggevay ggevay added the A-CLUSTER Topics related to the CLUSTER layer label Apr 21, 2026
Copy link
Copy Markdown
Contributor

@frankmcsherry frankmcsherry left a comment

Choose a reason for hiding this comment

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

Seems fine. Personally, the error seems most accurately "Error: not exactly one row", and the different variants for negative vs more than one may not pay dividends. But .. not opposed other than wrt the structured error bloat.

@ggevay
Copy link
Copy Markdown
Contributor Author

ggevay commented Apr 21, 2026

The reason for making the negative case its own error is that we'd like to handle this case specially: The error message should say something like "this is an internal error, unless you are using repeat_row". Also, we might want to create an incident for this one if repeat_row is not enabled in the user's env, but never create an incident for the >1 case. (I'll do the error message tweak in a separate PR, together with all the other negative accumulation error msgs.)

@ggevay ggevay merged commit d7ba94d into MaterializeInc:main Apr 21, 2026
122 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-CLUSTER Topics related to the CLUSTER layer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants