Skip to content

[compute] Correct index permutation in subscribe snapshot optimization#34893

Merged
bkirwi merged 3 commits intoMaterializeInc:mainfrom
bkirwi:sub-fix
Feb 4, 2026
Merged

[compute] Correct index permutation in subscribe snapshot optimization#34893
bkirwi merged 3 commits intoMaterializeInc:mainfrom
bkirwi:sub-fix

Conversation

@bkirwi
Copy link
Contributor

@bkirwi bkirwi commented Feb 3, 2026

#34791 has a bug: it does not correctly permute the data from the arrangement to match the order of columns in the original relation type. The original set of tests did not catch this: I had tests only with default arrangements, which AFAICT include all columns in the original order by default.

  • Add a repro of the original bug, and some randomized testing which gives us automatic coverage of this case.
  • Fix the bug by applying the appropriate MFP. (Following closely the way the existing sink code makes collections from arrangements.)

Motivation

Previously-unreported bug. (Found during manual testing in staging. 😬)

Tips for reviewer

Here's a nightly that reproduces the issue (before the fix).

@bkirwi bkirwi force-pushed the sub-fix branch 2 times, most recently from 754268e to eb9c17f Compare February 3, 2026 02:55
@bkirwi bkirwi marked this pull request as ready for review February 3, 2026 04:43
@bkirwi bkirwi requested review from a team as code owners February 3, 2026 04:43
@bkirwi bkirwi requested a review from antiguru February 3, 2026 04:43
@bkirwi
Copy link
Contributor Author

bkirwi commented Feb 3, 2026

@def- - Made a small change to the parallel workload in case you want to take a look!

Also, my first Nightly run didn't repro this bug because it was caught by the ignore for this issue. I spot checked a couple builds and didn't find any recent examples; any easy way to check the logs and see if the original issue's cropped up recently?

Copy link
Contributor

@def- def- left a comment

Choose a reason for hiding this comment

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

Thanks, test changes lgtm

@def-
Copy link
Contributor

def- commented Feb 3, 2026

any easy way to check the logs and see if the original issue's cropped up recently?

Easiest way is just removing the ignore and seeing if it still falls over: #34897

We don't currently log expected query errors in parallel-workload, because it would lead to huge amounts of logs.

@bkirwi bkirwi merged commit b975410 into MaterializeInc:main Feb 4, 2026
154 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