Skip to content

Commit

Permalink
comprules: avoid including pg_catalog tables by default
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@knz
knz committed Feb 6, 2023
1 parent 765de89 commit b4c088f
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 45 deletions.
30 changes: 21 additions & 9 deletions pkg/cli/clisqlshell/testdata/complete/sql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
19 changes: 15 additions & 4 deletions pkg/sql/comprules/rules.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)))"
Expand All @@ -237,6 +238,7 @@ func completeObjectInCurrentDatabase(
schemaTok = c.RelToken(-2)
}
schema = "= " + lexbase.EscapeSQLString(schemaTok.Str)
hasSchemaPrefix = true

default:
c.Trace("not completing")
Expand All @@ -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,
Expand All @@ -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
}

Expand Down
88 changes: 56 additions & 32 deletions pkg/sql/comprules/testdata/completion_patterns/objects
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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

Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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

Expand All @@ -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,
Expand All @@ -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.
Expand All @@ -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,
Expand All @@ -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.
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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

0 comments on commit b4c088f

Please sign in to comment.