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: make the table reference syntax carry its alias, if any #17031

Merged
merged 1 commit into from
Jul 18, 2017

Conversation

knz
Copy link
Contributor

@knz knz commented Jul 14, 2017

This patch extends the numeric table reference syntax (e.g. [123])
with the ability to give the source and its columns an alias.

The following new syntax forms are allowed:

  • [123(1,2,3) as t] the same as [123(1,2,3)] as t.
  • [123 as t(x,y,z)] the same as [123] as t(x,y,z).
  • [123(1,2,3) as t(x,y,z)] the same as [123(1,2,3]) as t(x,y,z).

Meanwhile things like [123 as t] as u are still possible.

Reminder: the previously supported syntax [123(1,2,3)]is a data
source providing only columns 1,2,3 from table 123. Its column names
are those listed in the table descriptor.

The new syntax forms are introduced in preparation of a subsequent
patch to enhance the handling of view descriptors.

Also,the new syntax captures the alias definition (AS clause) inside
the square brackets, as we plan to replace table names by this square
bracket syntax, and the replacement must syntactically fit in a way
that does not cause grammar error. In a table expression of the form
foo as t, a naive replacement using [123] as foo as t would be
syntactically invalid, whereas [123 as foo] as t is valid.

I am extracting this functionality from the more complex #15388, because I would like to rebase #16884 on top of it.

@knz knz requested a review from RaduBerinde July 14, 2017 22:10
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@knz knz force-pushed the 20170714-num-table-refs branch 2 times, most recently from cfccf33 to f3e7fd9 Compare July 14, 2017 22:22
@RaduBerinde
Copy link
Member

Can you include some motivation in the commit message? Why is this better? [123] AS t seems closer to the regular syntax.

Are there any upgrade concerns for existing views?


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


Comments from Reviewable

@knz knz force-pushed the 20170714-num-table-refs branch from f3e7fd9 to 4e063d6 Compare July 15, 2017 15:40
@knz
Copy link
Contributor Author

knz commented Jul 15, 2017

I am sorry the commit message was so terse. I have extended it. There is no concern for existing descriptors / queries, the syntax is backward compatible.

@knz knz force-pushed the 20170714-num-table-refs branch 2 times, most recently from c7c99e1 to 5cf9f08 Compare July 15, 2017 16:16
@RaduBerinde
Copy link
Member

Thanks, LGTM!

@knz knz force-pushed the 20170714-num-table-refs branch from 5cf9f08 to 90a16cb Compare July 18, 2017 15:29
This patch extends the numeric table reference syntax (e.g. `[123]`)
with the ability to give the source and its columns an alias.

The following new syntax forms are allowed:

- `[123(1,2,3) as t]` the same as `[123(1,2,3)] as t`.
- `[123 as t(x,y,z)]` the same as `[123] as t(x,y,z)`.
- `[123(1,2,3) as t(x,y,z)]` the same as `[123(1,2,3]) as t(x,y,z)`.

Meanwhile things like `[123 as t] as u` are still possible.

Reminder: the previously supported syntax `[123(1,2,3)]`is a data
source providing only columns 1,2,3 from table 123. Its column names
are those listed in the table descriptor.

The new syntax forms are introduced in preparation of a subsequent
patch to enhance the handling of view descriptors.

Also,the new syntax captures the alias definition (AS clause) inside
the square brackets, as we plan to replace table names by this square
bracket syntax, and the replacement must syntactically fit in a way
that does not cause grammar error. In a table expression of the form
`foo as t`, a naive replacement using `[123] as foo as t` would be
syntactically invalid, whereas `[123 as foo] as t` is valid.
@knz knz force-pushed the 20170714-num-table-refs branch from 90a16cb to 6585df2 Compare July 18, 2017 17:51
@knz
Copy link
Contributor Author

knz commented Jul 18, 2017

TFYR!

@knz knz merged commit 1fce55c into cockroachdb:master Jul 18, 2017
@knz knz deleted the 20170714-num-table-refs branch July 18, 2017 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants