Skip to content

sql: make SHOW and SET more consistent.#13678

Merged
knz merged 1 commit intocockroachdb:masterfrom
knz:fix-show-user
Apr 28, 2017
Merged

sql: make SHOW and SET more consistent.#13678
knz merged 1 commit intocockroachdb:masterfrom
knz:fix-show-user

Conversation

@knz
Copy link
Copy Markdown
Contributor

@knz knz commented Feb 20, 2017 •
edited
Loading

This patch is intended to smoothe user experience:

  • SHOW TRANSACTION STATUS can display the current transaction state;
    make SHOW ALL report this as well.
  • SHOW ALL can displayy the value of session_user. Ensure that SHOW SESSION_USER works too.

Also this patch annotates the grammar to avoid separate syntax diagrams
for the various syntactic variants of the (single) SHOW and SET
statements.

Needed for #12408.

Corresponding doc changes covered in cockroachdb/docs#1107.

This change is Reviewable

@knz knz mentioned this pull request Feb 20, 2017
Merged
@knz knz mentioned this pull request Feb 20, 2017
Closed
@madelynnblue
Copy link
Copy Markdown
Contributor

madelynnblue commented Feb 21, 2017

:lgtm: assuming the docs folks are ok with the changes in sql.y

Review status: 0 of 8 files reviewed at latest revision, all discussions resolved, all commit checks successful.

Comments from Reviewable

@knz knz requested a review from jseldess February 21, 2017 06:21
@jseldess
Copy link
Copy Markdown
Contributor

jseldess commented Feb 21, 2017

Looks good at a glance, @knz , but I need to find a stretch of time without kids (on vacation). Will review more closely as soon as I can.

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Feb 21, 2017 via email

@jseldess
Copy link
Copy Markdown
Contributor

jseldess commented Feb 21, 2017

Actually, @sploiselle, can you review once you're back in the office?

@jseldess jseldess requested a review from sploiselle February 21, 2017 15:02
@knz
sql: make SHOW and SET more consistent.
5fff14d 
This patch is intended to smoothe user experience:

- `SHOW TRANSACTION STATUS` can display the current transaction state;
  make `SHOW ALL` report this as well.
- `SHOW ALL` can display the value of `session_user`. Ensure that `SHOW
  SESSION_USER` works too.

Also this patch annotates the grammar to avoid separate syntax
diagrams for the various syntactic variants of the (single) SHOW and
SET statements.
@knz
Copy link
Copy Markdown
Contributor Author

knz commented Apr 27, 2017

Rebased to current master.
@sploiselle this PR goes together with cockroachdb/docs#1107. I'll wait on your comments there before I merge this one.

@knz
Copy link
Copy Markdown
Contributor Author

knz commented Apr 28, 2017

I'm going to merge this so that we get the fix for show transaction status and show session_user in for the RC. We can carry the docs conversation asynchronously by simply not re-generating the sql diagrams until that discussion is resolved.

@knz knz merged commit 1935e19 into cockroachdb:master Apr 28, 2017
@knz knz deleted the fix-show-user branch April 28, 2017 19:31
@sploiselle
Copy link
Copy Markdown
Contributor

sploiselle commented Apr 29, 2017

Thanks @knz. Sorry for the delay in reviewing this.

@jseldess jseldess mentioned this pull request May 1, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jseldess jseldess Awaiting requested review from jseldess

@sploiselle sploiselle Awaiting requested review from sploiselle

Assignees

@madelynnblue madelynnblue

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@knz @madelynnblue @jseldess @sploiselle