clisqlshell: new infrastructure for describe commands #88061

knz · 2022-09-16T18:10:31Z

The SQL shell (cockroach sql, demo) now
supports the client-side commands \l, \dn, \d, \di, \dm,
\ds, \dt, \dv, \dC, \dT, \dd, \dg, \du, \df and \dd in a
way similar to psql, including the modifier flags S and +, for
convenience for users migrating from PostgreSQL.

A notable difference is that when a pattern argument is specified, it
should use the SQL "LIKE" syntax (with % representing the wildcard
character) instead of PostgreSQL's glob-like syntax (with *
representing wildcards).

Issues discovered:

cockroach-teamcity · 2022-09-16T18:10:40Z

This change is

nvanbenschoten

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @knz, @nvanbenschoten, and @rafiss)

pkg/cli/clisqlshell/sql.go line 1236 at r2 (raw file):

	cmd := strings.Fields(line)
	if strings.HasPrefix(cmd[0], `\d`) && cmd[0] != `\demo` {

There are a few additional informational patterns that don't start with \d. For instance, \s and \z. Do we want to support those as well?

knz · 2022-09-16T19:53:08Z

There are a few additional informational patterns that don't start with \d. For instance, \s and \z. Do we want to support those as well?

Done

knz · 2022-09-16T20:31:55Z

added the unit tests too

knz · 2022-09-16T21:58:29Z

now with datadriven tests too

knz · 2022-09-17T21:03:12Z

Remaining to do: list indexes and constraints under table details.

knz · 2022-09-18T14:20:25Z

Ok I have completed the functionality -- this is more or less ready for review, the followup issues listed at top notwithstanding.

93157: loqrecovery: add replica info collection to admin server r=erikgrinaker,knz a=aliher1911 This commit adds replica info and range descriptor collection from partially available cluster that lost quorum on some ranges. Collection is done using AdminServer calls. Cluster wide call performs fan-out using node info from gossip. Local replica info collection on nodes is done by scanning storages. Release note: File format used for transient loss of quorum recovery files has changed. It is not possible to use replica info files generated by earlier versions to be used with current and future versions. Fixes #93040 Note that we recovery doesn't need collect command to be "remote capable" but allowing this makes development and debugging simpler as you can create local snapshot from a cluster that you can subsequently verify you planning against. 95289: sql: populate `pg_proc.prokind` r=rafiss a=knz Needed for #88061. Fixes #95288. Epic: CRDB-23454 Co-authored-by: Oleg Afanasyev <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]>

94239: loqrecovery: use captured meta range content for LOQ plans r=erikgrinaker a=aliher1911 Note: only last commit belongs to this PR. Will update description once #93157 lands. Previously loss of quorum recovery planner was using local replica info collected from all nodes to find source of truth for replicas that lost quorum. With online approach local info snapshots don't have atomicity. This could cause planner to fail if available replicas are caught in different states on different nodes. This commit adds alternative planning approach when range metadata is available. Instead of fixing individual replicas that can't make progress it finds ranges that can't make progress from metadata using descriptors and updates their replicas to recover from loss of quorum. This commit also adds replica collection stage as a part of make-plan command itself. To invoke collection from a cluster instead of using files one needs to provide --host and other standard cluster connection related flags (--cert-dir, --insecure etc.) as appropriate. Example command output for a local cluster with 3 out of 5 nodes surrvivng looks like: ``` ~/tmp ❯❯❯ cockroach debug recover make-plan --insecure --host 127.0.0.1:26257 >recover-plan.json Nodes scanned: 3 Total replicas analyzed: 228 Ranges without quorum: 15 Discarded live replicas: 0 Proposed changes: range r4:/System/tsd updating replica (n2,s2):3 to (n2,s2):15. Discarding available replicas: [], discarding dead replicas: [(n5,s5):4,(n4,s4):2]. range r80:/Table/106/1 updating replica (n1,s1):1 to (n1,s1):14. Discarding available replicas: [], discarding dead replicas: [(n5,s5):3,(n4,s4):2]. range r87:/Table/106/1/"paris"/"\xcc\xcc\xcc\xcc\xcc\xcc@\x00\x80\x00\x00\x00\x00\x00\x00(" updating replica (n1,s1):1 to (n1,s1):14. Discarding available replicas: [], discarding dead replicas: [(n5,s5):3,(n4,s4):2]. range r88:/Table/106/1/"seattle"/"ffffffH\x00\x80\x00\x00\x00\x00\x00\x00\x14" updating replica (n3,s3):3 to (n3,s3):15. Discarding available replicas: [], discarding dead replicas: [(n5,s5):4,(n4,s4):2]. range r105:/Table/106/1/"washington dc"/"L\xcc\xcc\xcc\xcc\xccL\x00\x80\x00\x00\x00\x00\x00\x00\x0f" updating replica (n3,s3):3 to (n3,s3):14. Discarding available replicas: [], discarding dead replicas: [(n5,s5):1,(n4,s4):2]. range r98:/Table/107/1/"boston"/"333333D\x00\x80\x00\x00\x00\x00\x00\x00\x03" updating replica (n2,s2):3 to (n2,s2):15. Discarding available replicas: [], discarding dead replicas: [(n5,s5):4,(n4,s4):2]. range r95:/Table/107/1/"seattle"/"ffffffH\x00\x80\x00\x00\x00\x00\x00\x00\x06" updating replica (n3,s3):2 to (n3,s3):15. Discarding available replicas: [], discarding dead replicas: [(n4,s4):4,(n5,s5):3]. range r125:/Table/107/1/"washington dc"/"DDDDDDD\x00\x80\x00\x00\x00\x00\x00\x00\x04" updating replica (n3,s3):2 to (n3,s3):14. Discarding available replicas: [], discarding dead replicas: [(n4,s4):1,(n5,s5):3]. range r115:/Table/108/1/"boston"/"8Q\xeb\x85\x1e\xb8B\x00\x80\x00\x00\x00\x00\x00\x00n" updating replica (n2,s2):3 to (n2,s2):15. Discarding available replicas: [], discarding dead replicas: [(n5,s5):4,(n4,s4):2]. range r104:/Table/108/1/"new york"/"\x1c(\xf5\u008f\\I\x00\x80\x00\x00\x00\x00\x00\x007" updating replica (n2,s2):2 to (n2,s2):15. Discarding available replicas: [], discarding dead replicas: [(n5,s5):4,(n4,s4):3]. range r102:/Table/108/1/"seattle"/"p\xa3\xd7\n=pD\x00\x80\x00\x00\x00\x00\x00\x00\xdc" updating replica (n3,s3):2 to (n3,s3):15. Discarding available replicas: [], discarding dead replicas: [(n4,s4):4,(n5,s5):3]. range r126:/Table/108/1/"washington dc"/"Tz\xe1G\xae\x14L\x00\x80\x00\x00\x00\x00\x00\x00\xa5" updating replica (n3,s3):2 to (n3,s3):14. Discarding available replicas: [], discarding dead replicas: [(n4,s4):1,(n5,s5):3]. range r86:/Table/108/3 updating replica (n1,s1):1 to (n1,s1):14. Discarding available replicas: [], discarding dead replicas: [(n4,s4):3,(n5,s5):2]. range r59:/Table/109/1 updating replica (n2,s2):3 to (n2,s2):15. Discarding available replicas: [], discarding dead replicas: [(n5,s5):4,(n4,s4):2]. range r65:/Table/111/1 updating replica (n3,s3):3 to (n3,s3):15. Discarding available replicas: [], discarding dead replicas: [(n5,s5):4,(n4,s4):2]. Discovered dead nodes would be marked as decommissioned: n4, n5 Proceed with plan creation [y/N] y Plan created. To stage recovery application in half-online mode invoke: 'cockroach debug recover apply-plan --host=127.0.0.1:26257 --insecure=true <plan file>' Alternatively distribute plan to below nodes and invoke 'debug recover apply-plan --store=<store-dir> <plan file>' on: - node n2, store(s) s2 - node n1, store(s) s1 - node n3, store(s) s3 ``` Release note: None Fixes: #93038 Fixes: #93046 94948: changefeedccl: give notice when metrics_label set without child_metrics r=samiskin a=samiskin Resolves #75682 Surfaces a notice of ``` server.child_metrics.enabled is set to false, metrics will only be published to the '<scope>' label when it is set to true" ``` When child_metrics setting isn't enabled during changefeed creation Release note (enterprise change): Changefeeds created/altered with a metrics_label set while server.child_metrics.enabled is false will now provide the user a notice upon creation. 95009: tree: fix panic when encoding tuple r=rafiss a=rafiss fixes #95008 This adds a bounds check to avoid a panic. Release note (bug fix): Fixed a crash that could happen when formatting a tuple with an unknown type. 95294: sql: make pg_description aware of builtin function descriptions r=rafiss,msirek a=knz Epic: CRDB-23454 Fixes #95292. Needed for #88061. First commit from #95289. This also extends the completion rules to properly handle functions in multiple namespaces. Release note (bug fix): `pg_catalog.pg_description` and `pg_catalog.obj_description()` are now able to retrieve the descriptive help for built-in functions. 95356: server: remove unused migrationExecutor r=ajwerner a=ajwerner This is no longer referenced since #91627. Epic: none Release note: None Co-authored-by: Oleg Afanasyev <[email protected]> Co-authored-by: Shiranka Miskin <[email protected]> Co-authored-by: Rafi Shamim <[email protected]> Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: Andrew Werner <[email protected]>

knz · 2023-01-19T09:18:37Z

@rafiss do you want to update your review disposition on this?

rafiss

this lgtm! and i'm excited for the nice tests

rafiss · 2023-01-20T15:31:12Z

though the CI failure is interesting. is that an optimizer bug?

        testdata/describe:33:
         \l
        output didn't match expected:
        @@ -6,11 +6,52 @@
                d.datcollate AS "Collate",
                d.datctype AS "Ctype",
                COALESCE(pg_catalog.array_to_string(d.datacl, e'\n'), '') AS "Access privileges"
           FROM pg_catalog.pg_database d
         ORDER BY 1
        -Name,Owner,Encoding,Collate,Ctype,Access privileges
        -defaultdb,root,UTF8,en_US.utf8,en_US.utf8,
        -mydb,root,UTF8,en_US.utf8,en_US.utf8,
        -postgres,root,UTF8,en_US.utf8,en_US.utf8,
        -system,unknown (OID=3233629770),UTF8,en_US.utf8,en_US.utf8,
        +ERROR: internal error: lookup for ComparisonExpr ((@18)[oid] = (('defaultdb')[string])[name])[bool]'s CmpOp failed
        +SQLSTATE: XX000
        +DETAIL: stack trace:
        +github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:491: MemoizeComparisonExprOp()
        +github.com/cockroachdb/cockroach/pkg/sql/sem/tree/expr.go:400: NewTypedComparisonExpr()
        +github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:257: buildComparison()
        +github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/scalar.go:107: buildScalar()
        +github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/builder.go:325: BuildScalar()
        +github.com/cockroachdb/cockroach/pkg/sql/opt/exec/execbuilder/format.go:54: fmtInterceptor()

knz · 2023-01-20T23:30:42Z

wowzer, this is new. It wasn't there before today's rebase.

knz · 2023-01-20T23:43:17Z

filed #95633

ZhouXing19

!

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @knz, @nvanbenschoten, and @rafiss)

