release-19.2: changefeedccl: add scan boundaries based on change in set of columns #42669
Conversation
|
This change is |
|
@ajwerner what was our decision here? hold off on merging this in 19.2.2 and get it into 19.2.3? |
|
Totally dropped the ball on this one, looking now. |
|
You're fine, I'm also a reviewer. I was intentionally waiting because I thought we were going to hold off on merging this, but dropped the ball on putting that fact down here |
There was a problem hiding this comment.
What's going on w.r.t. the build here?
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @ajwerner and @danhhz)
|
|
There was a problem hiding this comment.
I'm cool with this change even going in to 19.2.2 if there's no drama getting it to build. This change is certainly a behavior change and seems like the riskiest of the backports we plan to actually do but it's also a good change.
Reviewed 4 of 4 files at r1.
Reviewable status:
pkg/ccl/changefeedccl/cdctest/testfeed.go, line 528 at r1 (raw file):
} // ReformatJSON marshals a golang stdlib based JSON into a byte slice preserving
nit: this comment could use another pass.
|
|
Correct. See #42390. We don't want to backport the interface change. Instead, you should adapt the backport to the old interface. That is, |
I want to say this was just a name change. See |
Currently, the changefeed poller detects a scan boundary when it detects that the last version of a table descriptor has a pending mutation but the current version doesn't. In case of an `ALTER TABLE DROP COLUMN` statement, the point at which this happens is the point at which the schema change backfill completes. However, this is incorrect since the dropped column gets logically dropped before that. This PR corrects this problem by instead using the set of column descriptors within the current and previous table descriptors to detect a scan boundary. Fixes cockroachdb#41961 Release note (bug fix): Changefeeds now emit backfill row updates for dropped column when the table descriptor drops that column.
aayushshah15
force-pushed
the
backport19.2-42053
branch
from
9083eef to
45e3a73
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/ccl/changefeedccl/cdctest/testfeed.go, line 528 at r1 (raw file):
Previously, ajwerner wrote…
nit: this comment could use another pass.
Done. let me know if it's still unclear.
|
The build should succeed now. are we good to merge this once it's green? |
Fine by me. |
Backport 1/1 commits from #42053.
/cc @cockroachdb/release
Currently, the changefeed poller detects a scan boundary when
detects that the last version of a table descriptor has a
mutation but the current version doesn't. In case of an
ALTER TABLE DROP COLUMNstatement, the point at which this happens is the pointat which the schema change backfill completes. This is incorrect since
the dropped column is logically dropped before this point.
This PR corrects this problem by instead checking that the last version of the descriptor has a mutation AND that the number of columns in the current table descriptor is different from the number of columns in the last table descriptor.
Fixes #41961
Release note (bug fix): Changefeeds now emit backfill row updates for
dropped column when the table descriptor drops
that column.