sql: revert multiple partial stats requests error to a pgerror#128862
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 |
DrewKimball
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @michae2 and @Uzair5162)
pkg/sql/distsql_plan_stats.go line 294 at r1 (raw file):
// so we only support one requested stat at a time here. if len(reqStats) > 1 { return nil, pgerror.Newf(pgcode.FeatureNotSupported, "cannot process multiple partial statistics at once")
[nit] For a feature that isn't unimplemented yet, we usually use unimplemented.New or preferably unimplemented.NewWithIssue.
michae2
left a comment
There was a problem hiding this comment.
Nice quick fix!
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:
pkg/sql/distsql_plan_stats.go line 294 at r1 (raw file):
Previously, DrewKimball (Drew Kimball) wrote…
[nit] For a feature that isn't unimplemented yet, we usually use
unimplemented.Newor preferablyunimplemented.NewWithIssue.
+1
8588eb6 to
0a5ee95
Compare
This commit reverts a change in cockroachdb#127836 that changed the error returned when multiple stats are requested at once from a pgerror to a failed assertion. This change caused `GenerateAndCheckRedactedExplainsForPII()` to fail when generating SQL commands for certain explains on partial stats collections with multiple columns, which is unsupported and should be ignored like other pgerrors. Closes: cockroachdb#128848 Release note: None
0a5ee95 to
daf39c2
Compare
Uzair5162
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/sql/distsql_plan_stats.go line 294 at r1 (raw file):
Previously, michae2 (Michael Erickson) wrote…
+1
Done.
|
bors r+ |
|
Build succeeded: |
This commit reverts a change in #127836 that changed the error returned when multiple stats are requested at once from a pgerror to a failed assertion. This change caused
GenerateAndCheckRedactedExplainsForPII()to fail when generating SQL commands for certain explains on partial stats collections with multiple columns, which is unsupported and should be ignored like other pgerrors.Fixes: #128848
Release note: None