feat(frontend): deprecate CREATE MATERIALIZED SOURCE syntax #7281

xx01cyx · 2023-01-10T05:33:06Z

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

What's changed and what's your intention?

From this PR on, we no longer support CREATE MATERIALIZED SOURCE syntax. Users will use CREATE TABLE instead of CREATE MATERIALIZED SOURCE to create a table with a connector associated with it. The remaining part of the SQL command is the same as before.

Also, users will be able to perform INSERT, DELETE, and UPDATE operations on a table with a connector associated with it.

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

Types of user-facing changes

Connector (sources & sinks)
SQL commands, functions, and operators

Release note

Create a table with a connector associated with it:

CREATE TABLE t (
    v int
) WITH (
    connector = ‘datagen’
) ROW FORMAT json;

CREATE_TABLE

Insert into a table with a connector associated with it:

INSERT INTO t VALUES (99999999);

INSERT 0 1

Update a table with a connector associated with it:

UPDATE t SET v = 11111111 WHERE v = 99999999;

UPDATE 1

Delete from a table with a connector associated with it:

DELETE FROM t WHERE v = 11111111;

DELETE 1

Misuse old syntax:

CREATE MATERIALIZED SOURCE s (
    v int
) WITH (
    connector = ‘datagen’
) ROW FORMAT json;

ERROR:  QueryError: sql parser error: CREATE MATERIALIZED SOURCE has been deprecated, use CREATE TABLE instead

Refer to a related PR or issue link

Close #5949

hzxa21 · 2023-01-10T12:38:47Z

Does this PR maintain backward compatibility? In other words, if a user has already created a materialized source with CREATE MATERIALIZED SOURCE and upgrades the codes to include this PR, will the existing materialized source still be usable?

xx01cyx · 2023-01-11T04:55:54Z

Does this PR maintain backward compatibility? In other words, if a user has already created a materialized source with CREATE MATERIALIZED SOURCE and upgrades the codes to include this PR, will the existing materialized source still be usable?

Not tested yet, but I'll try to make it backward-compatible.

…materialized-source

st1page · 2023-01-11T05:57:59Z

Does this PR maintain backward compatibility? In other words, if a user has already created a materialized source with CREATE MATERIALIZED SOURCE and upgrades the codes to include this PR, will the existing materialized source still be usable?

I prefer to not maintain the compatibility at all. IIUC, currently our catalog and other meta format does not provide any compatibility guarantee. 🤔

xx01cyx · 2023-01-11T06:15:50Z

Does this PR maintain backward compatibility? In other words, if a user has already created a materialized source with CREATE MATERIALIZED SOURCE and upgrades the codes to include this PR, will the existing materialized source still be usable?

Not tested yet, but I'll try to make it backward-compatible.

Per offline discussion with @st1page , backward capability is non-trivial and our system currently has no guarantee on it. We could find out a workaround if there are user requirements for this. So this PR might not include backward capability.

hzxa21 · 2023-01-11T06:21:16Z

Does this PR maintain backward compatibility? In other words, if a user has already created a materialized source with CREATE MATERIALIZED SOURCE and upgrades the codes to include this PR, will the existing materialized source still be usable?

Not tested yet, but I'll try to make it backward-compatible.

Per offline discussion with @st1page , backward capability is non-trivial and our system currently has no guarantee on it. We could find out a workaround if there are user requirements for this. So this PR might not include backward capability.

Also discussed offline with @st1page. Maintaining compatibility is hard and may not worth it. If there is a need to upgrade version for the already deployed cases, we can write a simple tool to migrate old metadata to the new one on demand. @fuyufjh What do you think?

…materialized-source

codecov · 2023-01-13T06:24:15Z

Codecov Report

Merging #7281 (c471f60) into main (0bb78f6) will increase coverage by 0.02%.
The diff coverage is 67.82%.

@@            Coverage Diff             @@
##             main    #7281      +/-   ##
==========================================
+ Coverage   71.63%   71.66%   +0.02%     
==========================================
  Files        1083     1083              
  Lines      173692   173575     -117     
==========================================
- Hits       124428   124384      -44     
+ Misses      49264    49191      -73

