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

Resolve logical view's dependants #6160

Closed
xxchan opened this issue Nov 1, 2022 · 5 comments · Fixed by #8784
Closed

Resolve logical view's dependants #6160

xxchan opened this issue Nov 1, 2022 · 5 comments · Fixed by #8784

Comments

@xxchan
Copy link
Member

xxchan commented Nov 1, 2022

Original posted at #6023

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.

@xxchan
Copy link
Member Author

xxchan commented Nov 10, 2022

It is a little bit tricky to resolve dependencies correctly...

Since currently we resolve dependencies for mvs from the plan, unnecessary tables in a sql will not be treated as dependencies.

e.g. Current behavior in risingwave:

dev=> create table t1(x int); create table t2(x int);
      create materialized view mv as 
         with ctx as (select x from t1) select x from t2;
dev=> drop table t1;
DROP_TABLE

In PG, this is error: materialized view mv depends on table t1

If we resolve dependencies syntactically from the BoundQuery, it will behaves the same. (The problem for view is similar, i.e., view doesn't occur in the plan).

However, there's another problem: a table (index, specifically) can occur in the plan even if it doesn't occur in the ast.

- sql: |
    create table t1 (v1 int, v2 float);
    create table t2 (v3 int, v4 numeric, v5 bigint);
    create index t1_v1 on t1(v1) include(v2);
    create index t2_v3 on t2(v3) include(v4, v5);
    /* should generate delta join plan, and stream index scan */
    select * from t1, t2 where t1.v1 = t2.v3;
  stream_plan: |
    StreamMaterialize { columns: [v1, v2, v3, v4, v5, t1_v1.t1._row_id(hidden), t2_v3.t2._row_id(hidden)], pk_columns: [t1_v1.t1._row_id, t2_v3.t2._row_id, v1, v3] }
    └─StreamDeltaJoin { type: Inner, predicate: t1_v1.v1 = t2_v3.v3, output: [t1_v1.v1, t1_v1.v2, t2_v3.v3, t2_v3.v4, t2_v3.v5, t1_v1.t1._row_id, t2_v3.t2._row_id] }
      ├─StreamIndexScan { index: t1_v1, columns: [t1_v1.v1, t1_v1.v2, t1_v1.t1._row_id], pk: [t1_v1.t1._row_id], dist: HashShard(t1_v1.v1) }
      └─StreamIndexScan { index: t2_v3, columns: [t2_v3.v3, t2_v3.v4, t2_v3.v5, t2_v3.t2._row_id], pk: [t2_v3.t2._row_id], dist: HashShard(t2_v3.v3) }

@xxchan
Copy link
Member Author

xxchan commented Nov 10, 2022

Well, resolving dependencies twice can solve the problem..

@xxchan
Copy link
Member Author

xxchan commented Nov 10, 2022

TBH I think current behavior is also reasonable to some extent... 🤪

@github-actions
Copy link
Contributor

This issue has been open for 60 days with no activity. Could you please update the status? Feel free to continue discussion or close as not planned.

@xxchan
Copy link
Member Author

xxchan commented Mar 27, 2023

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.

This is added as shared_views when implementing LogicalShare.

Resolving dependencies is implemented in #8784

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

Successfully merging a pull request may close this issue.

2 participants