tree: introduce concept of "default" collation#56598
tree: introduce concept of "default" collation#56598craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
6bf5d4a to
9cd47b1
Compare
Release note (sql change): Introduced a pg_collation of "default". Strings now return the "default" collation OID in the pg_attribute table (this was previously en_US). The "default" collation is also visible on the pg_collation virtual table.
9cd47b1 to
530057b
Compare
rafiss
left a comment
There was a problem hiding this comment.
lgtm -- but wondering about char and name
| 17 bytea 0 0 NULL NULL NULL | ||
| 18 char 0 3903121477 NULL NULL NULL | ||
| 19 name 0 3903121477 NULL NULL NULL | ||
| 18 char 0 3403232968 NULL NULL NULL |
There was a problem hiding this comment.
seems like char in PG has a typcollation of 0
There was a problem hiding this comment.
hmm, i think this is a separate issue in that this is what pg_type reports, which is different.
i think that's a separate issue. we currently base this column on the collation of the type...
| 18 char 0 3903121477 NULL NULL NULL | ||
| 19 name 0 3903121477 NULL NULL NULL | ||
| 18 char 0 3403232968 NULL NULL NULL | ||
| 19 name 0 3403232968 NULL NULL NULL |
There was a problem hiding this comment.
name has a collation of C in PG
???
There was a problem hiding this comment.
yeah i think that makes sense as name is an internal thing isn't it? either way we don't have C, so i think we can leave this as is
| // To most behave like postgres, set the CollatedString type if a non-"default" | ||
| // collation is used. | ||
| if locale != DefaultCollationTag { | ||
| _, err := language.Parse(locale) |
There was a problem hiding this comment.
so i guess here would be the place where we could normalize the collation name using lex.EncodeLocaleName. do you think we should do that?
(either in this PR, or at any point?)
There was a problem hiding this comment.
yeah, as well as change all language.Parse calls to ensure it does this appropriately...
There was a problem hiding this comment.
yeah, that sounds about right. maybe somewhere with alter column type as well.
maybe in general we shouldn't use language.Parse but write our own wrapper around it.
|
char and name seems like an existing problem based on the type descriptor -- so i'm going to ignore this for now. https://github.com/cockroachdb/cockroach/blob/master/pkg/sql/pg_catalog.go#L2276 going to leave this to another issue. TFTR! bors r=rafiss |
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build succeeded: |
Resolves #54989
Release note (sql change): Introduced a pg_collation of "default".
Strings now return the "default" collation OID in the pg_attribute
table (this was previously en_US). The "default" collation is also
visible on the pg_collation virtual table.