fix(frontend): refine error msg on materialized views(#3178)

[infdahai]

Signed-off-by: clundro [email protected]

What's changed and what's your intention?

Add an unit test drop_materialized_view_using_drop_source in handler/drop_source.rs .

Add some judges on materialized view before function get_source_by_name(with the error msg source not found).

Checklist

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

Refer to a related PR or issue link (optional)

issue: https://github.com/singularity-data/risingwave/issues/3178

[CLAassistant]

All committers have signed the CLA.

[codecov[bot]]

Codecov Report

Merging #3258 (3160b03) into main (faa6c30) will decrease coverage by 0.00%. The diff coverage is 70.09%.

@@            Coverage Diff             @@
##             main    #3258      +/-   ##
==========================================
- Coverage   74.31%   74.30%   -0.01%     
==========================================
  Files         772      772              
  Lines      109180   109258      +78     
==========================================
+ Hits        81134    81189      +55     
- Misses      28046    28069      +23     

Flag	Coverage Δ
rust	74.30% <70.09%> (-0.01%)	:arrow_down:

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

Impacted Files	Coverage Δ
src/frontend/src/handler/drop_source.rs	78.46% <27.27%> (-15.75%)	:arrow_down:
src/frontend/test_runner/src/lib.rs	73.23% <67.27%> (-1.01%)	:arrow_down:
src/frontend/src/binder/delete.rs	75.86% <77.77%> (-0.33%)	:arrow_down:
src/frontend/src/binder/insert.rs	93.04% <80.00%> (-1.30%)	:arrow_down:
src/frontend/src/binder/update.rs	69.44% <80.00%> (+1.19%)	:arrow_up:
src/frontend/src/catalog/root_catalog.rs	87.34% <100.00%> (+2.98%)	:arrow_up:
src/meta/src/hummock/mock_hummock_meta_client.rs	40.56% <0.00%> (-0.95%)	:arrow_down:
src/meta/src/hummock/compaction_group/manager.rs	90.47% <0.00%> (-0.48%)	:arrow_down:
src/meta/src/rpc/server.rs	80.13% <0.00%> (-0.24%)	:arrow_down:
... and 8 more

:mega: Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

[skyzh]

Please sign CLA :)

[infdahai]

dev=> CREATE TABLE t;
CREATE_TABLE
dev=> create materialized view mv as select * from t;
CREATE_MATERIALIZED_VIEW
dev=> insert into mv values (233);
ERROR:  Invalid input syntax: cannot change materialized view mv

I change the original error msg to Invalid input syntax: cannot change materialized view mv.

[infdahai]

 - sql:  |
     create table t;
     create source t with ( 'connector' = 'datagen' ) row format json;
#  error msg: failed basic_query.yaml 
#   failed on case #22 (id: <none>): Catalog error: table with name t exists: Catalog error: table with name t exists
 - sql:  |
     create source t with ( 'connector' = 'datagen' ) row format json;
     create table t;
#  error msg: failed basic_query.yaml 
#   failed on case #22 (id: <none>): Catalog error: source with name t exists: Catalog error: source with name t exists
 - sql:  |
     create materialized source t (v1 int) with ( 'connector' = 'datagen' )  row format json;
     create source t with ( 'connector' = 'datagen' ) row format json;
#  error msg: failed basic_query.yaml 
#   failed on case #22 (id: <none>): Catalog error: materialized source with name t exists: Catalog error: materialized source with name t exists
 - sql:
     create table s;
     create materialized view mv as select * from s;
     drop source mv;
#  error msg: failed basic_query.yaml 
#   failed on case #22 (id: <none>): Invalid input syntax: Use `DROP MATERIALIZED VIEW` to drop a materialized view.: Invalid input syntax: Use `DROP MATERIALIZED VIEW` to drop a materialized view.

The above test cases can produce Catalog error msg and I havn't found the corresponding solution. So I comment out these sql codes.

[BugenZhao]

```yaml
 - sql:  |
     create table t;
     create source t with ( 'connector' = 'datagen' ) row format json;
#  error msg: failed basic_query.yaml 
#   failed on case #22 (id: <none>): Catalog error: table with name t exists: Catalog error: table with name t exists
 - sql:  |
     create source t with ( 'connector' = 'datagen' ) row format json;
     create table t;
#  error msg: failed basic_query.yaml 
#   failed on case #22 (id: <none>): Catalog error: source with name t exists: Catalog error: source with name t exists
 - sql:  |
     create materialized source t (v1 int) with ( 'connector' = 'datagen' )  row format json;
     create source t with ( 'connector' = 'datagen' ) row format json;
#  error msg: failed basic_query.yaml 
#   failed on case #22 (id: <none>): Catalog error: materialized source with name t exists: Catalog error: materialized source with name t exists
 - sql:
     create table s;
     create materialized view mv as select * from s;
     drop source mv;
#  error msg: failed basic_query.yaml 
#   failed on case #22 (id: <none>): Invalid input syntax: Use `DROP MATERIALIZED VIEW` to drop a materialized view.: Invalid input syntax: Use `DROP MATERIALIZED VIEW` to drop a materialized view.

The above test cases can produce Catalog error msg and I havn't found the corresponding solution. So I comment out these sql codes.

Do you mean we cannot capture these error in the test runner? cc @skyzh

[infdahai]

Do you mean we cannot capture these error in the test runner? cc @skyzh

Afterwards I add the catalog_err part to the test runner and then the command ./risedev do-apply-planner-test outputs Diff applied!. So can you review that?

[infdahai]

I report this error for changing mv operations, not creating. So this error will be thrown in binder error. And creating mv error occurs in catalog error.

@liurenjie1024 I search the bind_table_source and find it's invoked on binder/insert.rs, binder/delete.rs and binder/update.rs .

So bind_table_source just is used by insert,delete and update option.

[liurenjie1024]

I report this error for changing mv operations, not creating. So this error will be thrown in binder error. And creating mv error occurs in catalog error.

@liurenjie1024 I search the bind_table_source and find it's invoked on binder/insert.rs, binder/delete.rs and binder/update.rs .

So bind_table_source just is used by insert,delete and update option.

I don't think we should do this check in binder just because it's only used by those. We should explicity disable changing mv in handlers, which makes code clear and more readable.

[infdahai]

I report this error for changing mv operations, not creating. So this error will be thrown in binder error. And creating mv error occurs in catalog error. @liurenjie1024 I search the bind_table_source and find it's invoked on binder/insert.rs, binder/delete.rs and binder/update.rs . So bind_table_source just is used by insert,delete and update option.

I don't think we should do this check in binder just because it's only used by those. We should explicity disable changing mv in handlers, which makes code clear and more readable.

I understand your words. And I move this code to concrete insert,delete and update error handler .

[fuyufjh]

Any updates?

[xxchan]

Kindly ping @infdahai. Are you still intersted in this PR? If not, I will help resolve conflicts and refine it later.