Flag	Coverage Δ
rust	`71.66% <67.82%> (+0.02%)`	⬆️

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

Impacted Files	Coverage Δ
src/connector/src/source/datagen/source/reader.rs	`85.53% <ø> (ø)`
src/frontend/src/handler/create_table_as.rs	`0.00% <ø> (ø)`
src/frontend/src/handler/drop_source.rs	`0.00% <0.00%> (ø)`
src/frontend/src/catalog/root_catalog.rs	`67.81% <33.33%> (ø)`
src/frontend/src/handler/alter_table.rs	`85.40% <50.00%> (ø)`
src/frontend/src/handler/drop_table.rs	`83.33% <50.00%> (+0.31%)`	⬆️
src/sqlparser/src/ast/mod.rs	`88.76% <50.00%> (+<0.01%)`	⬆️
src/sqlparser/src/parser.rs	`91.99% <66.66%> (-0.11%)`	⬇️
src/frontend/src/catalog/table_catalog.rs	`93.58% <75.00%> (+0.60%)`	⬆️
src/frontend/src/handler/create_source.rs	`65.28% <77.77%> (+0.47%)`	⬆️
... and 26 more

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

xx01cyx · 2023-01-13T06:50:16Z

This PR is ready for review. PTAL @st1page @BugenZhao @tabVersion

fuyufjh

LGTM

fuyufjh · 2023-01-13T06:59:48Z

e2e_test/source/basic/ddl.slt

+# TODO: `show sources` should display connectors created by `create source` only.
+# We should introduce `show connectors` to display all connectors created via both
+# `create source` and `create table`.


🤔 In which case a user may want to show connectors? (IIUC connectors cannot be used to do anything)

Maybe users should be able to view all created connectors. 🤔 cc. @st1page @yezizp2012

I think user may only care about which connector is used by some table and SHOW CREATE TABLE is already enough for it. 🤔

agree. no need for show connectors

yezizp2012

LGTM!

src/frontend/planner_test/src/lib.rs

BugenZhao · 2023-01-13T10:03:58Z

src/frontend/src/catalog/table_catalog.rs

@@ -239,15 +239,7 @@ impl TableCatalog {
                "Use `DROP MATERIALIZED VIEW` to drop a materialized view."
            }
            TableType::Index => "Use `DROP INDEX` to drop an index.",
