feat: EXCEPT in SELECT #10438

wugouzi · 2023-06-20T09:02:06Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

Add support to following syntax:

SELECT * EXCEPT (column1, column2, ...)
...

close #9092.

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
All checks passed in ./risedev check (or alias, ./risedev c)
My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)

Documentation

My PR contains user-facing changes.

Click here for Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove the others.

Installation and deployment
Connector (sources & sinks)
SQL commands, functions, and operators
RisingWave cluster configuration changes
Other (please specify in the release note below)

Release note

lmatz · 2023-06-20T09:12:48Z

Can we add more tests, e.g.:

except in an inner subquery
except in the outer query when there is an inner subquery
a combination of 1 + 2

st1page

we can not bind it in the query level, we should treat it as a modifier on the * or WildCard.

dev=# select * from t;
 a | b | c 
---+---+---
 2 | 1 | 3
(1 row)

dev=# select *, * from t;
 a | b | c | a | b | c 
---+---+---+---+---+---
 2 | 1 | 3 | 2 | 1 | 3
(1 row)

we need to support multiple value in the except. SELECT * EXCEPT (i, j, k);

In conclusion, we might need to add SelectItem::ExceptWithExcept(Vec<Expr>) even use it to replace the original WildCard

xxchan · 2023-06-20T09:17:39Z

src/sqlparser/tests/testdata/select.yaml

@@ -37,6 +37,8 @@
  formatted_sql: SELECT id, fname, lname FROM customer WHERE salary <> 'Not Provided' AND salary <> ''
 - input: SELECT id FROM customer WHERE NOT salary = ''
  formatted_sql: SELECT id FROM customer WHERE NOT salary = ''
+- input: SELECT * EXCEPT v1 FROM foo
+  formatted_sql: SELECT EXCEPT v1 FROM foo


Is this valid?

This is valid in current implementation and there are many things to be fixed and there will be possible change later.

xxchan · 2023-06-20T09:23:23Z

except in the outer query when there is an inner subquery

e.g., the TopN pattern

BTW, is the syntax decided?

Whether to use EXCEPT (clickhouse, spanner) or EXCLUDE (snowflakes, duckdb)
Whether () is required (all 4 DBs above seem to require that).

Besides, multiple columns are not tested and things like select a,b except are not tested.

kwannoel · 2023-06-20T09:25:15Z

Hi could you add some sqlsmith fuzzing tests? Feel free to ask me if any questions.

You may check out the developer docs here: https://github.com/risingwavelabs/risingwave/blob/main/src/tests/sqlsmith/develop.md

st1page · 2023-06-20T09:31:16Z

except in the outer query when there is an inner subquery

e.g., the TopN pattern

BTW, is the syntax decided?

Whether to use EXCEPT (clickhouse, spanner) or EXCLUDE (snowflakes, duckdb)

Whether () is required (all 4 DBs above seem to require that).

Besides, multiple columns are not tested and things like select a,b except are not tested.

EXCEPT
() is required

https://duckdb.org/docs/sql/query_syntax/select.html

codecov · 2023-06-20T09:35:18Z

Codecov Report

Merging #10438 (0bfaa3b) into main (f885259) will decrease coverage by 0.01%.
The diff coverage is 61.53%.

@@            Coverage Diff             @@
##             main   #10438      +/-   ##
==========================================
- Coverage   70.38%   70.37%   -0.01%     
==========================================
  Files        1269     1269              
  Lines      217167   217216      +49     
==========================================
+ Hits       152843   152868      +25     
- Misses      64324    64348      +24

