[Feature] Support config column/primaryKey/constraintKey in schema #5564

ruanwenjun · 2023-09-26T08:43:31Z

Purpose of this pull request

Add some options in the schema used to define the whole TableSchema.

Does this PR introduce any user-facing change?

Yes, but this is compatibility with the history version.

How was this patch tested?

Add UT for this PR.
Add e2e to check the CatalogTable can be parsed from the config file and sent to sink.

Check list

If any new Jar binary package adding in your PR, please add License Notice according
New License Guide
If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
If you are contributing the connector code, please check that the following files are updated:
1. Update change log that in connector document. For more details you can refer to connector-v2
2. Update plugin-mapping.properties and add new connector information in it
3. Update the pom file of seatunnel-dist
Update the release-note.

docs/en/concept/schema-feature.md

Hisoka-X · 2023-09-27T13:23:01Z

docs/en/concept/schema-feature.md

+|:---------------|:------------|
+| KEY            | key         |
+| UNIQUE_KEY     | unique key  |
+| FOREIGN_KEY    | foreign key |


How to use FOREIGN_KEY in SeaTunnel? Seem like never use it now?

In fact, we don't support foreign keys!

So I think we should remove it?

OK, I will remove this until we support it in the future

Hisoka-X · 2023-09-27T13:23:30Z

docs/en/concept/schema-feature.md

+
+| ConstraintType | Description |
+|:---------------|:------------|
+| KEY            | key         |


What's KEY means?

KEY means the index in schema, I am not clear if we need to rename it to index, since in the backend code it is KEY.

how about INDEX_KEY?

Yes, use INDEX_KEY is better

seatunnel-api/src/main/java/org/apache/seatunnel/api/table/catalog/CatalogTableUtil.java

...el-api/src/main/java/org/apache/seatunnel/api/table/catalog/schema/ReadonlyConfigParser.java

seatunnel-api/src/test/java/org/apache/seatunnel/api/table/catalog/CatalogTableUtilTest.java

...c/test/java/org/apache/seatunnel/connectors/seatunnel/fake/source/FakeDataGeneratorTest.java

seatunnel-common/src/main/java/org/apache/seatunnel/common/utils/JsonUtils.java

Hisoka-X · 2023-09-28T06:07:37Z

docs/en/concept/schema-feature.md

+|:---------------|:------------|
+| KEY            | key         |
+| UNIQUE_KEY     | unique key  |
+| FOREIGN_KEY    | foreign key |


So I think we should remove it?

Hisoka-X · 2023-09-28T06:07:56Z

docs/en/concept/schema-feature.md

+
+| ConstraintType | Description |
+|:---------------|:------------|
+| KEY            | key         |


how about INDEX_KEY?

Hisoka-X · 2023-09-28T06:10:02Z

seatunnel-api/src/main/java/org/apache/seatunnel/api/table/catalog/CatalogTableUtil.java

+    @Deprecated
    public static final Option<Map<String, String>> SCHEMA =


Can we just delete it now? Since it just be used in internal code.

Yes, we can

Hisoka-X · 2023-09-28T06:10:19Z

seatunnel-api/src/main/java/org/apache/seatunnel/api/table/catalog/CatalogTableUtil.java

+    // Use TableSchemaOptions.FieldOptions.FIELDS instead
+    @Deprecated
    public static final Option<Map<String, Object>> FIELDS =
            Options.key("schema.fields")


Hisoka-X · 2023-09-28T06:18:55Z

Any demo we can directly run? I find we add this new feature but we don't have any e2e demo to prove it worked.

ruanwenjun · 2023-09-28T08:48:14Z

Any demo we can directly run? I find we add this new feature but we don't have any e2e demo to prove it worked.

I will add e2e after #5562 merged, right now the CatalogTable cannot transport from source to post task.

hailin0

LGTM

hailin0 · 2023-10-08T12:46:54Z

...el-api/src/main/java/org/apache/seatunnel/api/table/catalog/schema/ReadonlyConfigParser.java

