Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: Add information_schema.views #10288

Merged
merged 1 commit into from
Nov 2, 2016

Conversation

szpony
Copy link
Contributor

@szpony szpony commented Oct 28, 2016

Add information_schema.views.
Pls refer to #8675


This change is Reviewable

@CLAassistant
Copy link

CLAassistant commented Oct 28, 2016

CLA assistant check
All committers have signed the CLA.

@a-robinson
Copy link
Contributor

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


pkg/sql/information_schema.go, line 434 at r1 (raw file):

                  parser.NewDString(db.Name),         // table_schema
                  parser.NewDString(table.Name),      // table_name
                  parser.NewDString(table.ViewQuery), // view_definition

You may want to include a disclaimer/TODO about the view query printed here (as I've done in #10276) about how this won't properly insert column aliases into the query


Comments from Reviewable

@szpony szpony force-pushed the information_schema branch 2 times, most recently from 1f47e5b to 2cc126b Compare October 29, 2016 11:34
@szpony
Copy link
Contributor Author

szpony commented Oct 29, 2016

Review status: 0 of 2 files reviewed at latest revision, 1 unresolved discussion, all commit checks successful.


pkg/sql/information_schema.go, line 434 at r1 (raw file):

Previously, a-robinson (Alex Robinson) wrote…

You may want to include a disclaimer/TODO about the view query printed here (as I've done in #10276) about how this won't properly insert column aliases into the query

done. Thanks

Comments from Reviewable

@nvanbenschoten
Copy link
Member

:lgtm: other than the small nits. Thanks @mtMabo!


Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


pkg/sql/information_schema.go, line 434 at r1 (raw file):

Previously, mtMabo (MaBo) wrote…

done. Thanks

`s/TODO/TODO(a-robinson)/` so that this doesnt get lost.

pkg/sql/testdata/information_schema, line 675 at r2 (raw file):


query TTTTTBBBBB colnames
SELECT * FROM information_schema.views WHERE TABLE_NAME='v_xyz'; 

Let's split this into two queries so it's a little more readable.


Comments from Reviewable

@szpony
Copy link
Contributor Author

szpony commented Oct 31, 2016

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


pkg/sql/testdata/information_schema, line 675 at r2 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Let's split this into two queries so it's a little more readable.

What do you mean "split this into two queries so it's a little more readable" ?

Comments from Reviewable

@nvanbenschoten
Copy link
Member

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


pkg/sql/testdata/information_schema, line 675 at r2 (raw file):

Previously, mtMabo (MaBo) wrote…

What do you mean "split this into two queries so it's a little more readable" ?

Instead of `SELECT *`, select from 5 columns each in two queries.

Comments from Reviewable

@szpony szpony force-pushed the information_schema branch from 2cc126b to 541d004 Compare November 1, 2016 04:07
@szpony
Copy link
Contributor Author

szpony commented Nov 1, 2016

Review status: 0 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


pkg/sql/information_schema.go, line 434 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

s/TODO/TODO(a-robinson)/ so that this doesnt get lost.

Done.

pkg/sql/testdata/information_schema, line 675 at r2 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

Instead of SELECT *, select from 5 columns each in two queries.

Done. Thanks

Comments from Reviewable

@szpony szpony force-pushed the information_schema branch from fec876d to 2330221 Compare November 2, 2016 02:06
@nvanbenschoten nvanbenschoten merged commit 7915024 into cockroachdb:master Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants