sql: desugar TEXT to STRING#28950
Conversation
dee2493 to
73136f9
Compare
BramGruneir
left a comment
There was a problem hiding this comment.
Just some questions on leaving in tests for TEXT.
Reviewed 4 of 4 files at r1, 1 of 1 files at r2, 18 of 18 files at r3, 2 of 2 files at r4, 7 of 7 files at r5, 3 of 3 files at r6, 13 of 13 files at r7.
Reviewable status:complete! 0 of 0 LGTMs obtained (and 1 stale)
pkg/ccl/importccl/read_import_pgdump_test.go, line 126 at r7 (raw file):
fmt.Fprintf(&sb, "%s;\n", s) } const expect = `CREATE TABLE public.second (i INTEGER NOT NULL, s STRING);
Does pgdump ever dump a TEXT column? If so, this should still be tested.
pkg/sql/sem/tree/col_types_test.go, line 57 at r7 (raw file):
{"VARCHAR", &coltypes.TString{Name: "VARCHAR"}}, {"CHAR(11)", &coltypes.TString{Name: "CHAR", N: 11}}, {"TEXT", &coltypes.TString{Name: "TEXT"}},
Shouldn't this now just report STRING instead? Why remove the test and not just fix the expected?
pkg/sql/sem/tree/testdata/pretty/join1.align-deindent.golden, line 1 at r7 (raw file):
1:
We need to mark this file as auto-generated somehow.
73136f9 to
a196c70
Compare
knz
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained
pkg/ccl/importccl/read_import_pgdump_test.go, line 126 at r7 (raw file):
Previously, BramGruneir (Bram Gruneir) wrote…
Does pgdump ever dump a TEXT column? If so, this should still be tested.
This is tested, the input SQL uses text as type; we're verifying here that it is desugared to string.
pkg/sql/sem/tree/col_types_test.go, line 57 at r7 (raw file):
Previously, BramGruneir (Bram Gruneir) wrote…
Shouldn't this now just report STRING instead? Why remove the test and not just fix the expected?
No, CHAR is now a separate type from TEXT/STRING (and also separate from VARCHAR, and separate from "char").
pkg/sql/sem/tree/testdata/pretty/join1.align-deindent.golden, line 1 at r7 (raw file):
Previously, BramGruneir (Bram Gruneir) wrote…
We need to mark this file as auto-generated somehow.
Agreed. Done.
a196c70 to
b839d82
Compare
|
bors r+ |
Build failed |
The distinction between "TEXT" and "STRING" inside CockroachDB is inconsequential. Remove it. The data type is already properly reported in pg_catalog and information_schema as "text". Release note: None
Release note: None
b839d82 to
d1d5995
Compare
|
bors just detected a legitimate merge skew! good bors bors r+ |
28950: sql: desugar TEXT to STRING r=knz a=knz First commits from #28949 and priors. Forked off from #28690. The distinction between "TEXT" and "STRING" inside CockroachDB is inconsequential. Remove it. The data type is already properly reported in pg_catalog and information_schema as "text". Release note: None Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
Build succeeded |
29006: backport-2.1: fix the handling of SQL types r=knz a=knz Backport 1/1 commits from #28937. Backport 1/1 commits from #28942. Backport 17/17 commits from #28943. Backport 1/1 commits from #28944. Backport 1/1 commits from #28945. Backport 1/1 commits from #28949. Backport 2/2 commits from #28950. Backport 1/1 commits from #28951 (#28959). Backport 1/1 commits from #28952 (#28959). Backport 1/1 commits from #28955 (#28959). Backport 1/1 commits from #28959. Backport 1/1 commits from #28959. Backport 1/1 commits from #28690. cc @cockroachdb/release Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
29006: backport-2.1: fix the handling of SQL types r=knz a=knz Backport 1/1 commits from #28937. Backport 1/1 commits from #28942. Backport 17/17 commits from #28943. Backport 1/1 commits from #28944. Backport 1/1 commits from #28945. Backport 1/1 commits from #28949. Backport 2/2 commits from #28950. Backport 1/1 commits from #28951 (#28959). Backport 1/1 commits from #28952 (#28959). Backport 1/1 commits from #28955 (#28959). Backport 1/1 commits from #28959. Backport 1/1 commits from #28959. Backport 1/1 commits from #28690. cc @cockroachdb/release Co-authored-by: Raphael 'kena' Poss <knz@cockroachlabs.com>
First commits from #28949 and priors.
Forked off from #28690.
The distinction between "TEXT" and "STRING" inside CockroachDB is
inconsequential. Remove it. The data type is already properly reported
in pg_catalog and information_schema as "text".
Release note: None