Skip to content

feat(fe,meta): support logical view - #6023

Merged
mergify[bot] merged 19 commits into
mainfrom
xxchan/v
Nov 2, 2022
Merged

mergify[bot] merged 19 commits into
mainfrom
xxchan/v

Conversation

@xxchan

@xxchan xxchan commented Oct 25, 2022 •

Copy link
Copy Markdown
Contributor

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

What's changed and what's your intention?

close #2317

See e2e_test/batch/basic/logical_view.slt.part for examples

idea:

  • store sql (see View in catalog.proto).
  • create & drop are straightforward
  • query: get view from catalog, and bind it as subquery relation (see binder/relation/table_or_source.rs)

limitations:

  • View's dependants not resolved. e.g., when view V depends on mv (or table) MV, we can't drop MV before dropping V. But if mv (or view) MV depends on V, we can drop V.

    The difficulty is that currently after binding, view is completely replaced by the sql and the information of the view is lost. To support it, we may collect the dependent view ids in binder. Note that currently for mv, dependent relation ids are resolved in meta from stream fragment plan. For view, they are resolved in frontend from batch plan.

  • How to handle * in case of ddl (specifically adding column) is not yet considered

    example: create table t(x int); create view v as select * from t; alter table t add column y int; In PG, v only outputs x without y. This means * is resolved before the view is stored. This behavior is expected also because we can specify column names when creating views. It only makes sense if the view outputs fixed number of columns.

    The difficulty is that * is resolved in binder, but after binding, we can't restore the BoundQuery to AST and unparse it. One solution is to serializing plan instead of sql, but as discussed earlier we don't want to persist plan because it may be subject to change. Another solution is to rewriting AST while binding, but it may be a little bit hacky.

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

Release note

Support CREATE VIEW

See e2e_test/batch/basic/logical_view.slt.part for examples

@xxchan xxchan added the user-facing-changes Contains changes that are visible to users label Oct 25, 2022
@codecov

codecov Bot commented Oct 31, 2022 •

Copy link
Copy Markdown

Codecov Report

Merging #6023 (0bbb13e) into main (a878685) will decrease coverage by 0.29%.
The diff coverage is 30.71%.

@@            Coverage Diff             @@
##             main    #6023      +/-   ##
==========================================
- Coverage   74.65%   74.35%   -0.30%     
==========================================
  Files         931      935       +4     
  Lines      149324   150090     +766     
==========================================
+ Hits       111471   111601     +130     
- Misses      37853    38489     +636     
Flag Coverage Δ
rust 74.35% <30.71%> (-0.30%) ⬇️

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

Impacted Files Coverage Δ
src/common/src/catalog/internal_table.rs 100.00% <ø> (ø)
src/frontend/src/catalog/catalog_service.rs 5.42% <0.00%> (-0.36%) ⬇️
src/frontend/src/catalog/mod.rs 75.67% <ø> (ø)
src/frontend/src/catalog/view_catalog.rs 0.00% <0.00%> (ø)
src/frontend/src/handler/create_view.rs 0.00% <0.00%> (ø)
src/frontend/src/handler/drop_database.rs 80.51% <0.00%> (ø)
src/frontend/src/handler/drop_schema.rs 68.29% <0.00%> (ø)
src/frontend/src/handler/drop_view.rs 0.00% <0.00%> (ø)
src/frontend/src/handler/handle_privilege.rs 78.60% <0.00%> (+1.43%) ⬆️
src/frontend/src/observer/observer_manager.rs 0.00% <0.00%> (ø)
... and 75 more

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

@github-actions github-actions Bot added the type/feature Type: New feature. label Oct 31, 2022
@xxchan
xxchan marked this pull request as ready for review October 31, 2022 17:25

@HuaHuaY HuaHuaY left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Good Job!

Comment thread src/frontend/src/binder/relation/mod.rs
Comment thread src/frontend/src/handler/create_sink.rs Outdated
Comment thread src/frontend/src/handler/create_index.rs
Comment thread src/meta/src/manager/catalog/mod.rs
Comment thread src/meta/src/manager/catalog/mod.rs Outdated
Comment thread src/frontend/src/observer/observer_manager.rs

@yezizp2012 yezizp2012 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

generally LGTM, great job!!!

Comment thread proto/user.proto
uint32 source_id = 4;
uint32 all_tables_schema_id = 5;
uint32 all_sources_schema_id = 6;
uint32 view_id = 8;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We might need to support grant/revoke privileges for views (and all views in schema maybe) in user service. But that's non-related to this PR. Cc @HuaHuaY

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

Labels

type/feature Type: New feature. user-facing-changes Contains changes that are visible to users

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support logical view (CREATE VIEW commands)

3 participants