Skip to content

First attempt of supporting CTE#14771

Merged
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
amosbird:withsub
Sep 19, 2020
Merged

First attempt of supporting CTE#14771
alexey-milovidov merged 1 commit intoClickHouse:masterfrom
amosbird:withsub

Conversation

@amosbird
Copy link
Copy Markdown
Collaborator

@amosbird amosbird commented Sep 12, 2020

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category (leave one):

  • New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Now we support WITH <identifier> AS (subquery) ... to introduce named subqueries in the query context. This closes #2416. This closes #4967.

Detailed description / Documentation draft:

Non-recursive CTE introduces named subqueries to the current and children query context. Those identifiers can appear in places where table objects are allowed. Recursion is prevented by hiding the current level CTEs from the WITH expression.

@alexey-milovidov
Copy link
Copy Markdown
Member

For the reference (that both x AS subquery and subquery AS x are Ok):

Amos Bird, [07.09.20 05:11]
BTW, what do you think of having CTE in clickhouse?

Currently the WITH statement can only introduce scalars and the grammar is of reversed order of SQL's CTE. Perhaps we can provide support like

WITH <name> AS (subquery)
while still keep the old "CSE" syntax.

Alexey Milovidov, [07.09.20 05:16]
Yes, we need to support it for FROM, IN, etc. And it should be easy to do.

Amos Bird, [07.09.20 05:17]
hmm, is the syntax ok? I was under the impression that it might introduce confusions to users. But now I feel that there might not be better ways...

Alexey Milovidov, [07.09.20 05:18]
As it's compatible with CTE, it's Ok.

We can also support

WITH (subquery) AS name 

just for consistency with existing syntax.
Because we can disambiguate these variants.

@amosbird
Copy link
Copy Markdown
Collaborator Author

For the reference cond.

Amos Bird, [13.09.20 14:24]
> For the reference (that both x AS subquery and subquery AS x are Ok):

Hmm, do you want both variants to be treated exactly the same? Currently I left the old one's behavior intact..

Alexey Milovidov, [13.09.20 14:25]
It is treat as scalar subquery?

Amos Bird, [13.09.20 14:25]
yes

Alexey Milovidov, [13.09.20 14:26]
But there's no point to have scalar subquery in FROM
and almost no point to have it in IN.

Alexey Milovidov, [13.09.20 14:26]
Is there any special cases if both variants will work the same?

Amos Bird, [13.09.20 14:27]
[In reply to Alexey Milovidov]
oh, no.  I mean with (scalar subquery) as name   and with name as (general subquery)

Amos Bird, [13.09.20 14:28]
hmm, I think I will extend it a bit, to have both variants define general subqueries

Amos Bird, [13.09.20 14:28]
not sure if there would be consequences

Alexey Milovidov, [13.09.20 14:29]
Maybe both variants can work exactly as if subquery is substituted - no matter it will be scalar or general subquery?

Example:

WITH (SELECT 1) AS x SELECT x
WITH (SELECT 1) AS x SELECT * FROM x
WITH x AS (SELECT 1) SELECT x
WITH x AS (SELECT 1) SELECT * FROM x

Amos Bird, [13.09.20 14:29]
Yes, it's possible. I need to reason it a bit. There might be tricky corner cases

Alexey Milovidov, [13.09.20 14:30]
Ok.

Amos Bird, [13.09.20 14:33]
It means x is always a table (not a column)

Amos Bird, [13.09.20 14:33]
it will break something I guess

Alexey Milovidov, [13.09.20 14:34]
[In reply to Amos Bird]
In what query?

Amos Bird, [13.09.20 14:37]
http://sqlfiddle.com/#!17/e98d3/1/0

Amos Bird, [13.09.20 14:37]
The visibility rule is different

Amos Bird, [13.09.20 14:39]
It appears to me that separating CTE and CSE might be a good design

Alexey Milovidov, [13.09.20 14:46]
In this example there is ambiguity. We can stick to some behaviour in case of ambiguous names - and it's ok even if it does not correspond to Postgres.

It's also Ok to separate CTE. Let's keep your variant. You can also copy-paste all this discussion "for the reference".

Amos Bird, [13.09.20 14:48]
[In reply to Alexey Milovidov]
Ok. I like the idea of separating CTE because in ClickHouse, subqueries have two basic forms, and scalar variant is exactly the same as other expressions used in the WITH statement. It's more consistent. So if we write WITH (subquery) as a ..., a can be used anywhere that other exprs are used, and vice versa

Alexey Milovidov, [13.09.20 14:49]
Good, I like the reasoning :)

@hczhcz
Copy link
Copy Markdown
Contributor

hczhcz commented Sep 14, 2020

It seems slightly confusing to me.

with
    x1 as (select 1) -- syntax #1
select
    ...
from
    x1,
    (select 2) as x2, -- syntax #2
    x3 as (select 3) -- error?

I would suggest something like this:

with query
    (select 1) as x
with
    (select 1) as y
select
    y
from
    x

@amosbird
Copy link
Copy Markdown
Collaborator Author

amosbird commented Sep 14, 2020

I would suggest something like this:

That's more complicated and not similar to any standard.

x3 as (select 3) -- error?

It's consistent with other databases.

@alexey-milovidov alexey-milovidov self-assigned this Sep 19, 2020
with_element->subquery = subquery;
node = with_element;
}
else
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is inconsistent that not everything in WITH is parsed as ASTWithElement.


namespace DB
{
// TODO After we support `union_with_global`, this visitor should also be extended to match ASTSelectQueryWithUnion.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW it will be something like with_global because it will apply for all subqueries with correct scoping:

WITH 1 AS x SELECT x;
WITH 1 AS x SELECT * FROM (SELECT x);
WITH 1 AS x SELECT *, x FROM (WITH 2 AS x SELECT x AS y);

@alexey-milovidov alexey-milovidov merged commit acbdc51 into ClickHouse:master Sep 19, 2020
@den-crane den-crane mentioned this pull request Sep 20, 2020
@akuzm
Copy link
Copy Markdown
Contributor

akuzm commented Sep 22, 2020

The clone() method of ASTWithElement is broken -- the cloned node references some source nodes. Apparently the reason is that the IAST::children is not cleared after copy construction, so it references both the old and the new nodes. As far as I understand, the general model for ASTs should be that children owns the child nodes, and other fields such as subquery are references into children, so they better be non-owning pointers (e.g. raw).

#15129

@durden-deeplay
Copy link
Copy Markdown

Describe, please, how this PR closes #2416?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-feature Pull request with new product feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support for non-scalar subqueries in WITH. Support for non-scalar values in WITH clause

6 participants