New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sqlbase: alias coltype.BigInt to VisibleType.BIGINT #20798
sqlbase: alias coltype.BigInt to VisibleType.BIGINT #20798
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tossing the thought out there: is there any way changing the results seen in show_table
would change how a driver interprets the column information?
LGTM thought.
@@ -21,6 +21,7 @@ var ( | |||
Bool = &TBool{Name: "BOOL"} | |||
// Boolean is an immutable T instance. | |||
Boolean = &TBool{Name: "BOOLEAN"} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I just noticed this was some surgical spacing. Carry on :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pkg/sql/table_test.go
Outdated
@@ -53,6 +53,11 @@ func TestMakeTableDescColumns(t *testing.T) { | |||
sqlbase.ColumnType{SemanticType: sqlbase.ColumnType_INT}, | |||
true, | |||
}, | |||
{ | |||
"BIGINT", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while you're here, can you add one for DOUBLE PRECISION
. We had a typo in the alias up until recently :(
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Review status: 0 of 4 files reviewed at latest revision, 2 unresolved discussions, some commit checks pending. pkg/sql/table_test.go, line 57 at r1 (raw file): Previously, lego (Joey Pereira) wrote…
I was also going to say maybe add test cases for Comments from Reviewable |
Fixes cockroachdb#19197. This change cleans up the `coltype` to `VisibleType` alias mapping and adds an alias for `coltype.BigInt -> VisibleType.BIGINT`. The `coltypes.Double -> VisibleType.DOUBLE_PRECISION` mapping referenced in the associated issue was already addressed during some previous refactor. Release note (bug fix): The BIGINT type alias is now correctly shown when using SHOW CREATE TABLE.
567bf56
to
9a33302
Compare
TFTRs!
I don't think any driver/ORM is going to parse the results of SHOW CREATE TABLE. I don't even think that it's standard syntax. Instead, they use |
Fixes #19197.
This change cleans up the
coltype
toVisibleType
alias mapping and addsan alias for
coltype.BigInt -> VisibleType.BIGINT
.The
coltypes.Double -> VisibleType.DOUBLE_PRECISION
mapping referenced inthe associated issue was already addressed during some previous refactor.
Release note (bug fix): The BIGINT type alias is now correctly shown
when using SHOW CREATE TABLE.