Skip to content
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

sql: INT4 column type not reported as INT4 when inspecting column types (SHOW CREATE, errors etc) #25098

Closed
a-robinson opened this issue Apr 26, 2018 · 3 comments
Assignees
Labels
A-sql-semantics C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.

Comments

@a-robinson
Copy link
Contributor

a-robinson commented Apr 26, 2018

According to https://www.cockroachlabs.com/docs/stable/int.html#names-and-aliases, INT4 is treated as a 32-bit type that can't simply be aliased to a full 64-bit INTEGER. However, the actual code does convert INT4 columns to INTEGER on both v1.1.4 and v2.0.1:

root@:26257/> create database foo;                                                                                                                                                                          CREATE DATABASE

Time: 5.524673ms

root@:26257/> create table foo.bar (x INT4, y INT, z INT2);
CREATE TABLE

Time: 5.551593ms

root@:26257/> show create table foo.bar;
+---------+---------------------------------------+
|  Table  |              CreateTable              |
+---------+---------------------------------------+
| foo.bar | CREATE TABLE bar (                    |
|         |                                       |
|         |     x INTEGER NULL,                   |
|         |                                       |
|         |     y INT NULL,                       |
|         |                                       |
|         |     z SMALLINT NULL,                  |
|         |                                       |
|         |     FAMILY "primary" (x, y, z, rowid) |
|         |                                       |
|         | )                                     |
+---------+---------------------------------------+
(1 row)

Time: 7.215492ms

Which is wrong -- the code or the docs?

@a-robinson
Copy link
Contributor Author

Reported by @borovan on Gitter.

@a-robinson a-robinson added the O-community Originated from the community label Apr 26, 2018
@a-robinson
Copy link
Contributor Author

Oh, weird. It does do 32-bit int validation even though it claims to be INTEGER:

root@:26257/> insert into foo.bar values (2147483647, 0, 0);
INSERT 1

Time: 2.085641ms

root@:26257/> insert into foo.bar values (2147483648, 0, 0);
pq: integer out of range for type INTEGER (column "x")

@knz
Copy link
Contributor

knz commented Apr 26, 2018

The error is in the output of SHOW CREATE TABLE and the generation of the text of the error messages. Underneath in the table descriptor the type is different.

@knz knz added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Apr 26, 2018
@knz knz added the S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption. label Apr 26, 2018
@knz knz changed the title sql: INT4 column type automatically converted to INTEGER on table creation sql: INT4 column type not reported as INT4 when inspecting column types (SHOW CREATE, errors etc) Apr 26, 2018
craig bot pushed a commit that referenced this issue Aug 23, 2018
28690: sql: fix the handling of integer types r=knz a=knz

Addresses a large chunk of #26925.
Fixes #25098.
Informs #24686.

Prior to this patch, CockroachDB maintained an unnecessary distinction
between "INT" and "INTEGER", between "BIGINT" and "INT8", etc.

This distinction is unnecessary but also costly, as we were paying the
price of a "name" attribute in coltypes.TInt, with a string comparison
and hash table lookup on every use of the type.

What really matters is that the type shows up properly in
introspection; this has already been ensured by various
OID-to-pgcatalog mappings and the recently introduced
`InformationSchemaTypeName()`.

Any distinction beyond that is unnecessary and can be dropped from the
implementation.

Release note: None


Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
@craig craig bot closed this as completed in #28690 Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-semantics C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. O-community Originated from the community S-3-ux-surprise Issue leaves users wondering whether CRDB is behaving properly. Likely to hurt reputation/adoption.
Projects
None yet
Development

No branches or pull requests

2 participants