Flag	Coverage Δ
rust	`70.37% <61.53%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/meta/src/manager/catalog/utils.rs	`49.47% <0.00%> (-1.96%)`	⬇️
src/sqlparser/src/ast/mod.rs	`85.02% <16.66%> (-0.67%)`	⬇️
src/frontend/src/binder/expr/function.rs	`86.97% <50.00%> (-0.11%)`	⬇️
src/sqlparser/src/parser.rs	`86.68% <76.92%> (-0.07%)`	⬇️
src/frontend/src/binder/select.rs	`91.85% <93.33%> (-0.01%)`	⬇️
src/frontend/src/handler/create_sink.rs	`94.93% <100.00%> (ø)`
src/sqlparser/src/ast/query.rs	`87.58% <100.00%> (+0.50%)`	⬆️

... and 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

xxchan · 2023-06-20T09:35:35Z

You shared an EXCLUDE doc 😄

st1page · 2023-06-20T09:46:13Z

You shared an EXCLUDE doc smile

yes and just to say that all the grammer need ()

kwannoel · 2023-06-21T04:23:40Z

src/frontend/planner_test/tests/testdata/output/basic_query.yaml

@@ -42,6 +42,15 @@
    StreamMaterialize { columns: [v1, t._row_id(hidden)], stream_key: [t._row_id], pk_columns: [t._row_id], pk_conflict: "NoCheck" }
    └─StreamFilter { predicate: (t.v1 < 1:Int32) }
      └─StreamTableScan { table: t, columns: [t.v1, t._row_id], pk: [t._row_id], dist: UpstreamHashShard(t._row_id) }
+- sql: |
+    create table t (v1 int, v2 int, v3 int);
+    select * except (v1, v2) from t;


Can we add more tests which explicitly select except columns?
Something like:

select v3 from (select * except (v1, v2) from t

Can we also add tests which have selected the excepted row, and re-select already included rows?

select * except (v1, v2), v1 from t; -- v1 excluded from except. select * except (v1, v2), v3 from t; -- v3 included from except.

src/sqlparser/src/ast/mod.rs

st1page · 2023-06-25T04:33:36Z

src/frontend/src/binder/select.rs

+                            if self.context.range_of.is_empty() {
+                                return Err(ErrorCode::BindError(
+                                    "SELECT * with no tables specified is not valid".into(),
+                                )
+                                .into());
+                            }


extract it out of the match?

st1page · 2023-06-25T04:34:42Z

src/frontend/src/binder/select.rs

+                            let (exprs, names) = self.iter_column_groups();
+                            select_list.extend(exprs);
+                            aliases.extend(names);
+
+                            // Bind columns that are not in groups
+                            let (exprs, names) = Self::iter_bound_columns(
+                                self.context.columns[..].iter().filter(|c| {
+                                    !c.is_hidden
+                                        && !self
+                                            .context
+                                            .column_group_context
+                                            .mapping
+                                            .contains_key(&c.index)
+                                }),
+                            );


I think some common logic about the column_groups is lost

st1page

LGTM

xxchan

There aren't any tests for syntax errors 😟

wugouzi · 2023-06-26T02:07:42Z

src/sqlparser/tests/testdata/select.yaml

+  formatted_sql: SELECT * EXCEPT (v1, v2) FROM foo
+  formatted_ast: 'Query(Query { with: None, body: Select(Select { distinct: All, projection: [WildcardOrWithExcept(Some([Identifier(Ident { value: "v1", quote_style: None }), Identifier(Ident { value: "v2", quote_style: None })]))], from: [TableWithJoins { relation: Table { name: ObjectName([Ident { value: "foo", quote_style: None }]), alias: None, for_system_time_as_of_proctime: false }, joins: [] }], lateral_views: [], selection: None, group_by: [], having: None }), order_by: [], limit: None, offset: None, fetch: None })'
+- input: SELECT v3 EXCEPT (v1, v2) FROM foo
+  error_msg: |-


I think this is a test for syntax. But it is not sufficient indeed. I will add some.

wugouzi added 2 commits June 20, 2023 08:41

feat: except in select

633fbcf

fix: format

959cbbe

github-actions bot added type/feature user-facing-changes Contains changes that are visible to users labels Jun 20, 2023

wugouzi added 2 commits June 20, 2023 09:06

clean code

e16d529

add comments

1777a88

wugouzi marked this pull request as ready for review June 20, 2023 09:08

wugouzi requested a review from st1page June 20, 2023 09:08

st1page reviewed Jun 20, 2023

View reviewed changes

xxchan reviewed Jun 20, 2023

View reviewed changes

wugouzi added 5 commits June 21, 2023 02:53

fix: fix grammar

82f23c2

add test

c674d5b

fix format

2d42799

fix test

3c3115b

fix format

81159cd

kwannoel reviewed Jun 21, 2023

View reviewed changes

st1page reviewed Jun 21, 2023

View reviewed changes

src/sqlparser/src/ast/mod.rs Outdated Show resolved Hide resolved

combine except and wildcard

40e7f35

lmatz added the experimental label Jun 22, 2023

wugouzi added 4 commits June 25, 2023 02:25

update

e82639e

Merge branch 'main' into qiao/select-exclude

da60675

more test

6284499

clean code

6700bdb

clean code

2bca994

wugouzi mentioned this pull request Jun 25, 2023

Support SELECT * EXCEPT in sqlsmith #10516

Open

unit test

be0e24d

st1page reviewed Jun 25, 2023

View reviewed changes

reuse common logic of wildcard

66da86d

st1page approved these changes Jun 25, 2023

View reviewed changes

wugouzi added this pull request to the merge queue Jun 25, 2023

wugouzi removed this pull request from the merge queue due to a manual request Jun 25, 2023

github-merge-queue bot pushed a commit that referenced this pull request Jun 25, 2023

feat: EXCEPT in SELECT (#10438)

4fa9783

wugouzi added this pull request to the merge queue Jun 25, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 25, 2023

wugouzi added 3 commits June 25, 2023 06:57

Merge branch 'main' into qiao/select-exclude

b796a25

XMerge branch 'main' into qiao/select-exclude

4c4f69a

fix new format

0bfaa3b

wugouzi enabled auto-merge June 25, 2023 07:27

wugouzi disabled auto-merge June 25, 2023 07:29

wugouzi enabled auto-merge June 25, 2023 07:49

wugouzi added this pull request to the merge queue Jun 25, 2023

Merged via the queue into main with commit 12c145c Jun 25, 2023

wugouzi deleted the qiao/select-exclude branch June 25, 2023 08:14

xxchan reviewed Jun 25, 2023

View reviewed changes

wugouzi commented Jun 26, 2023

View reviewed changes

wugouzi mentioned this pull request Jun 26, 2023

test: more test for select * except #10542

Merged

2 tasks

emile-00 added the 📖✓ Covered or will be covered in the user docs. label Jul 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: EXCEPT in SELECT #10438

feat: EXCEPT in SELECT #10438

wugouzi commented Jun 20, 2023 •

edited

Loading

lmatz commented Jun 20, 2023

st1page left a comment •

edited

Loading

xxchan Jun 20, 2023

wugouzi Jun 20, 2023 •

edited

Loading

xxchan commented Jun 20, 2023 •

edited

Loading

kwannoel commented Jun 20, 2023

st1page commented Jun 20, 2023 •

edited

Loading

codecov bot commented Jun 20, 2023 •

edited

Loading

xxchan commented Jun 20, 2023

st1page commented Jun 20, 2023

kwannoel Jun 21, 2023 •

edited

Loading

st1page Jun 25, 2023

st1page Jun 25, 2023

st1page left a comment

xxchan left a comment

wugouzi Jun 26, 2023

feat: EXCEPT in SELECT #10438

feat: EXCEPT in SELECT #10438

Conversation

wugouzi commented Jun 20, 2023 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Types of user-facing changes

Release note

lmatz commented Jun 20, 2023

st1page left a comment • edited Loading

Choose a reason for hiding this comment

xxchan Jun 20, 2023

Choose a reason for hiding this comment

wugouzi Jun 20, 2023 • edited Loading

Choose a reason for hiding this comment

xxchan commented Jun 20, 2023 • edited Loading

kwannoel commented Jun 20, 2023

st1page commented Jun 20, 2023 • edited Loading

codecov bot commented Jun 20, 2023 • edited Loading

Codecov Report

xxchan commented Jun 20, 2023

st1page commented Jun 20, 2023

kwannoel Jun 21, 2023 • edited Loading

Choose a reason for hiding this comment

st1page Jun 25, 2023

Choose a reason for hiding this comment

st1page Jun 25, 2023

Choose a reason for hiding this comment

st1page left a comment

Choose a reason for hiding this comment

xxchan left a comment

Choose a reason for hiding this comment

wugouzi Jun 26, 2023

Choose a reason for hiding this comment

wugouzi commented Jun 20, 2023 •

edited

Loading

st1page left a comment •

edited

Loading

wugouzi Jun 20, 2023 •

edited

Loading

xxchan commented Jun 20, 2023 •

edited

Loading

st1page commented Jun 20, 2023 •

edited

Loading

codecov bot commented Jun 20, 2023 •

edited

Loading

kwannoel Jun 21, 2023 •

edited

Loading