feat(frontend): initially introduce table def sql purification #19949

BugenZhao · 2024-12-27T09:37:12Z

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

See #17472 for the background. This PR initially introduces SQL purification for table definition, and adopt it in SHOW CREATE TABLE. Further adoption (like for replacing table plan) will be done in future PRs, as they require additional work that is not directly relevant to the current scope to ensure a complete end-to-end implementation.

It turns out that we only need to restore the ColumnDef (specifically, the DataType, which is already contributed by @StrikeW). This makes the procedure much simpler than initially thought.

Checklist

I have written necessary rustdoc comments.
I have added necessary unit tests and integration tests.
I have added test labels as necessary.
I have added fuzzing tests or opened an issue to track them.
My PR contains breaking changes.
My PR changes performance-critical code, so I will run (micro) benchmarks and present the results.
My PR contains critical fixes that are necessary to be merged into the latest release.

Documentation

My PR needs documentation updates.

Release note

BugenZhao · 2024-12-27T09:37:31Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

src/frontend/src/catalog/table_catalog.rs

StrikeW

LGTM

src/frontend/src/catalog/purify.rs

xxchan · 2025-01-02T06:18:14Z

e2e_test/ddl/show_purify.slt

+show create table ctas;
+----
+public.ctas	CREATE TABLE ctas (v0 INT, v1 NUMERIC, v2 TIMESTAMP, v3 TIMESTAMP WITH TIME ZONE, v4 CHARACTER VARYING[], v5 STRUCT<i BIGINT, j STRUCT<a BIGINT, b CHARACTER VARYING>>, v6 rw_int256, v7 MAP(CHARACTER VARYING,INT))


After the change, how/is it possible (is it meaningful?) to get the "original create sql" for CTAS and schema registry? It seems useful for debugging/development purpose.

It can be done via inspecting definition field of the metadata (like through the dashboard). However, I suppose it's indeed not that meaningful, as altering a CTAS will cause the purified definition to be persisted.

as altering a CTAS will cause the purified definition to be persisted.

Note: This is not the current behavior as purified definition is only used for SHOW but not the base for schema change replanning.

xxchan · 2025-01-02T09:10:29Z

src/frontend/src/catalog/purify.rs

+
+    // Schema inferred. Now derive the missing columns and constraints.
+    // First, remove the wildcard from the definition.
+    *wildcard_idx = None;


We might need tests for wildcard, and generated column, include column ,etc

Currently, it's used solely for observability, so we might introduce comprehensive tests at a later stage.

wildcard, and generated column

Added in 265f6f7.

include column

Will be included in #19965.

src/frontend/src/catalog/purify.rs

github-actions

license-eye has checked 5535 files.

Valid	Invalid	Ignored	Fixed
2332	2	3201	0

Click to see the invalid file list

src/frontend/src/catalog/purify.rs
src/frontend/src/utils/data_type.rs

Use this command to fix any missing license headers