+            implements TableSchemaParser.ConstraintKeyParser<ReadonlyConfig> {
+
+        @Override
+        public List<ConstraintKey> parse(ReadonlyConfig schemaConfig) {


this lambda is less readable

hailin0 · 2023-10-08T12:48:08Z

...el-api/src/main/java/org/apache/seatunnel/api/table/catalog/schema/ReadonlyConfigParser.java

+                                () ->
+                                        new IllegalArgumentException(
+                                                "schema.constraintKeys.* config need option [constraintType], please correct your config first"));
+        List<ConstraintKey.ConstraintKeyColumn> columns =


lambda is less readable

After ReadonlyConfig refactor, we may support get object from ReadonlyConfig or do this check in a better way.

ruanwenjun force-pushed the dev_wenjun_refactorSchemaDev branch 2 times, most recently from 9715f38 to a6d35a8 Compare September 26, 2023 08:49

ruanwenjun added the feature New feature label Sep 26, 2023

Hisoka-X added this to the 2.3.4 milestone Sep 26, 2023

ruanwenjun force-pushed the dev_wenjun_refactorSchemaDev branch from a6d35a8 to 2e595a0 Compare September 26, 2023 11:38

ruanwenjun requested review from Hisoka-X, ashulin and TyrantLucifer September 26, 2023 11:40

ruanwenjun force-pushed the dev_wenjun_refactorSchemaDev branch 3 times, most recently from f0b787c to 2b296e2 Compare September 27, 2023 09:42

Hisoka-X reviewed Sep 27, 2023

View reviewed changes

ruanwenjun force-pushed the dev_wenjun_refactorSchemaDev branch 2 times, most recently from 70682c7 to 640c451 Compare September 27, 2023 17:28

ruanwenjun requested a review from Hisoka-X September 27, 2023 17:30

Hisoka-X reviewed Sep 28, 2023

View reviewed changes

ruanwenjun marked this pull request as draft September 28, 2023 08:48

ruanwenjun force-pushed the dev_wenjun_refactorSchemaDev branch 11 times, most recently from b80fd3f to b08f83a Compare October 7, 2023 09:03

ruanwenjun force-pushed the dev_wenjun_refactorSchemaDev branch 8 times, most recently from a621ba9 to af7e2d7 Compare October 7, 2023 13:31

ruanwenjun marked this pull request as ready for review October 7, 2023 13:31

ruanwenjun requested a review from Hisoka-X October 8, 2023 02:38

ruanwenjun force-pushed the dev_wenjun_refactorSchemaDev branch from af7e2d7 to da21881 Compare October 8, 2023 06:46

Hisoka-X previously approved these changes Oct 8, 2023

View reviewed changes

github-actions bot added approved reviewed labels Oct 8, 2023

ruanwenjun dismissed Hisoka-X’s stale review via cb7a606 October 8, 2023 07:20

ruanwenjun force-pushed the dev_wenjun_refactorSchemaDev branch from da21881 to cb7a606 Compare October 8, 2023 07:20

github-actions bot removed approved reviewed labels Oct 8, 2023

Support config column/primaryKey/constraintKey in schema

d300a88

ruanwenjun force-pushed the dev_wenjun_refactorSchemaDev branch from cb7a606 to d300a88 Compare October 8, 2023 09:47

Hisoka-X approved these changes Oct 8, 2023

View reviewed changes

github-actions bot added approved reviewed labels Oct 8, 2023

hailin0 reviewed Oct 8, 2023

View reviewed changes

hailin0 approved these changes Oct 8, 2023

View reviewed changes

ruanwenjun merged commit eac76b4 into apache:dev Oct 8, 2023
8 checks passed

ruanwenjun deleted the dev_wenjun_refactorSchemaDev branch October 8, 2023 13:50

gnehil pushed a commit to gnehil/seatunnel that referenced this pull request Oct 12, 2023

Support config column/primaryKey/constraintKey in schema (apache#5564)

cc8f4bd

ruanwenjun mentioned this pull request Oct 17, 2023

[OnlineMeeting& Oct.17]SeaTunnel community meeting Topic collect #5639

Closed

3 tasks

ruanwenjun changed the title ~~Support config column/primaryKey/constraintKey in schema~~ [Feature] Support config column/primaryKey/constraintKey in schema Oct 17, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Feature] Support config column/primaryKey/constraintKey in schema #5564

[Feature] Support config column/primaryKey/constraintKey in schema #5564

ruanwenjun commented Sep 26, 2023 •

edited

Loading

Hisoka-X Sep 27, 2023

ruanwenjun Sep 27, 2023

Hisoka-X Sep 28, 2023

ruanwenjun Sep 28, 2023

Hisoka-X Sep 27, 2023

ruanwenjun Sep 27, 2023

Hisoka-X Sep 28, 2023

ruanwenjun Sep 28, 2023

Hisoka-X Sep 28, 2023

Hisoka-X Sep 28, 2023

Hisoka-X Sep 28, 2023

ruanwenjun Sep 28, 2023

Hisoka-X Sep 28, 2023

Hisoka-X commented Sep 28, 2023

ruanwenjun commented Sep 28, 2023

hailin0 left a comment

hailin0 Oct 8, 2023

hailin0 Oct 8, 2023

ruanwenjun Oct 8, 2023

		@Deprecated
		public static final Option<Map<String, String>> SCHEMA =

[Feature] Support config column/primaryKey/constraintKey in schema #5564

[Feature] Support config column/primaryKey/constraintKey in schema #5564

Conversation

ruanwenjun commented Sep 26, 2023 • edited Loading

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hisoka-X commented Sep 28, 2023

ruanwenjun commented Sep 28, 2023

hailin0 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

ruanwenjun commented Sep 26, 2023 •

edited

Loading