Require region codes are globally unique #621
Conversation
googlebot
added
the
cla: yes
| // I call this the "Microsoft method". For each duplicate realm, | ||
| // append -N, starting with 1. If there are 3 realms with the region | ||
| // code "PA", their new values will be "PA", "PA-1", and "PA-2" | ||
| // respectively. |
There was a problem hiding this comment.
I don't think there is a 'good' way do this this automatically.
I also don't have another suggestion.
sethvargo
force-pushed
the
sethvargo/region_unique
branch
from
bf22392 to
de0f01d
Compare
sethvargo
force-pushed
the
sethvargo/region_unique
branch
from
de0f01d to
b0f3c28
Compare
| // handle NULL and uniqueness, the field is converted from it's ptr type to a | ||
| // concrete type in callbacks. Do not modify RegionCodePtr directly. | ||
| RegionCode string `gorm:"-"` | ||
| RegionCodePtr *string `gorm:"column:region_code; type:varchar(10);"` |
There was a problem hiding this comment.
does gorm work w/ sql.NullString?
There was a problem hiding this comment.
Yea, but our code doesn't (really). Changing the type of RegionCode requires every caller to update their type. So all cases of:
realm.RegionCode = "foo"
printf(realm.RegionCode)become
if v != "" {
realm.RegionCode = sql.NullString{Value:v, Valid: true}
} else {
ream.RegionCode = sql.NullString{}
}
if realm.RegionCode.Valid() {
printf(realm.RegionCode.Value)
} else {
printf("")
}and I don't want to put that burden on callers tbh.
This is required for the ENX redirector, but we've also always made the assumption that these need to be globally unique. Note that this requires changing the column to citext (for case-insensitive matching) and to NULL out the default of '', since '' is a duplicate of ''. It also handles any existing duplicates by appending -N to them before applying the database-level unique constraint.
sethvargo
force-pushed
the
sethvargo/region_unique
branch
from
b0f3c28 to
b6b0791
Compare
|
@mikehelmick PTAnotherL |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jeremyfaller, mikehelmick, sethvargo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This is required for the ENX redirector, but we've also always made the
assumption that these need to be globally unique.
Note that this requires changing the column to citext (for
case-insensitive matching) and to NULL out the default of '', since ''
is a duplicate of ''.
It also handles any existing duplicates by appending -N to them before
applying the database-level unique constraint.
Release Note
/cc @icco @mikehelmick