pkg/cli/clisqlshell/describe.go line 588 at r7 (raw file):

Previously, knz (Raphael 'kena' Poss) wrote…

Why? this is what denotes foreign key constraints.

Ah I see, sorry for the noise!

Release note (cli change): The SQL shell (`cockroach sql`, `demo`) now supports the client-side commands `\l`, `\dn`, `\d`, `\di`, `\dm`, `\ds`, `\dt`, `\dv`, `\dC`, `\dT`, `\dd`, `\dg`, `\du` and `\dd` in a way similar to `psql`, including the modifier flags `S` and `+`, for convenience for users migrating from PostgreSQL. A notable difference is that when a pattern argument is specified, it should use the SQL "LIKE" syntax (with `%` representing the wildcard character) instead of PostgreSQL's glob-like syntax (with `*` representing wildcards). Co-authored-by: Nathan VanBenschoten <[email protected]>

knz · 2023-02-24T20:59:29Z

TFYRs!

bors r=rafiss,ZhouXing19

craig · 2023-02-24T21:43:48Z

Build succeeded:

Bazel Essential CI (Cockroach)

knz requested review from nvanbenschoten and a team September 16, 2022 18:10

knz requested a review from a team as a code owner September 16, 2022 18:10

knz force-pushed the 20220916-sql branch 2 times, most recently from e546083 to ae1e65b Compare September 16, 2022 18:23

