risingwavelabs / risingwave

Best-in-class stream processing, analytics, and management. Perform continuous analytics, or build event-driven applications, real-time ETL pipelines, and feature stores in minutes. Unified streaming and batch. PostgreSQL compatible.
https://go.risingwave.com/slack
Apache License 2.0
7.07k stars 582 forks source link

bug(meta): `QueryRewriter` for `rename` shall be aware of schemas #19028

Open xiangjinwu opened 1 month ago

xiangjinwu commented 1 month ago

Describe the bug

The current definition accidentally updates all schemas. See reproduction steps for details. https://github.com/risingwavelabs/risingwave/blob/3c57ef8dd5d26f6eaedf42c169309a0751bcf5e6/src/meta/src/controller/rename.rs#L141-L144

Error message/log

No error reported, but the meta catalog would contain invalid SQL.

To Reproduce

dev=> create schema a;
CREATE_SCHEMA

dev=> create schema b;
CREATE_SCHEMA

dev=> create table a.foo(f1 int);
CREATE_TABLE

dev=> create table b.foo(f1 int);
CREATE_TABLE

dev=> create materialized view mv as select * from a.foo union all select * from b.foo;
CREATE_MATERIALIZED_VIEW

dev=> alter table a.foo rename to bar;
ALTER_TABLE

dev=> select definition from rw_materialized_views ;
                                           definition                                           
------------------------------------------------------------------------------------------------
 CREATE MATERIALIZED VIEW mv AS SELECT * FROM a.bar AS foo UNION ALL SELECT * FROM b.bar AS foo
(1 row)

Expected behavior

b.foo referenced by mv shall remain unchanged:

dev=> select definition from rw_materialized_views ;
                                           definition                                           
------------------------------------------------------------------------------------------------
 CREATE MATERIALIZED VIEW mv AS SELECT * FROM a.bar AS foo UNION ALL SELECT * FROM b.foo
(1 row)

How did you deploy RisingWave?

No response

The version of RisingWave

No response

Additional context

May not be that common in practice.

yezizp2012 commented 1 month ago

The complexity increases when a catalog is created with search_path set to specific schemas. Since we don't rewrite and persist the schema in the definition, renaming them leads to more inconsistencies. Therefore, it's necessary to also persist the schemas. I will try to fix it.

fuyufjh commented 4 weeks ago

Another related issue is select * from table. The table catalogs (columns) might be changed after alter table, so it's better to expand the * to actual columns when exeucuting create mv.