fix(stream): add assertion to ensure pk indices consistency #12763
Conversation
stdrc
force-pushed
the
rc/fix-pk-indices
branch
from
94e66df
to
8efc56c
Compare
Codecov Report
@@ Coverage Diff @@
## main #12763 +/- ##
==========================================
+ Coverage 69.17% 69.25% +0.08%
==========================================
Files 1493 1493
Lines 246462 246466 +4
==========================================
+ Hits 170501 170701 +200
+ Misses 75961 75765 -196
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 19 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
You may need to rebase this branch. :) |
Signed-off-by: Richard Chien <[email protected]>
stdrc
force-pushed
the
rc/fix-pk-indices
branch
from
8efc56c
to
0169a00
Compare
Done. BTW, for others' information, although this is a fix, I guess we'd better not cherrypick this PR to v1.3-rc, as there may be cases not covered by our e2e tests. Let's wait for one release cycle to see if there is any. |
Signed-off-by: Richard Chien <[email protected]>
| @@ -197,7 +196,7 @@ impl ExecutorBuilder for SourceExecutorBuilder { | |||
| (0..source.columns.len()).collect_vec(), // eliminate the last column (_rw_offset) | |||
| None, | |||
| schema.clone(), | |||
| pk_indices, | |||
There was a problem hiding this comment.
This is to fix https://buildkite.com/risingwavelabs/pull-request/builds/33652#018b407f-56a9-455f-bccf-9f0fc8eb6998. Plz help take a look @StrikeW.
There was a problem hiding this comment.
So for a source that has defined pk, does params.pk_indices contains the index to the primary key or it is empty?
There was a problem hiding this comment.
Currently params.pk_indices, which is derived by optimizer, is empty. So basically this change only ensures executor use the same pk_indices as optimizer, the actual content of pk_indices doesn't matter, because the Materialize node at downstream has correct primary key. (I just want to keep changes minimal)
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
As title.
Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.