knz requested a review from rafiss September 16, 2022 18:28

nvanbenschoten reviewed Sep 16, 2022

View reviewed changes

knz force-pushed the 20220916-sql branch from ae1e65b to bd6ae67 Compare September 16, 2022 19:53

knz force-pushed the 20220916-sql branch from bd6ae67 to ebf5ce9 Compare September 16, 2022 20:31

knz force-pushed the 20220916-sql branch 2 times, most recently from 3de1cd2 to 86dd2c3 Compare September 16, 2022 21:58

knz force-pushed the 20220916-sql branch 2 times, most recently from a3293e2 to e445484 Compare September 17, 2022 20:19

This was referenced Sep 17, 2022

sql: query planning/execution correctness bug on a vtable anti join #88096

Closed

sql: pg_table_is_visible() doesn't work for indexes #88097

Closed

knz marked this pull request as draft September 17, 2022 21:02

knz mentioned this pull request Sep 17, 2022

sql: add missing obj_description case #88098

Merged

knz force-pushed the 20220916-sql branch from e445484 to 3dfd8e8 Compare September 17, 2022 21:16

knz marked this pull request as ready for review September 18, 2022 14:20

knz force-pushed the 20220916-sql branch from 3dfd8e8 to 2334cf4 Compare September 18, 2022 14:20

