From 750f9307c669defa0b7023f66490e6dc827f53d4 Mon Sep 17 00:00:00 2001 From: Oleg Afanasyev Date: Mon, 6 Feb 2023 18:33:18 +0000 Subject: [PATCH 1/8] allocator: fix panic when accessing nil store descriptor Previously allocator was trying to update store descriptor counters on unitialized stores. Instead of checking if detail is nil which should never happen, it should check if descriptor in detail is not nil as it is only eventually populated by gossip. Release note: None --- pkg/kv/kvserver/allocator/storepool/store_pool.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/kv/kvserver/allocator/storepool/store_pool.go b/pkg/kv/kvserver/allocator/storepool/store_pool.go index 55beabc549ff..3158746adae5 100644 --- a/pkg/kv/kvserver/allocator/storepool/store_pool.go +++ b/pkg/kv/kvserver/allocator/storepool/store_pool.go @@ -620,14 +620,14 @@ func (sp *StorePool) UpdateLocalStoreAfterRelocate( updateTargets := func(targets []roachpb.ReplicationTarget) { for _, target := range targets { - if toDetail := sp.GetStoreDetailLocked(target.StoreID); toDetail != nil { + if toDetail := sp.GetStoreDetailLocked(target.StoreID); toDetail.Desc != nil { toDetail.Desc.Capacity.RangeCount++ } } } updatePrevious := func(previous []roachpb.ReplicaDescriptor) { for _, old := range previous { - if toDetail := sp.GetStoreDetailLocked(old.StoreID); toDetail != nil { + if toDetail := sp.GetStoreDetailLocked(old.StoreID); toDetail.Desc != nil { toDetail.Desc.Capacity.RangeCount-- } } From ea3342d33f86bc21bf6290fe440676b80f490a59 Mon Sep 17 00:00:00 2001 From: Rafi Shamim Date: Mon, 6 Feb 2023 12:43:30 -0500 Subject: [PATCH 2/8] sql: fix column_default in info_schema so literals are quoted fixes https://github.com/cockroachdb/cockroach/issues/96098 In bc3fccd2e4b14b07d2572434203dd8f548523623, the format flags were changed to use FmtPgWireText for information_schema.columns(column_default). This does not work for string literals, since that format does not wrap literals in single quotes. Now we've changed to FmtParsableNumerics, which does properly add quotes, and also avoids adding the `:::` type annotation as FmtPgWireText does. No release note since this bug was never released. Release note: None --- pkg/server/admin_test.go | 4 ++-- pkg/sql/information_schema.go | 2 +- pkg/sql/logictest/testdata/logic_test/enums | 4 ++-- pkg/sql/logictest/testdata/logic_test/information_schema | 2 +- pkg/sql/logictest/testdata/logic_test/system | 8 ++++---- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pkg/server/admin_test.go b/pkg/server/admin_test.go index d5a295e3332b..b3a2b02e5505 100644 --- a/pkg/server/admin_test.go +++ b/pkg/server/admin_test.go @@ -714,7 +714,7 @@ func TestAdminAPITableDetails(t *testing.T) { name, dbName, tblName, pkName string }{ {name: "lower", dbName: "test", tblName: "tbl", pkName: "tbl_pkey"}, - {name: "lower", dbName: "test", tblName: `testschema.tbl`, pkName: "tbl_pkey"}, + {name: "lower other schema", dbName: "test", tblName: `testschema.tbl`, pkName: "tbl_pkey"}, {name: "lower with space", dbName: "test test", tblName: `"tbl tbl"`, pkName: "tbl tbl_pkey"}, {name: "upper", dbName: "TEST", tblName: `"TBL"`, pkName: "TBL_pkey"}, // Regression test for issue #14056 } { @@ -778,7 +778,7 @@ func TestAdminAPITableDetails(t *testing.T) { {Name: "nulls_allowed", Type: "INT8", Nullable: true, DefaultValue: ""}, {Name: "nulls_not_allowed", Type: "INT8", Nullable: false, DefaultValue: "1000"}, {Name: "default2", Type: "INT8", Nullable: true, DefaultValue: "2"}, - {Name: "string_default", Type: "STRING", Nullable: true, DefaultValue: "default_string"}, + {Name: "string_default", Type: "STRING", Nullable: true, DefaultValue: "'default_string'"}, {Name: "rowid", Type: "INT8", Nullable: false, DefaultValue: "unique_rowid()", Hidden: true}, } testutils.SortStructs(expColumns, "Name") diff --git a/pkg/sql/information_schema.go b/pkg/sql/information_schema.go index bed7fe66620d..0a54ff084984 100644 --- a/pkg/sql/information_schema.go +++ b/pkg/sql/information_schema.go @@ -441,7 +441,7 @@ https://www.postgresql.org/docs/9.5/infoschema-columns.html`, colDefault := tree.DNull if column.HasDefault() { colExpr, err := schemaexpr.FormatExprForDisplay( - ctx, table, column.GetDefaultExpr(), &p.semaCtx, p.SessionData(), tree.FmtPgwireText, + ctx, table, column.GetDefaultExpr(), &p.semaCtx, p.SessionData(), tree.FmtParsableNumerics, ) if err != nil { return err diff --git a/pkg/sql/logictest/testdata/logic_test/enums b/pkg/sql/logictest/testdata/logic_test/enums index 81ef72fa9f48..ca94222704d5 100644 --- a/pkg/sql/logictest/testdata/logic_test/enums +++ b/pkg/sql/logictest/testdata/logic_test/enums @@ -536,8 +536,8 @@ WHERE ORDER BY column_name ---- -y hello -z hello IS OF (test.public.greeting, test.public.greeting) +y 'hello' +z 'hello' IS OF (test.public.greeting, test.public.greeting) # Test computed columns with enum values. statement ok diff --git a/pkg/sql/logictest/testdata/logic_test/information_schema b/pkg/sql/logictest/testdata/logic_test/information_schema index d1acf82d6264..79783fd52ad1 100644 --- a/pkg/sql/logictest/testdata/logic_test/information_schema +++ b/pkg/sql/logictest/testdata/logic_test/information_schema @@ -2410,7 +2410,7 @@ WHERE table_schema = 'public' AND table_name = 'with_defaults' ---- table_name column_name column_default with_defaults a 9 -with_defaults b default +with_defaults b 'default' with_defaults c NULL with_defaults d NULL with_defaults rowid unique_rowid() diff --git a/pkg/sql/logictest/testdata/logic_test/system b/pkg/sql/logictest/testdata/logic_test/system index 65934fffad89..913600b96361 100644 --- a/pkg/sql/logictest/testdata/logic_test/system +++ b/pkg/sql/logictest/testdata/logic_test/system @@ -250,10 +250,10 @@ descriptor BYTES true NULL · {primary} false query TTBTTTB SHOW COLUMNS FROM system.users ---- -username STRING false NULL · {primary,users_user_id_idx} false -hashedPassword BYTES true NULL · {primary} false -isRole BOOL false f · {primary} false -user_id OID false NULL · {primary,users_user_id_idx} false +username STRING false NULL · {primary,users_user_id_idx} false +hashedPassword BYTES true NULL · {primary} false +isRole BOOL false false · {primary} false +user_id OID false NULL · {primary,users_user_id_idx} false query TTBTTTB SHOW COLUMNS FROM system.zones From 3f50d81d062371d13c5cb56f9daeac6f31ede7d9 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 5 Feb 2023 12:09:16 +0100 Subject: [PATCH 3/8] sql: properly report information_schema functions in metadata Prior to this patch, the `information_schema` functions were listed with the `information_schema.` prefix in the pg_catalog namespace. This was incorrect and caused confusion during tab completion. Release note (bug fix): The compatibility scalar functions in `information_schema` are now listed in the proper namespace in `pg_catalog.pg_proc`. --- pkg/cli/clisqlshell/testdata/complete/composite_names | 9 +++++++++ pkg/cli/clisqlshell/testdata/complete/sql | 2 +- pkg/sql/crdb_internal.go | 4 ++++ pkg/sql/pg_catalog.go | 4 ++++ 4 files changed, 18 insertions(+), 1 deletion(-) diff --git a/pkg/cli/clisqlshell/testdata/complete/composite_names b/pkg/cli/clisqlshell/testdata/complete/composite_names index f7ba98c71b06..c14fd44fedd3 100644 --- a/pkg/cli/clisqlshell/testdata/complete/composite_names +++ b/pkg/cli/clisqlshell/testdata/complete/composite_names @@ -63,3 +63,12 @@ select "PG_CATALOG".@ complete 0 20 msg: "" (no completions generated) + +complete +select information_schema . _pg_index@ +---- +complete 0 37 +msg: "" +completions: +- "functions": + "information_schema._pg_index_position(" (Not usable; exposed only for compatibility with PostgreSQL) -> "information_schema._pg_index_position(" (0, 30) diff --git a/pkg/cli/clisqlshell/testdata/complete/sql b/pkg/cli/clisqlshell/testdata/complete/sql index eddd7e43f70d..2fba33363fae 100644 --- a/pkg/cli/clisqlshell/testdata/complete/sql +++ b/pkg/cli/clisqlshell/testdata/complete/sql @@ -19,7 +19,7 @@ completions: "\"extract_duration\"(" ((from schema pg_catalog) Extracts `element` from `input`) -> "\"extract_duration\"(" (0, 0) "\"family\"(" ((from schema pg_catalog) Extracts the IP family of the value; 4 for IPv4, 6 for IPv6) -> "\"family\"(" (0, 0) "\"greatest\"(" ((from schema pg_catalog) Returns the element with the greatest value) -> "\"greatest\"(" (0, 0) - "\"information_schema._pg_char_max_length\"(" ((from schema pg_catalog) Not usable; exposed only for compatibility with PostgreSQL) -> "\"information_schema._pg_char_max_length\"(" (0, 0) + "\"least\"(" ((from schema pg_catalog) Returns the element with the lowest value) -> "\"least\"(" (0, 0) ... entries omitted ... - "keyword": "ABORT" (unreserved) -> "ABORT" (0, 0) diff --git a/pkg/sql/crdb_internal.go b/pkg/sql/crdb_internal.go index 394942076a1b..ff7f68bea527 100644 --- a/pkg/sql/crdb_internal.go +++ b/pkg/sql/crdb_internal.go @@ -2753,9 +2753,13 @@ CREATE TABLE crdb_internal.builtin_functions ( props, overloads := builtinsregistry.GetBuiltinProperties(name) schema := catconstants.PgCatalogName const crdbInternal = catconstants.CRDBInternalSchemaName + "." + const infoSchema = catconstants.InformationSchemaName + "." if strings.HasPrefix(name, crdbInternal) { name = name[len(crdbInternal):] schema = catconstants.CRDBInternalSchemaName + } else if strings.HasPrefix(name, infoSchema) { + name = name[len(infoSchema):] + schema = catconstants.InformationSchemaName } for _, f := range overloads { if err := addRow( diff --git a/pkg/sql/pg_catalog.go b/pkg/sql/pg_catalog.go index e9e958be5065..7b73a635e4ea 100644 --- a/pkg/sql/pg_catalog.go +++ b/pkg/sql/pg_catalog.go @@ -2357,9 +2357,13 @@ func addPgProcBuiltinRow(name string, addRow func(...tree.Datum) error) error { _, overloads := builtinsregistry.GetBuiltinProperties(name) nspOid := tree.NewDOid(catconstants.PgCatalogID) const crdbInternal = catconstants.CRDBInternalSchemaName + "." + const infoSchema = catconstants.InformationSchemaName + "." if strings.HasPrefix(name, crdbInternal) { nspOid = tree.NewDOid(catconstants.CrdbInternalID) name = name[len(crdbInternal):] + } else if strings.HasPrefix(name, infoSchema) { + nspOid = tree.NewDOid(catconstants.InformationSchemaID) + name = name[len(infoSchema):] } for _, builtin := range overloads { From fdb4db90fc569e6756e7a5948f09199099cfa234 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 5 Feb 2023 13:08:28 +0100 Subject: [PATCH 4/8] clisqlshell: fix various line editor bugs This Bubbline dep upgrade fixes the following bugs - after multi-line paste, editing a line would corrupt lines afterwards. - filtering tab completions using the "/" key would result in a go panic if the filter didn't match any entry. - the tab completion widget would crowd the screen if there were many completions. - the initial display of the tab completion widget before a screen resize was incorrect. Release note: None --- DEPS.bzl | 12 ++++++------ build/bazelutil/distdir_files.bzl | 4 ++-- go.mod | 2 +- go.sum | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/DEPS.bzl b/DEPS.bzl index 5763daa75831..80e0ee240d7a 100644 --- a/DEPS.bzl +++ b/DEPS.bzl @@ -5123,20 +5123,20 @@ def go_deps(): name = "com_github_knz_bubbline", build_file_proto_mode = "disable_global", importpath = "github.com/knz/bubbline", - sha256 = "0da0560f0116618e6e8762810ffa3fb3f2373e2478bebac6e977ed9c23f7611e", - strip_prefix = "github.com/knz/bubbline@v0.0.0-20230124184034-a0b62576595b", + sha256 = "0922c6fae4190a73e2023b1a56401347e29afddf1174990b8fd881433b9c75a9", + strip_prefix = "github.com/knz/bubbline@v0.0.0-20230205122847-05558f88fdc4", urls = [ - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/bubbline/com_github_knz_bubbline-v0.0.0-20230124184034-a0b62576595b.zip", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/bubbline/com_github_knz_bubbline-v0.0.0-20230205122847-05558f88fdc4.zip", ], ) go_repository( name = "com_github_knz_catwalk", build_file_proto_mode = "disable_global", importpath = "github.com/knz/catwalk", - sha256 = "7b44ddd491c68b186426e5f98fcb9410c4d26a5c4fa82205b3ff2797ffc3b51b", - strip_prefix = "github.com/knz/catwalk@v0.1.2", + sha256 = "f422f7974090494e54226262586c7b34fe57b33ab7d668151ca55eba8e309c1e", + strip_prefix = "github.com/knz/catwalk@v0.1.4", urls = [ - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/catwalk/com_github_knz_catwalk-v0.1.2.zip", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/catwalk/com_github_knz_catwalk-v0.1.4.zip", ], ) go_repository( diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl index d8ac51caeb57..c1688200a0cc 100644 --- a/build/bazelutil/distdir_files.bzl +++ b/build/bazelutil/distdir_files.bzl @@ -548,8 +548,8 @@ DISTDIR_FILES = { "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/klauspost/cpuid/v2/com_github_klauspost_cpuid_v2-v2.0.9.zip": "52c716413296dce2b1698c6cdbc4c53927ce4aee2a60980daf9672e6b6a3b4cb", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/klauspost/crc32/com_github_klauspost_crc32-v0.0.0-20161016154125-cb6bfca970f6.zip": "6b632853a19f039138f251f94dbbdfdb72809adc3a02da08e4301d3d48275b06", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/klauspost/pgzip/com_github_klauspost_pgzip-v1.2.5.zip": "1143b6417d4bb46d26dc8e6223407b84b6cd5f32e5d705cd4a9fb142220ce4ba", - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/bubbline/com_github_knz_bubbline-v0.0.0-20230124184034-a0b62576595b.zip": "0da0560f0116618e6e8762810ffa3fb3f2373e2478bebac6e977ed9c23f7611e", - "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/catwalk/com_github_knz_catwalk-v0.1.2.zip": "7b44ddd491c68b186426e5f98fcb9410c4d26a5c4fa82205b3ff2797ffc3b51b", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/bubbline/com_github_knz_bubbline-v0.0.0-20230205122847-05558f88fdc4.zip": "0922c6fae4190a73e2023b1a56401347e29afddf1174990b8fd881433b9c75a9", + "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/catwalk/com_github_knz_catwalk-v0.1.4.zip": "f422f7974090494e54226262586c7b34fe57b33ab7d668151ca55eba8e309c1e", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/lipgloss-convert/com_github_knz_lipgloss_convert-v0.1.0.zip": "f9f9ffa12e7df4007cc60c87327d47ad42d1f71a80e360af4014674138de8bef", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/knz/strtime/com_github_knz_strtime-v0.0.0-20200318182718-be999391ffa9.zip": "c1e1b06c339798387413af1444f06f31a483d4f5278ab3a91b6cd5d7cd8d91a1", "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/konsorten/go-windows-terminal-sequences/com_github_konsorten_go_windows_terminal_sequences-v1.0.3.zip": "429b01413b972b108ea86bbde3d5e660913f3e8099190d07ccfb2f186bc6d837", diff --git a/go.mod b/go.mod index 55a7d0d49559..bc495dd40a80 100644 --- a/go.mod +++ b/go.mod @@ -159,7 +159,7 @@ require ( github.com/kisielk/gotool v1.0.0 github.com/klauspost/compress v1.15.11 github.com/klauspost/pgzip v1.2.5 - github.com/knz/bubbline v0.0.0-20230124184034-a0b62576595b + github.com/knz/bubbline v0.0.0-20230205122847-05558f88fdc4 github.com/knz/go-libedit v1.10.1 github.com/knz/strtime v0.0.0-20200318182718-be999391ffa9 github.com/kr/pretty v0.3.0 diff --git a/go.sum b/go.sum index 1f5f69b057c6..f7056c431d17 100644 --- a/go.sum +++ b/go.sum @@ -1475,10 +1475,10 @@ github.com/klauspost/pgzip v1.0.2-0.20170402124221-0bf5dcad4ada/go.mod h1:Ch1tH6 github.com/klauspost/pgzip v1.2.4/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs= github.com/klauspost/pgzip v1.2.5 h1:qnWYvvKqedOF2ulHpMG72XQol4ILEJ8k2wwRl/Km8oE= github.com/klauspost/pgzip v1.2.5/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs= -github.com/knz/bubbline v0.0.0-20230124184034-a0b62576595b h1:WkDRZUJ4jwXPpiwaprlozeoL2nULKk2BUW+7cc8MYLg= -github.com/knz/bubbline v0.0.0-20230124184034-a0b62576595b/go.mod h1:6A+ujZMesdx26uaMRz53ka0gtDTytz5+LXMxJefmUY0= -github.com/knz/catwalk v0.1.2 h1:sNLvF6WOXdvedeiCpqkpsHSGavOYxZwDsgdbKiu1IOc= -github.com/knz/catwalk v0.1.2/go.mod h1:Q+Yj4ny4AXgrOOyWyDGY/HJzmbGH8MFnsUqvCAiUT5s= +github.com/knz/bubbline v0.0.0-20230205122847-05558f88fdc4 h1:WT5NrWC3UParTXCijOl/PyfgZ9HhxCa2lAFfvxDxcIY= +github.com/knz/bubbline v0.0.0-20230205122847-05558f88fdc4/go.mod h1:ucXvyrucVy4jp/4afdKWNW1TVO73GMI72VNINzyT678= +github.com/knz/catwalk v0.1.4 h1:GgCxHbPp+nzyZBJcNL/CJd1aba4ACoeuI1lnsshAPkY= +github.com/knz/catwalk v0.1.4/go.mod h1:Q+Yj4ny4AXgrOOyWyDGY/HJzmbGH8MFnsUqvCAiUT5s= github.com/knz/lipgloss-convert v0.1.0 h1:qUPUt6r8mqvi9DIV3nBPu3kEmFyHrZtXzv0BlPBPLNQ= github.com/knz/lipgloss-convert v0.1.0/go.mod h1:S14GmtoiW/VAHqB7xEzuZOt0/G6GQ2dfjJN0fHpm30Q= github.com/knz/strtime v0.0.0-20200318182718-be999391ffa9 h1:GQE1iatYDRrIidq4Zf/9ZzKWyrTk2sXOYc1JADbkAjQ= From 6900a665368a0d4213a6df2d9737177e55c72b8d Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 5 Feb 2023 14:34:00 +0100 Subject: [PATCH 5/8] comprules: enhance completion rules Prior to this patch, completions for functions were offered after a semicolon; functions cannot appear there so we can skip functions in that case. Prior to this patch, completion of function or object names was incorrect if the cursor was inside whitespace past an identifier. In that case, the previous identifier was incorrectly used as prefix. The fix is to start with an empty prefix in that case. Release note: None --- pkg/cli/clisqlshell/testdata/complete/sql | 12 +++++ pkg/sql/comprules/rules.go | 48 ++++++++----------- .../testdata/completion_patterns/objects | 29 ++++++++++- 3 files changed, 59 insertions(+), 30 deletions(-) diff --git a/pkg/cli/clisqlshell/testdata/complete/sql b/pkg/cli/clisqlshell/testdata/complete/sql index 2fba33363fae..72a600dd5553 100644 --- a/pkg/cli/clisqlshell/testdata/complete/sql +++ b/pkg/cli/clisqlshell/testdata/complete/sql @@ -33,6 +33,18 @@ completions: "ALL" (reserved) -> "ALL" (0, 0) "ALTER" (unreserved) -> "ALTER" (0, 0) ... entries omitted ... +- "relation": + "pg_aggregate" (aggregated built-in functions (incomplete)) -> "pg_aggregate" (0, 0) + "pg_am" (index access methods (incomplete)) -> "pg_am" (0, 0) + "pg_amop" (pg_amop was created for compatibility and is currently unimplemented) -> "pg_amop" (0, 0) + "pg_amproc" (pg_amproc was created for compatibility and is currently unimplemented) -> "pg_amproc" (0, 0) + "pg_attrdef" (column default values) -> "pg_attrdef" (0, 0) + "pg_attribute" (table columns (incomplete - see also information_schema.columns)) -> "pg_attribute" (0, 0) + "pg_auth_members" (role membership) -> "pg_auth_members" (0, 0) + "pg_authid" (authorization identifiers - differs from postgres as we do not display passwords) -> "pg_authid" (0, 0) + "pg_available_extension_versions" (pg_available_extension_versions was created for compatibility and is currently u) -> "pg_available_extension_versions" (0, 0) + "pg_available_extensions" (available extensions) -> "pg_available_extensions" (0, 0) + ... entries omitted ... - "schema": "crdb_internal" () -> "crdb_internal" (0, 0) "information_schema" () -> "information_schema" (0, 0) diff --git a/pkg/sql/comprules/rules.go b/pkg/sql/comprules/rules.go index b07b8feb805f..d098755c2633 100644 --- a/pkg/sql/comprules/rules.go +++ b/pkg/sql/comprules/rules.go @@ -90,11 +90,13 @@ SELECT upper(word), return iter, err } -// A surely not qualified possible builtin name. -var compNotQualProcRe = regexp.MustCompile(`[^.](i'|_)`) +// A surely not qualified possible function name. +// Also, function names cannot appear directly after a semicolon. +var compNotQualProcRe = regexp.MustCompile(`[^.;](i'|_)`) -// A qualified possible builtin name. -var compMaybeQualProcRe = regexp.MustCompile(`i\.['_]|i\.i'`) +// A schema-qualified possible builtin name. +// Also, function names cannot appear directly after a semicolon. +var compMaybeQualProcRe = regexp.MustCompile(`[^.;]i\.(['_]|i')`) func completeFunction(ctx context.Context, c compengine.Context) (compengine.Rows, error) { // Complete function names: @@ -114,8 +116,6 @@ func completeFunction(ctx context.Context, c compengine.Context) (compengine.Row if atWord { start = int(c.RelToken(-2).Start) schemaName = c.RelToken(-2).Str - } - if atWord { prefix = c.RelToken(0).Str } end = int(c.RelToken(0).End) @@ -172,7 +172,7 @@ ORDER BY 1,2,3,4,5 } // A database name can only occur after a keyword or a comma (,). -var compDbRe = regexp.MustCompile(`i(i'|_)|,(_|i')`) +var compDbRe = regexp.MustCompile(`[i,](i'|_)`) func completeDatabase(ctx context.Context, c compengine.Context) (compengine.Rows, error) { var prefix string @@ -245,7 +245,7 @@ func completeObjectInCurrentDatabase( var prefix string var start, end int switch { - case atWord: + case c.CursorInToken() && atWord: curTok := c.RelToken(0) prefix = curTok.Str start = int(curTok.Start) @@ -287,14 +287,14 @@ func completeSchemaInCurrentDatabase( var prefix string var start, end int switch { - case c.CursorInSpace(): - start = c.QueryPos() - end = start - default: + case c.CursorInToken() && c.AtWord(): curTok := c.RelToken(0) prefix = curTok.Str start = int(curTok.Start) end = int(curTok.End) + default: + start = c.QueryPos() + end = start } c.Trace("completing for %q (%d,%d)", prefix, start, end) @@ -372,14 +372,21 @@ func completeObjectInOtherDatabase( var schema string atWord := c.AtWord() sketch := c.Sketch() + var dbTok scanner.InspectToken switch { case compOneQualPrefixRe.MatchString(sketch): schema = "public" + dbTok = c.RelToken(-1) + if atWord { + dbTok = c.RelToken(-2) + } case compTwoQualPrefixRe.MatchString(sketch): schemaTok := c.RelToken(-1) + dbTok = c.RelToken(-3) if atWord { schemaTok = c.RelToken(-2) + dbTok = c.RelToken(-4) } schema = schemaTok.Str @@ -387,27 +394,12 @@ func completeObjectInOtherDatabase( c.Trace("not completing") return nil, nil } - - var dbTok scanner.InspectToken - switch { - case compOneQualPrefixRe.MatchString(sketch): - dbTok = c.RelToken(-1) - if atWord { - dbTok = c.RelToken(-2) - } - - case compTwoQualPrefixRe.MatchString(sketch): - dbTok = c.RelToken(-3) - if atWord { - dbTok = c.RelToken(-4) - } - } dbname := dbTok.Str var prefix string var start, end int switch { - case atWord: + case c.CursorInToken() && atWord: curTok := c.RelToken(0) prefix = curTok.Str start = int(curTok.Start) diff --git a/pkg/sql/comprules/testdata/completion_patterns/objects b/pkg/sql/comprules/testdata/completion_patterns/objects index 7e2254bce92c..71c5e2685883 100644 --- a/pkg/sql/comprules/testdata/completion_patterns/objects +++ b/pkg/sql/comprules/testdata/completion_patterns/objects @@ -2,6 +2,31 @@ filter objects: ---- +subtest after_keyword + +comp at=8 +select +---- +i_ + ^ +-- +objects: completing for "" (8,8), schema: IN (TABLE unnest(current_schemas(true))) +--sql: +WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname IN (TABLE unnest(current_schemas(true)))), + t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +SELECT relname AS completion, + 'relation' AS category, + substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, + $2:::INT AS start, + $3:::INT AS end + FROM t +LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc + ON t.oid = cc.object_id AND cc.type = 'TableCommentType' + WHERE left(relname, length($1:::STRING)) = $1::STRING +--placeholders: []interface {}{"", 8, 8} + +subtest end + subtest at_ident comp at=8 @@ -31,7 +56,7 @@ select xor ii_ ^ -- -objects: completing for "xor" (7,10), schema: IN (TABLE unnest(current_schemas(true))) +objects: completing for "" (12,12), schema: IN (TABLE unnest(current_schemas(true))) --sql: WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname IN (TABLE unnest(current_schemas(true)))), t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) @@ -44,7 +69,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING ---placeholders: []interface {}{"xor", 7, 10} +--placeholders: []interface {}{"", 12, 12} subtest end From 765de897882e5c6bde60b497422a536b99af5fc2 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 5 Feb 2023 14:34:28 +0100 Subject: [PATCH 6/8] comprules: pre-sort the completion results This makes the list of completions more deterministic. Release note: None --- pkg/cli/clisqlshell/testdata/complete/sql | 4 ++-- pkg/sql/comprules/rules.go | 7 +++++- .../testdata/completion_patterns/builtins | 24 +++++++++---------- .../testdata/completion_patterns/databases | 5 ++++ .../testdata/completion_patterns/objects | 8 +++++++ .../completion_patterns/objects_other | 8 +++++++ .../testdata/completion_patterns/schemas | 4 ++++ .../completion_patterns/schemas_other | 5 ++++ 8 files changed, 50 insertions(+), 15 deletions(-) diff --git a/pkg/cli/clisqlshell/testdata/complete/sql b/pkg/cli/clisqlshell/testdata/complete/sql index 72a600dd5553..29762aed0ada 100644 --- a/pkg/cli/clisqlshell/testdata/complete/sql +++ b/pkg/cli/clisqlshell/testdata/complete/sql @@ -34,6 +34,8 @@ completions: "ALTER" (unreserved) -> "ALTER" (0, 0) ... entries omitted ... - "relation": + "geography_columns" (Shows all defined geography columns. Matches PostGIS' geography_columns function) -> "geography_columns" (0, 0) + "geometry_columns" (Shows all defined geometry columns. Matches PostGIS' geometry_columns functional) -> "geometry_columns" (0, 0) "pg_aggregate" (aggregated built-in functions (incomplete)) -> "pg_aggregate" (0, 0) "pg_am" (index access methods (incomplete)) -> "pg_am" (0, 0) "pg_amop" (pg_amop was created for compatibility and is currently unimplemented) -> "pg_amop" (0, 0) @@ -42,8 +44,6 @@ completions: "pg_attribute" (table columns (incomplete - see also information_schema.columns)) -> "pg_attribute" (0, 0) "pg_auth_members" (role membership) -> "pg_auth_members" (0, 0) "pg_authid" (authorization identifiers - differs from postgres as we do not display passwords) -> "pg_authid" (0, 0) - "pg_available_extension_versions" (pg_available_extension_versions was created for compatibility and is currently u) -> "pg_available_extension_versions" (0, 0) - "pg_available_extensions" (available extensions) -> "pg_available_extensions" (0, 0) ... entries omitted ... - "schema": "crdb_internal" () -> "crdb_internal" (0, 0) diff --git a/pkg/sql/comprules/rules.go b/pkg/sql/comprules/rules.go index d098755c2633..8ba878dcfa64 100644 --- a/pkg/sql/comprules/rules.go +++ b/pkg/sql/comprules/rules.go @@ -165,7 +165,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 ` iter, err := c.Query(ctx, query, prefix, start, end, schemaName) return iter, err @@ -208,6 +208,7 @@ LEFT OUTER JOIN system.public.comments sc ON d.oid = sc.object_id AND sc.type = 0 WHERE left(datname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 ` iter, err := c.Query(ctx, query, prefix, start, end) return iter, err @@ -268,6 +269,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 ` query := fmt.Sprintf(queryT, schema) iter, err := c.Query(ctx, query, prefix, start, end) @@ -308,6 +310,7 @@ SELECT nspname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'SchemaCommentType' WHERE left(nspname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 ` iter, err := c.Query(ctx, query, prefix, start, end) return iter, err @@ -358,6 +361,7 @@ SELECT schema_name AS completion, FROM "".information_schema.schemata WHERE catalog_name = $4:::STRING AND left(schema_name, length($1:::STRING)) = $1:::STRING +ORDER BY 1,3,4,5 ` iter, err := c.Query(ctx, query, prefix, start, end, dbname) return iter, err @@ -426,6 +430,7 @@ SELECT name AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.table_id = cc.object_id AND cc.type = 'TableCommentType' +ORDER BY 1,3,4,5 ` iter, err := c.Query(ctx, query, prefix, start, end, dbname, schema) return iter, err diff --git a/pkg/sql/comprules/testdata/completion_patterns/builtins b/pkg/sql/comprules/testdata/completion_patterns/builtins index 8c05d26253d4..20024e2c2acc 100644 --- a/pkg/sql/comprules/testdata/completion_patterns/builtins +++ b/pkg/sql/comprules/testdata/completion_patterns/builtins @@ -30,7 +30,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 7, 10, ""} comp at=12 @@ -59,7 +59,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 12, 12, ""} subtest end @@ -92,7 +92,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 7, 12, "a"} comp at=10 @@ -121,7 +121,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 7, 9, "a"} subtest end @@ -154,7 +154,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 7, 24, "crdb_internal"} comp at=21 @@ -183,7 +183,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 7, 21, "crdb_internal"} @@ -213,7 +213,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 7, 21, "pg_catalog"} comp at=18 @@ -242,7 +242,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 7, 18, "pg_catalog"} @@ -272,7 +272,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 7, 29, "information_schema"} comp at=26 @@ -301,7 +301,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 7, 26, "information_schema"} subtest end @@ -334,7 +334,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 7, 23, "pg_catalog"} # Quoted uppercase is an entire schema entirely. @@ -364,7 +364,7 @@ SELECT DISTINCT $2:::INT AS start, $3:::INT AS end FROM p -ORDER BY 1,2,3,4,5 +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 7, 23, "PG_CATALOG"} diff --git a/pkg/sql/comprules/testdata/completion_patterns/databases b/pkg/sql/comprules/testdata/completion_patterns/databases index 77507abe2580..bf108db6a6e7 100644 --- a/pkg/sql/comprules/testdata/completion_patterns/databases +++ b/pkg/sql/comprules/testdata/completion_patterns/databases @@ -23,6 +23,7 @@ LEFT OUTER JOIN system.public.comments sc ON d.oid = sc.object_id AND sc.type = 0 WHERE left(datname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 8, 8} comp at=8 @@ -44,6 +45,7 @@ LEFT OUTER JOIN system.public.comments sc ON d.oid = sc.object_id AND sc.type = 0 WHERE left(datname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"foo", 7, 10} comp at=8 @@ -65,6 +67,7 @@ LEFT OUTER JOIN system.public.comments sc ON d.oid = sc.object_id AND sc.type = 0 WHERE left(datname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"FOO", 7, 11} @@ -91,6 +94,7 @@ LEFT OUTER JOIN system.public.comments sc ON d.oid = sc.object_id AND sc.type = 0 WHERE left(datname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 12, 12} comp at=10 @@ -112,6 +116,7 @@ LEFT OUTER JOIN system.public.comments sc ON d.oid = sc.object_id AND sc.type = 0 WHERE left(datname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"abc", 9, 12} subtest end diff --git a/pkg/sql/comprules/testdata/completion_patterns/objects b/pkg/sql/comprules/testdata/completion_patterns/objects index 71c5e2685883..35474b4f0a20 100644 --- a/pkg/sql/comprules/testdata/completion_patterns/objects +++ b/pkg/sql/comprules/testdata/completion_patterns/objects @@ -23,6 +23,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 8, 8} subtest end @@ -48,6 +49,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 7, 10} comp at=12 @@ -69,6 +71,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 12, 12} subtest end @@ -94,6 +97,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 9, 12} comp at=10 @@ -115,6 +119,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 10, 10} comp at=9 @@ -136,6 +141,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 9, 9} @@ -162,6 +168,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 20, 23} # Quoted uppercase is a different schema entirely. @@ -184,6 +191,7 @@ SELECT relname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' WHERE left(relname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 20, 23} diff --git a/pkg/sql/comprules/testdata/completion_patterns/objects_other b/pkg/sql/comprules/testdata/completion_patterns/objects_other index 117304455e33..85ac6a94d1fb 100644 --- a/pkg/sql/comprules/testdata/completion_patterns/objects_other +++ b/pkg/sql/comprules/testdata/completion_patterns/objects_other @@ -47,6 +47,7 @@ SELECT name AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.table_id = cc.object_id AND cc.type = 'TableCommentType' +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 9, 12, "a", "public"} comp at=10 @@ -72,6 +73,7 @@ SELECT name AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.table_id = cc.object_id AND cc.type = 'TableCommentType' +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 10, 10, "a", "public"} comp at=9 @@ -97,6 +99,7 @@ SELECT name AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.table_id = cc.object_id AND cc.type = 'TableCommentType' +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 9, 9, "a", "public"} @@ -127,6 +130,7 @@ SELECT name AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.table_id = cc.object_id AND cc.type = 'TableCommentType' +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 11, 14, "a", "b"} comp at=12 @@ -152,6 +156,7 @@ SELECT name AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.table_id = cc.object_id AND cc.type = 'TableCommentType' +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 12, 12, "a", "b"} comp at=11 @@ -177,6 +182,7 @@ SELECT name AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.table_id = cc.object_id AND cc.type = 'TableCommentType' +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 11, 11, "a", "b"} @@ -207,6 +213,7 @@ SELECT name AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.table_id = cc.object_id AND cc.type = 'TableCommentType' +ORDER BY 1,3,4,5 --placeholders: []interface {}{"pg_", 12, 16, "mydb", "public"} # Quoted uppercase is an entire schema entirely. @@ -233,6 +240,7 @@ SELECT name AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.table_id = cc.object_id AND cc.type = 'TableCommentType' +ORDER BY 1,3,4,5 --placeholders: []interface {}{"PG_", 12, 16, "mydb", "public"} diff --git a/pkg/sql/comprules/testdata/completion_patterns/schemas b/pkg/sql/comprules/testdata/completion_patterns/schemas index ea9e2eaf9610..dd18a8ee4dbd 100644 --- a/pkg/sql/comprules/testdata/completion_patterns/schemas +++ b/pkg/sql/comprules/testdata/completion_patterns/schemas @@ -21,6 +21,7 @@ SELECT nspname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'SchemaCommentType' WHERE left(nspname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 7, 10} comp at=12 @@ -40,6 +41,7 @@ SELECT nspname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'SchemaCommentType' WHERE left(nspname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 12, 12} subtest end @@ -92,6 +94,7 @@ SELECT nspname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'SchemaCommentType' WHERE left(nspname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"pg_", 7, 11} # Quoted uppercase is an entire schema entirely. @@ -112,6 +115,7 @@ SELECT nspname AS completion, LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'SchemaCommentType' WHERE left(nspname, length($1:::STRING)) = $1::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"PG_", 7, 11} diff --git a/pkg/sql/comprules/testdata/completion_patterns/schemas_other b/pkg/sql/comprules/testdata/completion_patterns/schemas_other index f2d81bbe368d..e63f4a314053 100644 --- a/pkg/sql/comprules/testdata/completion_patterns/schemas_other +++ b/pkg/sql/comprules/testdata/completion_patterns/schemas_other @@ -40,6 +40,7 @@ SELECT schema_name AS completion, FROM "".information_schema.schemata WHERE catalog_name = $4:::STRING AND left(schema_name, length($1:::STRING)) = $1:::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"xor", 9, 12, "a"} comp at=10 @@ -58,6 +59,7 @@ SELECT schema_name AS completion, FROM "".information_schema.schemata WHERE catalog_name = $4:::STRING AND left(schema_name, length($1:::STRING)) = $1:::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 10, 10, "a"} comp at=9 @@ -76,6 +78,7 @@ SELECT schema_name AS completion, FROM "".information_schema.schemata WHERE catalog_name = $4:::STRING AND left(schema_name, length($1:::STRING)) = $1:::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"", 9, 9, "a"} @@ -99,6 +102,7 @@ SELECT schema_name AS completion, FROM "".information_schema.schemata WHERE catalog_name = $4:::STRING AND left(schema_name, length($1:::STRING)) = $1:::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"pg_", 12, 16, "mydb"} # Quoted uppercase is an entire schema entirely. @@ -118,6 +122,7 @@ SELECT schema_name AS completion, FROM "".information_schema.schemata WHERE catalog_name = $4:::STRING AND left(schema_name, length($1:::STRING)) = $1:::STRING +ORDER BY 1,3,4,5 --placeholders: []interface {}{"PG_", 12, 16, "mydb"} From b4c088f1c62542685cf830aa004f4bff89c9bb87 Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Sun, 5 Feb 2023 14:42:06 +0100 Subject: [PATCH 7/8] comprules: avoid including pg_catalog tables by default Previous to this patch, since `pg_catalog` is in the default `search_path`, the tab completion for relations would include all the `pg_catalog` tables. This is generally undesirable: there are very many of them. This patch enhances the situation by only completing pg_ relations in the following cases: - when the cursor is position after an explicit schema qualification; in which case we're going to filter to that schema anyway. - when the completion prefix already includes the `pg_` prefix, in which case we can assume the user wants to see these tables. Release note: None --- pkg/cli/clisqlshell/testdata/complete/sql | 30 +++++-- pkg/sql/comprules/rules.go | 19 +++- .../testdata/completion_patterns/objects | 88 ++++++++++++------- 3 files changed, 92 insertions(+), 45 deletions(-) diff --git a/pkg/cli/clisqlshell/testdata/complete/sql b/pkg/cli/clisqlshell/testdata/complete/sql index 29762aed0ada..a2e672364b8b 100644 --- a/pkg/cli/clisqlshell/testdata/complete/sql +++ b/pkg/cli/clisqlshell/testdata/complete/sql @@ -36,15 +36,7 @@ completions: - "relation": "geography_columns" (Shows all defined geography columns. Matches PostGIS' geography_columns function) -> "geography_columns" (0, 0) "geometry_columns" (Shows all defined geometry columns. Matches PostGIS' geometry_columns functional) -> "geometry_columns" (0, 0) - "pg_aggregate" (aggregated built-in functions (incomplete)) -> "pg_aggregate" (0, 0) - "pg_am" (index access methods (incomplete)) -> "pg_am" (0, 0) - "pg_amop" (pg_amop was created for compatibility and is currently unimplemented) -> "pg_amop" (0, 0) - "pg_amproc" (pg_amproc was created for compatibility and is currently unimplemented) -> "pg_amproc" (0, 0) - "pg_attrdef" (column default values) -> "pg_attrdef" (0, 0) - "pg_attribute" (table columns (incomplete - see also information_schema.columns)) -> "pg_attribute" (0, 0) - "pg_auth_members" (role membership) -> "pg_auth_members" (0, 0) - "pg_authid" (authorization identifiers - differs from postgres as we do not display passwords) -> "pg_authid" (0, 0) - ... entries omitted ... + "spatial_ref_sys" (Shows all defined Spatial Reference Identifiers (SRIDs). Matches PostGIS' spatia) -> "spatial_ref_sys" (0, 0) - "schema": "crdb_internal" () -> "crdb_internal" (0, 0) "information_schema" () -> "information_schema" (0, 0) @@ -124,3 +116,23 @@ completions: - "functions": "array_length(" ((from schema pg_catalog) Calculates the length of `input` on the provided `array_dimension`) -> "array_length(" (0, 7) "array_lower(" ((from schema pg_catalog) Calculates the minimum value of `input` on the provided `array_dimension`) -> "array_lower(" (0, 7) + +# Complete pg_catalog tables after a pg_catalog prefix. +complete +select * from pg_catalog.pg_pro@ +---- +complete 0 31 +msg: "" +completions: +- "relation": + "pg_proc" (built-in functions (incomplete)) -> "pg_proc" (0, 6) + +# Complete pg_catalog tables if the user has typed pg_ already. +complete +select * from pg_pro@ +---- +complete 0 20 +msg: "" +completions: +- "relation": + "pg_proc" (built-in functions (incomplete)) -> "pg_proc" (0, 6) diff --git a/pkg/sql/comprules/rules.go b/pkg/sql/comprules/rules.go index 8ba878dcfa64..f6156c7ae8eb 100644 --- a/pkg/sql/comprules/rules.go +++ b/pkg/sql/comprules/rules.go @@ -227,6 +227,7 @@ func completeObjectInCurrentDatabase( var schema string atWord := c.AtWord() sketch := c.Sketch() + hasSchemaPrefix := false switch { case compLocalTableRe.MatchString(sketch): schema = "IN (TABLE unnest(current_schemas(true)))" @@ -237,6 +238,7 @@ func completeObjectInCurrentDatabase( schemaTok = c.RelToken(-2) } schema = "= " + lexbase.EscapeSQLString(schemaTok.Str) + hasSchemaPrefix = true default: c.Trace("not completing") @@ -257,9 +259,19 @@ func completeObjectInCurrentDatabase( } c.Trace("completing for %q (%d,%d), schema: %s", prefix, start, end, schema) + // We only include pg_catalog relations in the following cases: + // + // - when the cursor is position after an explicit schema qualification; + // in which case we're going to filter to that schema anyway. + // - when the completion prefix already includes the `pg_` prefix, + // in which case we can assume the user wants to see these tables. const queryT = ` -WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname %s), - t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +WITH n AS (SELECT oid, nspname FROM pg_catalog.pg_namespace WHERE nspname %s), + t AS (SELECT c.oid, relname FROM pg_catalog.pg_class c + JOIN n ON n.oid = c.relnamespace + WHERE reltype != 0 + AND left(relname, length($1:::STRING)) = $1::STRING + AND (nspname != 'pg_catalog' OR $4:::BOOL OR left($1:::STRING, 3) = 'pg_')) SELECT relname AS completion, 'relation' AS category, substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, @@ -268,11 +280,10 @@ SELECT relname AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' - WHERE left(relname, length($1:::STRING)) = $1::STRING ORDER BY 1,3,4,5 ` query := fmt.Sprintf(queryT, schema) - iter, err := c.Query(ctx, query, prefix, start, end) + iter, err := c.Query(ctx, query, prefix, start, end, hasSchemaPrefix) return iter, err } diff --git a/pkg/sql/comprules/testdata/completion_patterns/objects b/pkg/sql/comprules/testdata/completion_patterns/objects index 35474b4f0a20..5e8b5e669e70 100644 --- a/pkg/sql/comprules/testdata/completion_patterns/objects +++ b/pkg/sql/comprules/testdata/completion_patterns/objects @@ -12,8 +12,12 @@ i_ -- objects: completing for "" (8,8), schema: IN (TABLE unnest(current_schemas(true))) --sql: -WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname IN (TABLE unnest(current_schemas(true)))), - t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +WITH n AS (SELECT oid, nspname FROM pg_catalog.pg_namespace WHERE nspname IN (TABLE unnest(current_schemas(true)))), + t AS (SELECT c.oid, relname FROM pg_catalog.pg_class c + JOIN n ON n.oid = c.relnamespace + WHERE reltype != 0 + AND left(relname, length($1:::STRING)) = $1::STRING + AND (nspname != 'pg_catalog' OR $4:::BOOL OR left($1:::STRING, 3) = 'pg_')) SELECT relname AS completion, 'relation' AS category, substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, @@ -22,9 +26,8 @@ SELECT relname AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' - WHERE left(relname, length($1:::STRING)) = $1::STRING ORDER BY 1,3,4,5 ---placeholders: []interface {}{"", 8, 8} +--placeholders: []interface {}{"", 8, 8, false} subtest end @@ -38,8 +41,12 @@ ii' -- objects: completing for "xor" (7,10), schema: IN (TABLE unnest(current_schemas(true))) --sql: -WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname IN (TABLE unnest(current_schemas(true)))), - t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +WITH n AS (SELECT oid, nspname FROM pg_catalog.pg_namespace WHERE nspname IN (TABLE unnest(current_schemas(true)))), + t AS (SELECT c.oid, relname FROM pg_catalog.pg_class c + JOIN n ON n.oid = c.relnamespace + WHERE reltype != 0 + AND left(relname, length($1:::STRING)) = $1::STRING + AND (nspname != 'pg_catalog' OR $4:::BOOL OR left($1:::STRING, 3) = 'pg_')) SELECT relname AS completion, 'relation' AS category, substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, @@ -48,9 +55,8 @@ SELECT relname AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' - WHERE left(relname, length($1:::STRING)) = $1::STRING ORDER BY 1,3,4,5 ---placeholders: []interface {}{"xor", 7, 10} +--placeholders: []interface {}{"xor", 7, 10, false} comp at=12 select xor @@ -60,8 +66,12 @@ ii_ -- objects: completing for "" (12,12), schema: IN (TABLE unnest(current_schemas(true))) --sql: -WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname IN (TABLE unnest(current_schemas(true)))), - t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +WITH n AS (SELECT oid, nspname FROM pg_catalog.pg_namespace WHERE nspname IN (TABLE unnest(current_schemas(true)))), + t AS (SELECT c.oid, relname FROM pg_catalog.pg_class c + JOIN n ON n.oid = c.relnamespace + WHERE reltype != 0 + AND left(relname, length($1:::STRING)) = $1::STRING + AND (nspname != 'pg_catalog' OR $4:::BOOL OR left($1:::STRING, 3) = 'pg_')) SELECT relname AS completion, 'relation' AS category, substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, @@ -70,9 +80,8 @@ SELECT relname AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' - WHERE left(relname, length($1:::STRING)) = $1::STRING ORDER BY 1,3,4,5 ---placeholders: []interface {}{"", 12, 12} +--placeholders: []interface {}{"", 12, 12, false} subtest end @@ -86,8 +95,12 @@ ii.i' -- objects: completing for "xor" (9,12), schema: = 'a' --sql: -WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname = 'a'), - t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +WITH n AS (SELECT oid, nspname FROM pg_catalog.pg_namespace WHERE nspname = 'a'), + t AS (SELECT c.oid, relname FROM pg_catalog.pg_class c + JOIN n ON n.oid = c.relnamespace + WHERE reltype != 0 + AND left(relname, length($1:::STRING)) = $1::STRING + AND (nspname != 'pg_catalog' OR $4:::BOOL OR left($1:::STRING, 3) = 'pg_')) SELECT relname AS completion, 'relation' AS category, substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, @@ -96,9 +109,8 @@ SELECT relname AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' - WHERE left(relname, length($1:::STRING)) = $1::STRING ORDER BY 1,3,4,5 ---placeholders: []interface {}{"xor", 9, 12} +--placeholders: []interface {}{"xor", 9, 12, true} comp at=10 select a. @@ -108,8 +120,12 @@ ii._ -- objects: completing for "" (10,10), schema: = 'a' --sql: -WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname = 'a'), - t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +WITH n AS (SELECT oid, nspname FROM pg_catalog.pg_namespace WHERE nspname = 'a'), + t AS (SELECT c.oid, relname FROM pg_catalog.pg_class c + JOIN n ON n.oid = c.relnamespace + WHERE reltype != 0 + AND left(relname, length($1:::STRING)) = $1::STRING + AND (nspname != 'pg_catalog' OR $4:::BOOL OR left($1:::STRING, 3) = 'pg_')) SELECT relname AS completion, 'relation' AS category, substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, @@ -118,9 +134,8 @@ SELECT relname AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' - WHERE left(relname, length($1:::STRING)) = $1::STRING ORDER BY 1,3,4,5 ---placeholders: []interface {}{"", 10, 10} +--placeholders: []interface {}{"", 10, 10, true} comp at=9 select a. @@ -130,8 +145,12 @@ ii.' -- objects: completing for "" (9,9), schema: = 'a' --sql: -WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname = 'a'), - t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +WITH n AS (SELECT oid, nspname FROM pg_catalog.pg_namespace WHERE nspname = 'a'), + t AS (SELECT c.oid, relname FROM pg_catalog.pg_class c + JOIN n ON n.oid = c.relnamespace + WHERE reltype != 0 + AND left(relname, length($1:::STRING)) = $1::STRING + AND (nspname != 'pg_catalog' OR $4:::BOOL OR left($1:::STRING, 3) = 'pg_')) SELECT relname AS completion, 'relation' AS category, substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, @@ -140,9 +159,8 @@ SELECT relname AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' - WHERE left(relname, length($1:::STRING)) = $1::STRING ORDER BY 1,3,4,5 ---placeholders: []interface {}{"", 9, 9} +--placeholders: []interface {}{"", 9, 9, true} subtest end @@ -157,8 +175,12 @@ ii.i' -- objects: completing for "xor" (20,23), schema: = 'pg_catalog' --sql: -WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname = 'pg_catalog'), - t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +WITH n AS (SELECT oid, nspname FROM pg_catalog.pg_namespace WHERE nspname = 'pg_catalog'), + t AS (SELECT c.oid, relname FROM pg_catalog.pg_class c + JOIN n ON n.oid = c.relnamespace + WHERE reltype != 0 + AND left(relname, length($1:::STRING)) = $1::STRING + AND (nspname != 'pg_catalog' OR $4:::BOOL OR left($1:::STRING, 3) = 'pg_')) SELECT relname AS completion, 'relation' AS category, substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, @@ -167,9 +189,8 @@ SELECT relname AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' - WHERE left(relname, length($1:::STRING)) = $1::STRING ORDER BY 1,3,4,5 ---placeholders: []interface {}{"xor", 20, 23} +--placeholders: []interface {}{"xor", 20, 23, true} # Quoted uppercase is a different schema entirely. comp at=22 @@ -180,8 +201,12 @@ ii.i' -- objects: completing for "xor" (20,23), schema: = 'PG_CATALOG' --sql: -WITH n AS (SELECT oid FROM pg_catalog.pg_namespace WHERE nspname = 'PG_CATALOG'), - t AS (SELECT oid, relname FROM pg_catalog.pg_class WHERE reltype != 0 AND relnamespace IN (TABLE n)) +WITH n AS (SELECT oid, nspname FROM pg_catalog.pg_namespace WHERE nspname = 'PG_CATALOG'), + t AS (SELECT c.oid, relname FROM pg_catalog.pg_class c + JOIN n ON n.oid = c.relnamespace + WHERE reltype != 0 + AND left(relname, length($1:::STRING)) = $1::STRING + AND (nspname != 'pg_catalog' OR $4:::BOOL OR left($1:::STRING, 3) = 'pg_')) SELECT relname AS completion, 'relation' AS category, substr(COALESCE(cc.comment, ''), e'[^\n]{0,80}') as description, @@ -190,9 +215,8 @@ SELECT relname AS completion, FROM t LEFT OUTER JOIN "".crdb_internal.kv_catalog_comments cc ON t.oid = cc.object_id AND cc.type = 'TableCommentType' - WHERE left(relname, length($1:::STRING)) = $1::STRING ORDER BY 1,3,4,5 ---placeholders: []interface {}{"xor", 20, 23} +--placeholders: []interface {}{"xor", 20, 23, true} subtest end From 30fd1b05914f83d1e759cf2ab2254b4a8e35ef4c Mon Sep 17 00:00:00 2001 From: Raphael 'kena' Poss Date: Mon, 6 Feb 2023 21:32:06 +0100 Subject: [PATCH 8/8] compengine,comprules: API rename The API `AtWord()` was hiding its true purpose, it returns true also when the cursor is in the middle of whitespace following a word. This change renames the method accordingly to `AtWordOrInSpaceFollowingWord()`. Release note: None --- pkg/sql/compengine/api.go | 8 +++++--- pkg/sql/compengine/engine.go | 4 ++-- pkg/sql/comprules/rules.go | 14 +++++++------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/pkg/sql/compengine/api.go b/pkg/sql/compengine/api.go index 95f0d87d0002..ae1a35aa4b2d 100644 --- a/pkg/sql/compengine/api.go +++ b/pkg/sql/compengine/api.go @@ -71,9 +71,11 @@ type Context interface { // returned. RelToken(pos int) scanner.InspectToken - // AtWord is equivalent to .RelMarker(0) == MarkIdentOrKeyword, - // and is provided for convenience. - AtWord() bool + // AtWordOrInSpaceFollowingWord is equivalent to .RelMarker(0) == + // MarkIdentOrKeyword, and is provided for convenience. This returns + // true both when the cursor is _on_ an identifier/keyword, or _at + // any whitespace position afterwards_. + AtWordOrInSpaceFollowingWord() bool // Query perform a SQL query. Query(ctx context.Context, query string, args ...interface{}) (Rows, error) diff --git a/pkg/sql/compengine/engine.go b/pkg/sql/compengine/engine.go index ca93f2e3acfb..421968ded22c 100644 --- a/pkg/sql/compengine/engine.go +++ b/pkg/sql/compengine/engine.go @@ -221,8 +221,8 @@ func (c *completions) RelToken(idx int) scanner.InspectToken { return c.tokens[off] } -// AtWord implements the Context interface. -func (c *completions) AtWord() bool { +// AtWordOrInSpaceFollowingWord implements the Context interface. +func (c *completions) AtWordOrInSpaceFollowingWord() bool { return c.RelMarker(0) == MarkIdentOrKeyword } diff --git a/pkg/sql/comprules/rules.go b/pkg/sql/comprules/rules.go index f6156c7ae8eb..3b6620fd5048 100644 --- a/pkg/sql/comprules/rules.go +++ b/pkg/sql/comprules/rules.go @@ -67,7 +67,7 @@ func completeKeyword(ctx context.Context, c compengine.Context) (compengine.Rows start = c.QueryPos() end = start - case c.AtWord() && !curTok.Quoted: + case c.AtWordOrInSpaceFollowingWord() && !curTok.Quoted: prefix = curTok.Str start = int(curTok.Start) end = int(curTok.End) @@ -107,7 +107,7 @@ func completeFunction(ctx context.Context, c compengine.Context) (compengine.Row var prefix string var start, end int var schemaName string - atWord := c.AtWord() + atWord := c.AtWordOrInSpaceFollowingWord() sketch := c.Sketch() switch { case compMaybeQualProcRe.MatchString(sketch): @@ -183,7 +183,7 @@ func completeDatabase(ctx context.Context, c compengine.Context) (compengine.Row c.Trace("not completing") return nil, nil - case c.CursorInToken() && c.AtWord(): + case c.CursorInToken() && c.AtWordOrInSpaceFollowingWord(): curTok := c.RelToken(0) prefix = curTok.Str start = int(curTok.Start) @@ -225,7 +225,7 @@ func completeObjectInCurrentDatabase( ctx context.Context, c compengine.Context, ) (compengine.Rows, error) { var schema string - atWord := c.AtWord() + atWord := c.AtWordOrInSpaceFollowingWord() sketch := c.Sketch() hasSchemaPrefix := false switch { @@ -300,7 +300,7 @@ func completeSchemaInCurrentDatabase( var prefix string var start, end int switch { - case c.CursorInToken() && c.AtWord(): + case c.CursorInToken() && c.AtWordOrInSpaceFollowingWord(): curTok := c.RelToken(0) prefix = curTok.Str start = int(curTok.Start) @@ -331,7 +331,7 @@ func completeSchemaInOtherDatabase( ctx context.Context, c compengine.Context, ) (compengine.Rows, error) { var dbname string - atWord := c.AtWord() + atWord := c.AtWordOrInSpaceFollowingWord() switch { case compOneQualPrefixRe.MatchString(c.Sketch()): dbTok := c.RelToken(-1) @@ -385,7 +385,7 @@ func completeObjectInOtherDatabase( ctx context.Context, c compengine.Context, ) (compengine.Rows, error) { var schema string - atWord := c.AtWord() + atWord := c.AtWordOrInSpaceFollowingWord() sketch := c.Sketch() var dbTok scanner.InspectToken switch {