sql: Remove unnecessary constraints check in show create #40417
Conversation
|
This change is |
rohany
force-pushed
the
show_create_fix
branch
from
ac60ea9 to
10d76cc
Compare
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @rohany and @solongordon)
pkg/sql/crdb_internal.go, line 1221 at r1 (raw file):
if zoneConfig.RangeMinBytes != nil || zoneConfig.RangeMaxBytes != nil || zoneConfig.NumReplicas != nil || len(zoneConfig.Constraints) != 0 || len(zoneConfig.LeasePreferences) != 0 {
This looks brittle. If anyone adds a new field to ZoneConfig and doesn't update this logic it'll be broken. Would it work to just compare to an empty config.ZoneConfig{} instead?
|
I just tried, and it doesn't seem like we can. The arrays inside of the zoneConfig can't be compared, so an error is thrown at compile time. A better solution might be to define an equality function on zone configs, and then drop a comment to update that? Or define a function isEmptyZoneConfig and then update that when the zoneConfig struct changes. |
|
If I'm not mistaken, |
|
Oh sweet -- i didnt know proto did that for us. Let me change that. |
The show create table function would not display a zone configuration if the constraints list was empty. However, this isn't a valid check because the constraints list could be empty but other fields set. We instead need to check that all the fields are default/empty values before not showing anything in the create statements. Release note: None
rohany
force-pushed
the
show_create_fix
branch
from
10d76cc to
7a76a6b
Compare
|
Alright, that looks like it worked. PTAL |
|
bors r+ |
40417: sql: Remove unnecessary constraints check in show create r=rohany a=rohany The show create table function would not display a zone configuration if the constraints list was empty. However, this isn't a valid check because the constraints list could be empty but other fields set. Release note: None 40441: storage: assert zero HardState.Commit on replica creation r=nvanbenschoten a=nvanbenschoten An uninitialized replica should never have a commit index in its HardState. And yet, we observed in #40213 that (likely) due to a missing Range deletion tombstone, this invariant can be violated in practice. This assertion will allow us to catch this kind of violation without the need for a MsgVote to race with a MsgSnap, hopefully making the bug implied by #40213 easier to track down. Release note: None 40472: opt: detect zero-cardinality semi/anti joins r=rytaft a=rytaft This commit adds two normalization rules to detect zero-cardinality semi and anti joins, and replace them with an empty `Values`. It also updates the stats estimate to ensure that the row count of semi and anti joins is not zero unless the cardinality is zero. Fixes #40456 Fixes #40440 Fixes #40394 Release note: None Co-authored-by: Rohan Yadav <[email protected]> Co-authored-by: Nathan VanBenschoten <[email protected]> Co-authored-by: Rebecca Taft <[email protected]>
Build succeeded |
The show create table function would not display a zone configuration
if the constraints list was empty. However, this isn't a valid check
because the constraints list could be empty but other fields set.
Release note: None