knz mentioned this pull request Jan 16, 2023

sql: pg_catalog.obj_description does not support pg_namespace #95322

Closed

knz force-pushed the 20220916-sql branch from 1b71538 to 180e74d Compare January 16, 2023 21:00

knz requested a review from a team as a code owner January 16, 2023 21:00

knz requested a review from yuzefovich January 16, 2023 21:00

yuzefovich removed request for a team and yuzefovich January 16, 2023 22:48

knz force-pushed the 20220916-sql branch 3 times, most recently from 01bd8a2 to 2d2e6b4 Compare January 17, 2023 14:27

knz mentioned this pull request Jan 18, 2023

sql: rationalize the handling of comments #95431

Closed

knz force-pushed the 20220916-sql branch from 2d2e6b4 to 4718c36 Compare January 18, 2023 14:29

knz mentioned this pull request Jan 20, 2023

builtins: make pg_get_indexdef handle expression indexes #95413

Merged

knz force-pushed the 20220916-sql branch from 4718c36 to 65b4923 Compare January 20, 2023 13:09

rafiss approved these changes Jan 20, 2023

View reviewed changes

knz mentioned this pull request Jan 20, 2023

sql: recent regression on planning comparisons #95633

Closed

ZhouXing19 approved these changes Feb 16, 2023

View reviewed changes

knz force-pushed the 20220916-sql branch from 65b4923 to 0c81bd1 Compare February 24, 2023 20:59

craig bot merged commit ae2e8b8 into cockroachdb:master Feb 24, 2023

cockroach-teamcity mentioned this pull request Feb 25, 2023

PR #88061 - clisqlshell: new infrastructure for describe commands cockroachdb/docs#16361

Open

ianjevans mentioned this pull request Mar 10, 2023

\dC+ command in SQL shell gives an error #98407

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

clisqlshell: new infrastructure for describe commands #88061

clisqlshell: new infrastructure for describe commands #88061

knz commented Sep 16, 2022 •

edited

Loading

cockroach-teamcity commented Sep 16, 2022

nvanbenschoten left a comment

knz commented Sep 16, 2022

knz commented Sep 16, 2022

knz commented Sep 16, 2022

knz commented Sep 17, 2022

knz commented Sep 18, 2022

knz commented Jan 19, 2023

rafiss left a comment

rafiss commented Jan 20, 2023

knz commented Jan 20, 2023

knz commented Jan 20, 2023

ZhouXing19 left a comment

knz commented Feb 24, 2023

craig bot commented Feb 24, 2023

clisqlshell: new infrastructure for describe commands #88061

clisqlshell: new infrastructure for describe commands #88061

Conversation

knz commented Sep 16, 2022 • edited Loading

cockroach-teamcity commented Sep 16, 2022

nvanbenschoten left a comment

Choose a reason for hiding this comment

knz commented Sep 16, 2022

knz commented Sep 16, 2022

knz commented Sep 16, 2022

knz commented Sep 17, 2022

knz commented Sep 18, 2022

knz commented Jan 19, 2023

rafiss left a comment

Choose a reason for hiding this comment

rafiss commented Jan 20, 2023

knz commented Jan 20, 2023

knz commented Jan 20, 2023

ZhouXing19 left a comment

Choose a reason for hiding this comment

knz commented Feb 24, 2023

craig bot commented Feb 24, 2023

knz commented Sep 16, 2022 •

edited

Loading