```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

src/frontend/src/catalog/purify.rs

src/frontend/src/utils/data_type.rs

Signed-off-by: Bugen Zhao <[email protected]>

st1page · 2025-01-03T05:41:52Z

src/frontend/src/catalog/table_catalog.rs

+    pub fn create_sql_purified(&self) -> String {
+        self.create_sql_ast_purified()
+            .map(|stmt| stmt.to_string())
+            .unwrap_or_else(|_| self.create_sql())
+    }


Should we still return the result here? Not sure if we can guarantee this for all exist tables created previously.

By falling back to self.create_sql() (i.e. self.definition) we ensure that the behavior gets strictly better compared to prior to this PR. 🤔 Not sure what you mean by "guarantee this"?

github-actions bot added the type/feature label Dec 27, 2024

BugenZhao mentioned this pull request Dec 27, 2024

refactor(frontend): extract code for parsing table def sql into single stmt #19948

Merged

8 tasks

graphite-app bot reviewed Dec 27, 2024

View reviewed changes

src/frontend/src/catalog/table_catalog.rs Outdated Show resolved Hide resolved

BugenZhao force-pushed the bz/purify-sql-part-1 branch 2 times, most recently from c2b4de2 to ca9e6ef Compare December 30, 2024 05:37

BugenZhao marked this pull request as ready for review December 30, 2024 05:47

BugenZhao requested review from StrikeW, xxchan and st1page December 30, 2024 05:48

Base automatically changed from bz/zealous-wombat to main December 30, 2024 06:19

graphite-app bot requested a review from a team December 30, 2024 06:19

BugenZhao force-pushed the bz/purify-sql-part-1 branch from 45ab48e to 073b15c Compare December 30, 2024 09:05

BugenZhao requested a review from fuyufjh December 30, 2024 09:35

BugenZhao mentioned this pull request Dec 30, 2024

feat(frontend): purification for source def sql #19965

Open

8 tasks

BugenZhao force-pushed the bz/purify-sql-part-1 branch from 7922a7b to 7fd1ad9 Compare December 31, 2024 07:40

graphite-app bot requested a review from a team December 31, 2024 08:23

StrikeW approved these changes Jan 2, 2025

View reviewed changes

src/frontend/src/catalog/purify.rs Show resolved Hide resolved

xxchan reviewed Jan 2, 2025

View reviewed changes

BugenZhao mentioned this pull request Jan 2, 2025

feat(frontend): "purify" column default value #19993

Open

8 tasks

xxchan reviewed Jan 2, 2025

View reviewed changes

src/frontend/src/catalog/purify.rs Show resolved Hide resolved

BugenZhao mentioned this pull request Jan 2, 2025

refactor(frontend): reuse def purification for assembling cdc table def when auto schema change #19997

Draft

8 tasks

github-actions bot reviewed Jan 2, 2025

View reviewed changes

src/frontend/src/catalog/purify.rs Outdated Show resolved Hide resolved

src/frontend/src/utils/data_type.rs Outdated Show resolved Hide resolved

BugenZhao force-pushed the bz/purify-sql-part-1 branch from 265f6f7 to efa9a22 Compare January 2, 2025 09:45

BugenZhao requested a review from xxchan January 3, 2025 02:43

BugenZhao added 4 commits January 3, 2025 11:29

purify sql part 1

0989795

Signed-off-by: Bugen Zhao <[email protected]>

add docs & rename

79e6808

Signed-off-by: Bugen Zhao <[email protected]>

refine docs

b6a4a39

Signed-off-by: Bugen Zhao <[email protected]>

methodize to_ast

d50570c

Signed-off-by: Bugen Zhao <[email protected]>

BugenZhao added 12 commits January 3, 2025 11:29

rename methods

c852c43

Signed-off-by: Bugen Zhao <[email protected]>

update create table docs

4280204

Signed-off-by: Bugen Zhao <[email protected]>

add show create table test for ctas

8f47274

Signed-off-by: Bugen Zhao <[email protected]>

correctly handle purification for external schema

a688e58

Signed-off-by: Bugen Zhao <[email protected]>

reorganize code

ae6a230

Signed-off-by: Bugen Zhao <[email protected]>

fix flaky test

4559392

Signed-off-by: Bugen Zhao <[email protected]>

also use purified in rw_tables

9dfc0d1

Signed-off-by: Bugen Zhao <[email protected]>

implement more to_ast

f39063a

Signed-off-by: Bugen Zhao <[email protected]>

test more data types

154c590

Signed-off-by: Bugen Zhao <[email protected]>

test for wildcard and generated column

9209bb8

Signed-off-by: Bugen Zhao <[email protected]>

fix license header

15371b8

Signed-off-by: Bugen Zhao <[email protected]>

add constraints

5a21eb2

Signed-off-by: Bugen Zhao <[email protected]>

BugenZhao force-pushed the bz/purify-sql-part-1 branch from 2869377 to 5a21eb2 Compare January 3, 2025 03:29

st1page reviewed Jan 3, 2025

View reviewed changes

xxchan approved these changes Jan 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(frontend): initially introduce table def sql purification #19949

feat(frontend): initially introduce table def sql purification #19949

BugenZhao commented Dec 27, 2024 •

edited

Loading

BugenZhao commented Dec 27, 2024 •

edited

Loading

StrikeW left a comment

xxchan Jan 2, 2025

BugenZhao Jan 2, 2025

BugenZhao Jan 2, 2025

xxchan Jan 2, 2025

BugenZhao Jan 2, 2025

github-actions bot left a comment

st1page Jan 3, 2025

BugenZhao Jan 3, 2025

feat(frontend): initially introduce table def sql purification #19949

Are you sure you want to change the base?

feat(frontend): initially introduce table def sql purification #19949

Conversation

BugenZhao commented Dec 27, 2024 • edited Loading

What's changed and what's your intention?

Checklist

Documentation

BugenZhao commented Dec 27, 2024 • edited Loading

StrikeW left a comment

Choose a reason for hiding this comment

xxchan Jan 2, 2025

Choose a reason for hiding this comment

BugenZhao Jan 2, 2025

Choose a reason for hiding this comment

BugenZhao Jan 2, 2025

Choose a reason for hiding this comment

xxchan Jan 2, 2025

Choose a reason for hiding this comment

BugenZhao Jan 2, 2025

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

st1page Jan 3, 2025

Choose a reason for hiding this comment

BugenZhao Jan 3, 2025

Choose a reason for hiding this comment

BugenZhao commented Dec 27, 2024 •

edited

Loading

BugenZhao commented Dec 27, 2024 •

edited

Loading