sql: return droppedView instead of nil #97802
Conversation
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
There was a problem hiding this comment.
Add some coverage for this?
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @chengxiong-ruan)
-- commits line 4 at r1:
nit: drop
|
Yeah, I was about to ask here. How do you feel like adding a new test file? The |
I think that makes sense for coverage |
chengxiong-ruan
force-pushed
the
return-dropped-view-instead-of-nil
branch
from
11fee10 to
d027c9c
Compare
There was a problem hiding this comment.
New logictest looks good, thanks for doing this.
Reviewable status:
In cockroachdb#97631 we refactor the method to drorp index and column cascade dependents. But we didn't return a correct droppedViews, instead, we returned a nil. This wasn't caught by the `event_log` logic test because we skip `local-legacy-schema-changer` test config in v23.1. Luckily,this was caught when backporting to v22.2 because there was a fallback. Epic: None Release note: None
chengxiong-ruan
force-pushed
the
return-dropped-view-instead-of-nil
branch
from
d027c9c to
75a70c1
Compare
|
TFTR! |
|
Build succeeded: |
In #97631 we refactor the method to drorp index and column cascade dependents. But we didn't return a correct droppedViews for event logging, instead, we returned a nil. This wasn't caught by the
event_loglogic test because welocal-legacy-schema-changertest config. Though this was caught when backporting to v22.2 because there was a fallback.Epic: None
Release note: None