-            TableType::Table => {
-                // TODO(Yuanxin): Remove this after unsupporting `CREATE MATERIALIZED SOURCE`.
-                // Note(bugen): may make this a method on `TableType` instead.


What about implementing this directly on TableType?

Not sure whether it's a good idea to separate Table and TableWithConnector as two types. cc @st1page

Both LGTM. Never mind. 😄

src/frontend/src/optimizer/mod.rs

tabVersion

basically LGTM

tabVersion · 2023-01-16T08:06:49Z

e2e_test/source/basic/ddl.slt

+# TODO: `show sources` should display connectors created by `create source` only.
+# We should introduce `show connectors` to display all connectors created via both
+# `create source` and `create table`.


agree. no need for show connectors

tabVersion · 2023-01-16T08:14:45Z

better add an error to tell the users that we no longer support CREATE MATERIALIZED SOURCE and let them know to use create table with connector instead

st1page · 2023-01-18T06:03:57Z

@Mergifyio refresh

mergify · 2023-01-18T06:04:04Z

refresh

✅ Pull request refreshed

deprecate CREATE MATERIALIZED SOURCE syntax

dadca92

github-actions bot added the type/feature label Jan 10, 2023

xx01cyx added the user-facing-changes Contains changes that are visible to users label Jan 10, 2023

xx01cyx marked this pull request as draft January 11, 2023 04:54

xx01cyx added 2 commits January 11, 2023 05:05

Merge remote-tracking branch 'origin/main' into cyx/deprecate-create-…

d2ed46c

…materialized-source

fix ci

13848d8

xx01cyx added 2 commits January 12, 2023 05:28

fix ci

fcda273

merge main and resolve conflicts

aaec00b

xx01cyx mentioned this pull request Jan 12, 2023

feat: SQL commands to show sources and connectors #7344

Closed

xx01cyx added 5 commits January 12, 2023 05:52

fix build

501ab79

fix planner test

0e9e2bd

resolve table before source when checking duplication

3833426

fix typo in invalid operation

9d41680

Merge remote-tracking branch 'origin/main' into cyx/deprecate-create-…

e2034d4

…materialized-source

add back the test case removed before

9aa11a8

xx01cyx marked this pull request as ready for review January 13, 2023 06:49

xx01cyx requested review from tabVersion, BugenZhao and st1page January 13, 2023 06:49

fuyufjh approved these changes Jan 13, 2023

View reviewed changes

yezizp2012 approved these changes Jan 13, 2023

View reviewed changes

BugenZhao reviewed Jan 13, 2023

View reviewed changes

tabVersion approved these changes Jan 16, 2023

View reviewed changes

remove match statement & remove materialized source from planner test

faa10bd

BugenZhao approved these changes Jan 17, 2023

View reviewed changes

st1page approved these changes Jan 17, 2023

View reviewed changes

merge main and resolve conflicts

e7215b7

xx01cyx added the mergify/can-merge label Jan 18, 2023

mergify bot added 2 commits January 18, 2023 04:03

Merge branch 'main' into cyx/deprecate-create-materialized-source

fa1a13c

Merge branch 'main' into cyx/deprecate-create-materialized-source

9f496db

Merge branch 'main' into cyx/deprecate-create-materialized-source

c471f60

st1page added mergify/high-priority and removed mergify/high-priority labels Jan 18, 2023

TennyZhuang approved these changes Jan 18, 2023

View reviewed changes

TennyZhuang merged commit 5187083 into main Jan 18, 2023

TennyZhuang deleted the cyx/deprecate-create-materialized-source branch January 18, 2023 07:44

st1page mentioned this pull request Jan 18, 2023

Tracking: new DML design #5949

Closed

9 tasks

neverchanje added the breaking-change label Jan 18, 2023

xxchan mentioned this pull request Jan 27, 2023

Discussion: consider backward compatibility risingwavelabs/rfcs#41

Open

hzxa21 mentioned this pull request Feb 7, 2023

show source tables in information_schema.tables and show tables #7739

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(frontend): deprecate CREATE MATERIALIZED SOURCE syntax #7281

feat(frontend): deprecate CREATE MATERIALIZED SOURCE syntax #7281

xx01cyx commented Jan 10, 2023 •

edited

Loading

hzxa21 commented Jan 10, 2023

xx01cyx commented Jan 11, 2023

st1page commented Jan 11, 2023

xx01cyx commented Jan 11, 2023

hzxa21 commented Jan 11, 2023

codecov bot commented Jan 13, 2023 •

edited

Loading

xx01cyx commented Jan 13, 2023

fuyufjh left a comment

fuyufjh Jan 13, 2023

xx01cyx Jan 13, 2023

yezizp2012 Jan 13, 2023

tabVersion Jan 16, 2023

yezizp2012 left a comment

BugenZhao Jan 13, 2023

xx01cyx Jan 16, 2023

BugenZhao Jan 17, 2023

tabVersion left a comment

tabVersion Jan 16, 2023

tabVersion commented Jan 16, 2023

st1page commented Jan 18, 2023

mergify bot commented Jan 18, 2023

feat(frontend): deprecate CREATE MATERIALIZED SOURCE syntax #7281

feat(frontend): deprecate CREATE MATERIALIZED SOURCE syntax #7281

Conversation

xx01cyx commented Jan 10, 2023 • 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

hzxa21 commented Jan 10, 2023

xx01cyx commented Jan 11, 2023

st1page commented Jan 11, 2023

xx01cyx commented Jan 11, 2023

hzxa21 commented Jan 11, 2023

codecov bot commented Jan 13, 2023 • edited Loading

Codecov Report

xx01cyx commented Jan 13, 2023

fuyufjh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yezizp2012 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tabVersion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tabVersion commented Jan 16, 2023

st1page commented Jan 18, 2023

mergify bot commented Jan 18, 2023

✅ Pull request refreshed

xx01cyx commented Jan 10, 2023 •

edited

Loading

codecov bot commented Jan 13, 2023 •

edited

Loading