feat(frontend): support show create [materialized] view command #6921

yezizp2012 · 2022-12-15T13:25:36Z

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

Support show create create [materialized] view commands.

Checklist

I have written necessary rustdoc comments
I have added necessary unit tests and integration tests
All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

If your pull request contains user-facing changes, please specify the types of the changes, and create a release note. Otherwise, please feel free to remove this section.

Types of user-facing changes

SQL commands, functions, and operators
Support show create create [materialized] view commands:

dev=> create table t2(v1 int);
CREATE_TABLE
dev=> create materialized view mv1 as select * from t2;
CREATE_MATERIALIZED_VIEW
dev=> create view v1 as select * from t2;
CREATE_VIEW
dev=> show create materialized view public.mv1;
    Name    |                    Create Sql
------------+--------------------------------------------------
 public.mv1 | CREATE MATERIALIZED VIEW mv1 AS SELECT * FROM t2
(1 row)

dev=> show create view v1;
   Name    |             Create Sql
-----------+------------------------------------
 public.v1 | CREATE VIEW v1 AS SELECT * FROM t2
(1 row)

Release note

Support show create create [materialized] view commands.

Refer to a related PR or issue link (optional)

Resolve #6913

hzxa21

LGTM.

Should we also support showing the view definition in a pg compatible way (via pg_get_viewdef or pg_views)?

yezizp2012 · 2022-12-15T14:15:30Z

LGTM.

Should we also support showing the view definition in a pg compatible way (via pg_get_viewdef or pg_views)?

Yes, we already support definition column in pg_views. 😄

fuyufjh

LGTM! Thank you a lot! ❤️

BugenZhao · 2022-12-16T05:54:54Z

src/frontend/src/handler/show.rs

+            // We only stored original sql in catalog, uses parser to parse and format.
+            let stmt = Parser::parse_sql(&view.sql)
+                .map_err(|err| anyhow!("Failed to parse view create sql: {}, {}", view.sql, err))?;
+            assert!(stmt.len() == 1);
+            stmt[0].to_string()


The SQL unparsed from the AST might be incorrect. :( #6801 Will this be a problem, if this is for some programs to read?

If so, I guess we need to carefully review the fmt::Display for all AST nodes and add a comprehensive test suite for it.

🥵 I think this is just for user to check how the mview and view are created.

If so, I guess we need to carefully review the fmt::Display for all AST nodes and add a comprehensive test suite for it.

But this's also needed, does the tests in sql_parser meet demand? It seems that we only have few tests there.

I'm not sure. cc @lmatz

We can test roundtrip parsing (parse(unparse(parse(sql))) == parse(sql) ) in sqlsmith. Created an issue #6935

We can test roundtrip parsing (parse(unparse(parse(sql))) == parse(sql) ) in sqlsmith.

I'm afraid there's a case that some different SQLs are parsed to the same AST, which requires some binder context to distinguish. So when we unparse it, we cannot find the original representation, and this roundtrip test cannot cover this. 🤔

I guess that's exactly why using AST instead of original sql to compare in xxchan's proposal?

codecov · 2022-12-16T06:11:19Z

Codecov Report

Merging #6921 (8ddc05d) into main (98a1853) will decrease coverage by 0.03%.
The diff coverage is 20.40%.

@@            Coverage Diff             @@
##             main    #6921      +/-   ##
==========================================
- Coverage   72.71%   72.68%   -0.04%     
==========================================
  Files        1036     1036              
  Lines      166192   166289      +97     
==========================================
+ Hits       120840   120859      +19     
- Misses      45352    45430      +78

Flag	Coverage Δ
rust	`72.68% <20.40%> (-0.04%)`	⬇️

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

Impacted Files	Coverage Δ
src/frontend/src/handler/mod.rs	`63.93% <0.00%> (-0.71%)`	⬇️
src/frontend/src/handler/show.rs	`63.59% <0.00%> (-23.20%)`	⬇️
src/sqlparser/src/parser.rs	`92.45% <44.00%> (-0.50%)`	⬇️
src/sqlparser/src/ast/mod.rs	`86.67% <69.23%> (-0.22%)`	⬇️

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

yezizp2012 added 2 commits December 15, 2022 19:37

feat: support show create on mview and mview

7e16006

fmt and add some test

0d5e5c4

github-actions bot added the type/feature label Dec 15, 2022

yezizp2012 added the user-facing-changes Contains changes that are visible to users label Dec 15, 2022

fmt

45a6c28

hzxa21 approved these changes Dec 15, 2022

View reviewed changes

Merge branch 'main' into feat/show-create

0e3ff69

yezizp2012 requested review from BugenZhao, xiangjinwu and fuyufjh December 16, 2022 05:49

fuyufjh approved these changes Dec 16, 2022

View reviewed changes

BugenZhao reviewed Dec 16, 2022

View reviewed changes

lmatz requested a review from xxchan December 16, 2022 07:07

xxchan mentioned this pull request Dec 16, 2022

sqlparser: test & fix unparsing AST to SQL #6935

Closed

yezizp2012 added the mergify/can-merge label Dec 16, 2022

Merge branch 'main' into feat/show-create

8ddc05d

BugenZhao approved these changes Dec 16, 2022

View reviewed changes

mergify bot merged commit 650810a into main Dec 16, 2022

mergify bot deleted the feat/show-create branch December 16, 2022 11:13

yezizp2012 mentioned this pull request Dec 16, 2022

fix(frontend): fix parse and format for ExprQualifiedWildcard and FieldIdentifier from AST #6940

Merged

3 tasks

BugenZhao mentioned this pull request Dec 30, 2022

feat(frontend): show create table #7152

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(frontend): support show create [materialized] view command #6921

feat(frontend): support show create [materialized] view command #6921

yezizp2012 commented Dec 15, 2022 •

edited

Loading

hzxa21 left a comment

yezizp2012 commented Dec 15, 2022

fuyufjh left a comment

BugenZhao Dec 16, 2022

BugenZhao Dec 16, 2022

yezizp2012 Dec 16, 2022 •

edited

Loading

BugenZhao Dec 16, 2022

xxchan Dec 16, 2022 •

edited

Loading

BugenZhao Dec 16, 2022

BugenZhao Dec 16, 2022 •

edited

Loading

lmatz Dec 16, 2022

codecov bot commented Dec 16, 2022 •

edited

Loading

feat(frontend): support show create [materialized] view command #6921

feat(frontend): support show create [materialized] view command #6921

Conversation

yezizp2012 commented Dec 15, 2022 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

Types of user-facing changes

Release note

Refer to a related PR or issue link (optional)

hzxa21 left a comment

Choose a reason for hiding this comment

yezizp2012 commented Dec 15, 2022

fuyufjh left a comment

Choose a reason for hiding this comment

BugenZhao Dec 16, 2022

Choose a reason for hiding this comment

BugenZhao Dec 16, 2022

Choose a reason for hiding this comment

yezizp2012 Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

BugenZhao Dec 16, 2022

Choose a reason for hiding this comment

xxchan Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

BugenZhao Dec 16, 2022

Choose a reason for hiding this comment

BugenZhao Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

lmatz Dec 16, 2022

Choose a reason for hiding this comment

codecov bot commented Dec 16, 2022 • edited Loading

Codecov Report

yezizp2012 commented Dec 15, 2022 •

edited

Loading

yezizp2012 Dec 16, 2022 •

edited

Loading

xxchan Dec 16, 2022 •

edited

Loading

BugenZhao Dec 16, 2022 •

edited

Loading

codecov bot commented Dec 16, 2022 •

edited

Loading