Skip to content

Commit

Permalink
Merge #28690
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
craig[bot] and knz committed Aug 23, 2018
2 parents e171f06 + 32f9509 commit 16be114
Show file tree
Hide file tree
Showing 28 changed files with 217 additions and 191 deletions.
4 changes: 2 additions & 2 deletions docs/generated/sql/bnf/stmt_block.bnf
Original file line number Diff line number Diff line change
Expand Up @@ -1793,12 +1793,12 @@ reference_actions ::=

numeric ::=
'INT'
| 'INTEGER'
| 'INT2'
| 'SMALLINT'
| 'INT4'
| 'INT8'
| 'INT64'
| 'INTEGER'
| 'SMALLINT'
| 'BIGINT'
| 'REAL'
| 'FLOAT4'
Expand Down
18 changes: 9 additions & 9 deletions pkg/ccl/importccl/import_stmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -483,16 +483,16 @@ COPY t (a, b, c) FROM stdin;
ORDER BY descriptor_name
`: {{
`CREATE TABLE a (
i INTEGER NOT NULL,
k INTEGER NULL,
i INT NOT NULL,
k INT NULL,
CONSTRAINT a_pkey PRIMARY KEY (i ASC),
CONSTRAINT a_k_fkey FOREIGN KEY (k) REFERENCES a (i),
INDEX a_auto_index_a_k_fkey (k ASC),
CONSTRAINT a_i_fkey FOREIGN KEY (i) REFERENCES b (j),
FAMILY "primary" (i, k)
)`}, {
`CREATE TABLE b (
j INTEGER NOT NULL,
j INT NOT NULL,
CONSTRAINT b_pkey PRIMARY KEY (j ASC),
CONSTRAINT b_j_fkey FOREIGN KEY (j) REFERENCES a (i),
FAMILY "primary" (j)
Expand Down Expand Up @@ -631,8 +631,8 @@ const (
)`
testPgdumpCreateWeather = `CREATE TABLE weather (
city VARCHAR(80) NULL,
temp_lo INTEGER NULL,
temp_hi INTEGER NULL,
temp_lo INT NULL,
temp_hi INT NULL,
prcp FLOAT4 NULL,
date DATE NULL,
CONSTRAINT weather_city_fkey FOREIGN KEY (city) REFERENCES cities (city),
Expand Down Expand Up @@ -2220,7 +2220,7 @@ func TestImportPgDump(t *testing.T) {
// Verify table schema because PKs and indexes are at the bottom of pg_dump.
sqlDB.CheckQueryResults(t, `SHOW CREATE TABLE simple`, [][]string{{
"simple", `CREATE TABLE simple (
i INTEGER NOT NULL,
i INT NOT NULL,
s STRING NULL,
b BYTES NULL,
CONSTRAINT simple_pkey PRIMARY KEY (i ASC),
Expand Down Expand Up @@ -2262,7 +2262,7 @@ func TestImportPgDump(t *testing.T) {
// Verify table schema because PKs and indexes are at the bottom of pg_dump.
sqlDB.CheckQueryResults(t, `SHOW CREATE TABLE second`, [][]string{{
"second", `CREATE TABLE second (
i INTEGER NOT NULL,
i INT NOT NULL,
s STRING NULL,
CONSTRAINT second_pkey PRIMARY KEY (i ASC),
FAMILY "primary" (i, s)
Expand Down Expand Up @@ -2296,8 +2296,8 @@ func TestImportPgDump(t *testing.T) {
if c.expected == expectAll {
sqlDB.CheckQueryResults(t, `SHOW CREATE TABLE seqtable`, [][]string{{
"seqtable", `CREATE TABLE seqtable (
a INTEGER NULL DEFAULT nextval('public.a_seq':::STRING),
b INTEGER NULL,
a INT NULL DEFAULT nextval('public.a_seq':::STRING),
b INT NULL,
FAMILY "primary" (a, b, rowid)
)`,
}})
Expand Down
22 changes: 11 additions & 11 deletions pkg/ccl/importccl/read_import_mysql.go
Original file line number Diff line number Diff line change
Expand Up @@ -511,25 +511,25 @@ func mysqlColToCockroach(
def.Type = coltypes.Bytes

case mysqltypes.Int8:
def.Type = coltypes.SmallInt
def.Type = coltypes.Int2
case mysqltypes.Uint8:
def.Type = coltypes.SmallInt
def.Type = coltypes.Int2
case mysqltypes.Int16:
def.Type = coltypes.SmallInt
def.Type = coltypes.Int2
case mysqltypes.Uint16:
def.Type = coltypes.SmallInt
def.Type = coltypes.Int2
case mysqltypes.Int24:
def.Type = coltypes.Int
def.Type = coltypes.Int4
case mysqltypes.Uint24:
def.Type = coltypes.Int
def.Type = coltypes.Int4
case mysqltypes.Int32:
def.Type = coltypes.Int
def.Type = coltypes.Int4
case mysqltypes.Uint32:
def.Type = coltypes.Int
def.Type = coltypes.Int4
case mysqltypes.Int64:
def.Type = coltypes.BigInt
def.Type = coltypes.Int8
case mysqltypes.Uint64:
def.Type = coltypes.BigInt
def.Type = coltypes.Int8

case mysqltypes.Float32:
def.Type = coltypes.Float4
Expand All @@ -548,7 +548,7 @@ func mysqlColToCockroach(
case mysqltypes.Datetime:
def.Type = coltypes.TimestampWithTZ
case mysqltypes.Year:
def.Type = coltypes.SmallInt
def.Type = coltypes.Int2

case mysqltypes.Enum:
def.Type = coltypes.String
Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/importccl/read_import_pgdump_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ COPY public.t (s) FROM stdin;
}
fmt.Fprintf(&sb, "%s;\n", s)
}
const expect = `CREATE TABLE public.second (i INTEGER NOT NULL, s STRING);
const expect = `CREATE TABLE public.second (i INT NOT NULL, s STRING);
COPY public.second (i, s) FROM STDIN;
"0" "0";
"1" "1";
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
CREATE TABLE everything (
i INT PRIMARY KEY,
i INT4 PRIMARY KEY,

c CHAR(10) NOT NULL,
s VARCHAR(100),
Expand All @@ -21,11 +21,11 @@ CREATE TABLE everything (
nu NUMERIC(10, 0),
d53 DECIMAL(5,3),

iw INT NOT NULL,
iz INT,
iw INT4 NOT NULL,
iz INT4,
ti SMALLINT,
si SMALLINT,
mi INT,
mi INT4,
bi BIGINT,

fl FLOAT4 NOT NULL,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
CREATE TABLE second (
i INT PRIMARY KEY,
k INT,
i INT4 PRIMARY KEY,
k INT4,
UNIQUE INDEX ik (i, k),
INDEX ki (k, i),
CONSTRAINT second_ibfk_1 FOREIGN KEY (k) REFERENCES simple (i) ON UPDATE CASCADE
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
CREATE SEQUENCe simple_auto_inc;

CREATE TABLE simple (
i INT PRIMARY KEY DEFAULT nextval('simple_auto_inc':::string),
i INT4 PRIMARY KEY DEFAULT nextval('simple_auto_inc':::string),
s text,
b bytea
)
10 changes: 5 additions & 5 deletions pkg/ccl/importccl/testdata/mysqldump/third.cockroach-schema.sql
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
CREATE TABLE third (
i INT PRIMARY KEY,
a INT,
b INT,
c INT,
i INT4 PRIMARY KEY,
a INT4,
b INT4,
c INT4,
INDEX a (a, b),
INDEX c (c),
FOREIGN KEY (a, b) REFERENCES second (i, k),
FOREIGN KEY (c) REFERENCES third (i) ON UPDATE CASCADE
);
);
25 changes: 9 additions & 16 deletions pkg/sql/coltypes/aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,29 +28,22 @@ var (
Bool = &TBool{}

// Int is an immutable T instance.
Int = &TInt{Name: "INT"}
Int = &TInt{}
// Int2 is an immutable T instance.
Int2 = &TInt{Name: "INT2", Width: 16, ImplicitWidth: true}
Int2 = &TInt{Width: 16}
// Int4 is an immutable T instance.
Int4 = &TInt{Name: "INT4", Width: 32, ImplicitWidth: true}
Int4 = &TInt{Width: 32}
// Int8 is an immutable T instance.
Int8 = &TInt{Name: "INT8"}
// Int64 is an immutable T instance.
Int64 = &TInt{Name: "INT64"}
// Integer is an immutable T instance.
Integer = &TInt{Name: "INTEGER"}
// SmallInt is an immutable T instance.
SmallInt = &TInt{Name: "SMALLINT", Width: 16, ImplicitWidth: true}
// BigInt is an immutable T instance.
BigInt = &TInt{Name: "BIGINT"}
Int8 = &TInt{Width: 64}

// Serial is an immutable T instance.
Serial = &TSerial{IntType: Int}
Serial = &TSerial{TInt: Int}
// Serial2 is an immutable T instance.
Serial2 = &TSerial{IntType: Int2}
Serial2 = &TSerial{TInt: Int2}
// Serial4 is an immutable T instance.
Serial4 = &TSerial{IntType: Int4}
Serial4 = &TSerial{TInt: Int4}
// Serial8 is an immutable T instance.
Serial8 = &TSerial{IntType: Int8}
Serial8 = &TSerial{TInt: Int8}

// Float4 is an immutable T instance.
Float4 = &TFloat{Short: true}
Expand Down
35 changes: 18 additions & 17 deletions pkg/sql/coltypes/arith.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,36 +34,37 @@ func (node *TBool) Format(buf *bytes.Buffer, f lex.EncodeFlags) {

// TInt represents an INT, INTEGER, SMALLINT or BIGINT type.
type TInt struct {
Name string
Width int
ImplicitWidth bool
Width int
}

// IntegerTypeNames maps a TInt data width to a canonical type name.
var IntegerTypeNames = map[int]string{
0: "INT",
16: "INT2",
32: "INT4",
64: "INT8",
}

// TypeName implements the ColTypeFormatter interface.
func (node *TInt) TypeName() string { return node.Name }
func (node *TInt) TypeName() string { return IntegerTypeNames[node.Width] }

// Format implements the ColTypeFormatter interface.
func (node *TInt) Format(buf *bytes.Buffer, f lex.EncodeFlags) {
buf.WriteString(node.Name)
if node.Width > 0 && !node.ImplicitWidth {
fmt.Fprintf(buf, "(%d)", node.Width)
}
buf.WriteString(node.TypeName())
}

// TSerial represents a SERIAL type.
type TSerial struct {
IntType *TInt
}
type TSerial struct{ *TInt }

var serialNames = map[*TInt]string{
Int: "SERIAL",
Int2: "SERIAL2",
Int4: "SERIAL4",
Int8: "SERIAL8",
var serialNames = map[int]string{
0: "SERIAL",
16: "SERIAL2",
32: "SERIAL4",
64: "SERIAL8",
}

// TypeName implements the ColTypeFormatter interface.
func (node *TSerial) TypeName() string { return serialNames[node.IntType] }
func (node *TSerial) TypeName() string { return serialNames[node.Width] }

// Format implements the ColTypeFormatter interface.
func (node *TSerial) Format(buf *bytes.Buffer, _ lex.EncodeFlags) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/logic_test/alter_column_type
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ query TTBTTTB colnames
SHOW COLUMNS FROM t
----
column_name data_type is_nullable column_default generation_expression indices is_hidden
a INTEGER true NULL · {} false
a INT true NULL · {} false
rowid INT false unique_rowid() · {"primary"} true

statement ok
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/array
Original file line number Diff line number Diff line change
Expand Up @@ -565,9 +565,9 @@ query TT
SHOW CREATE TABLE a
----
a CREATE TABLE a (
b SMALLINT[] NULL,
FAMILY "primary" (b, rowid)
)
b INT2[] NULL,
FAMILY "primary" (b, rowid)
)

statement error integer out of range for type SMALLINT \(column "b"\)
INSERT INTO a VALUES (ARRAY[100000])
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/information_schema
Original file line number Diff line number Diff line change
Expand Up @@ -826,6 +826,9 @@ DROP TABLE nullability
statement ok
CREATE TABLE data_types (
a INT,
a2 INT2,
a4 INT4,
a8 INT8,
b FLOAT,
br REAL,
c DECIMAL,
Expand All @@ -848,6 +851,9 @@ WHERE table_schema = 'public' AND table_name = 'data_types'
----
table_name column_name data_type crdb_sql_type
data_types a integer INT
data_types a2 smallint INT2
data_types a4 integer INT4
data_types a8 bigint INT8
data_types b double precision FLOAT8
data_types br real FLOAT4
data_types c numeric DECIMAL
Expand Down
8 changes: 4 additions & 4 deletions pkg/sql/logictest/testdata/logic_test/pg_catalog
Original file line number Diff line number Diff line change
Expand Up @@ -1270,22 +1270,22 @@ SHOW COLUMNS FROM pg_catalog.pg_stat_activity
column_name data_type is_nullable column_default generation_expression indices is_hidden
datid OID true NULL · {} false
datname NAME true NULL · {} false
pid INTEGER true NULL · {} false
pid INT true NULL · {} false
usesysid OID true NULL · {} false
username NAME true NULL · {} false
application_name STRING true NULL · {} false
client_addr INET true NULL · {} false
client_hostname STRING true NULL · {} false
client_port INTEGER true NULL · {} false
client_port INT true NULL · {} false
backend_start TIMESTAMPTZ true NULL · {} false
xact_start TIMESTAMPTZ true NULL · {} false
query_start TIMESTAMPTZ true NULL · {} false
state_change TIMESTAMPTZ true NULL · {} false
wait_event_type STRING true NULL · {} false
wait_event STRING true NULL · {} false
state STRING true NULL · {} false
backend_xid INTEGER true NULL · {} false
backend_xmin INTEGER true NULL · {} false
backend_xid INT true NULL · {} false
backend_xmin INT true NULL · {} false
query STRING true NULL · {} false


Expand Down
10 changes: 5 additions & 5 deletions pkg/sql/logictest/testdata/logic_test/serial
Original file line number Diff line number Diff line change
Expand Up @@ -241,8 +241,8 @@ query TT
SHOW CREATE TABLE smallbig
----
smallbig CREATE TABLE smallbig (
a SMALLINT NOT NULL DEFAULT nextval('smallbig_a_seq1':::STRING),
b BIGINT NOT NULL DEFAULT nextval('smallbig_b_seq1':::STRING),
a INT2 NOT NULL DEFAULT nextval('smallbig_a_seq1':::STRING),
b INT8 NOT NULL DEFAULT nextval('smallbig_b_seq1':::STRING),
c INT NULL,
FAMILY "primary" (a, b, c, rowid)
)
Expand All @@ -259,9 +259,9 @@ query TT
SHOW CREATE TABLE serials
----
serials CREATE TABLE serials (
a SMALLINT NOT NULL DEFAULT nextval('serials_a_seq1':::STRING),
b INTEGER NOT NULL DEFAULT nextval('serials_b_seq1':::STRING),
c BIGINT NOT NULL DEFAULT nextval('serials_c_seq1':::STRING),
a INT2 NOT NULL DEFAULT nextval('serials_a_seq1':::STRING),
b INT4 NOT NULL DEFAULT nextval('serials_b_seq1':::STRING),
c INT8 NOT NULL DEFAULT nextval('serials_c_seq1':::STRING),
d INT NULL,
FAMILY "primary" (a, b, c, d, rowid)
)
Expand Down
Loading

0 comments on commit 16be114

Please sign in to comment.