Hisoka-X commented on code in PR #5564: URL: https://github.com/apache/seatunnel/pull/5564#discussion_r1338601437
########## docs/en/concept/schema-feature.md: ########## @@ -2,9 +2,55 @@ ## Why we need schema -Some NoSQL databases or message queue are not strongly limited schema, so the schema cannot be obtained through the api. At this time, a schema needs to be defined to convert to SeaTunnelRowType and obtain data. +Some NoSQL databases or message queue are not strongly limited schema, so the schema cannot be obtained through the api. +At this time, a schema needs to be defined to convert to SeaTunnelRowType and obtain data. Review Comment: ```suggestion At this time, a schema needs to be defined to convert to CatalogTable and obtain data. ``` How about use CatalogTable? ########## seatunnel-api/src/main/java/org/apache/seatunnel/api/table/catalog/CatalogTableUtil.java: ########## @@ -117,7 +113,7 @@ public static List<CatalogTable> getCatalogTables(Config config, ClassLoader cla List<CatalogTable> catalogTables = catalog.getTables(catalogConfig); log.info( String.format( - "Get catalog tables, cost time: %d", + "Get catalog tables, cost time: %d/ms", Review Comment: ```suggestion "Get catalog tables, cost time: %d ms", ``` ########## seatunnel-api/src/main/java/org/apache/seatunnel/api/table/catalog/CatalogTableUtil.java: ########## @@ -184,16 +180,31 @@ public static List<CatalogTable> getCatalogTablesFromConfig( } public static CatalogTable buildWithConfig(Config config) { - ReadonlyConfig readonlyConfig = ReadonlyConfig.fromConfig(config); - return buildWithConfig(readonlyConfig); + if (!config.hasPath(TableSchemaOptions.SCHEMA.key())) { Review Comment: So we can't convert `Config` to `ReadonlyConfig` to parse schema now? Why we should use different logic to treat ReadonlyConfig and Config? ########## seatunnel-api/src/test/java/org/apache/seatunnel/api/table/catalog/CatalogTableUtilTest.java: ########## @@ -65,7 +65,7 @@ public void testComplexSchemaParse() throws FileNotFoundException, URISyntaxExce String path = getTestConfigFile("/conf/complex.schema.conf"); Config config = ConfigFactory.parseFile(new File(path)); SeaTunnelRowType seaTunnelRowType = - CatalogTableUtil.buildWithConfig(config).getSeaTunnelRowType(); + CatalogTableUtil.buildWithConfig(config).getTableSchema().toPhysicalRowDataType(); Review Comment: ditto ########## seatunnel-common/src/main/java/org/apache/seatunnel/common/utils/JsonUtils.java: ########## @@ -178,6 +180,24 @@ public static Map<String, Object> toMap(JsonNode jsonNode) { jsonNode, new TypeReference<Map<String, Object>>() {}); } + public static Map<String, String> toLinkedHashMap(String json) { Review Comment: The name doesn't feel right, in fact, the jackson will return LinkedHashMap as default. I think you only want convert all type value to String? ########## docs/en/concept/schema-feature.md: ########## @@ -26,9 +72,111 @@ Some NoSQL databases or message queue are not strongly limited schema, so the sc | map | `java.util.Map` | A Map is an object that maps keys to values. The key type includes `int` `string` `boolean` `tinyint` `smallint` `bigint` `float` `double` `decimal` `date` `time` `timestamp` `null` , and the value type includes `int` `string` `boolean` `tinyint` `smallint` `bigint` `float` `double` `decimal` `date` `time` `timestamp` `null` `array` `map`. | | array | `ValueType[]` | A array is a data type that represents a collection of elements. The element type includes `int` `string` `boolean` `tinyint` `smallint` `bigint` `float` `double` `array` `map`. | +### PrimaryKey + +Primary key is a config used to define the primary key in schema, it contains name, columns field. + +``` +primaryKey { + name = id + columns = [id] +} +``` + +| Field | Required | Default Value | Description | +|:--------|:---------|:--------------|-----------------------------------| +| name | Yes | - | The name of the primaryKey | +| columns | Yes | - | The column list in the primaryKey | + +### ConstraintKeys + +Constraint keys is a list of config used to define the constraint keys in schema, it contains constraintName, constraintType, constraintColumns field. + +``` +constraintKeys = [ + { + constraintName = "id_index" + constraintType = KEY + constraintColumns = [ + { + columnName = "id" + sortType = ASC + } + ] + }, + ] +``` + +| Field | Required | Default Value | Description | +|:------------------|:---------|:--------------|-------------------------------------------------------------------------------------------------------------------------------------------| +| constraintName | Yes | - | The name of the constraintKey | +| constraintType | No | KEY | The type of the constraintKey | +| constraintColumns | Yes | - | The column list in the primaryKey, each column should contains constraintType and sortType, sortType support ASC and DESC, default is ASC | + +#### What constraintType supported at now + +| ConstraintType | Description | +|:---------------|:------------| +| KEY | key | +| UNIQUE_KEY | unique key | +| FOREIGN_KEY | foreign key | Review Comment: How to use `FOREIGN_KEY` in SeaTunnel? Seem like never use it now? ########## docs/en/concept/schema-feature.md: ########## @@ -26,9 +72,111 @@ Some NoSQL databases or message queue are not strongly limited schema, so the sc | map | `java.util.Map` | A Map is an object that maps keys to values. The key type includes `int` `string` `boolean` `tinyint` `smallint` `bigint` `float` `double` `decimal` `date` `time` `timestamp` `null` , and the value type includes `int` `string` `boolean` `tinyint` `smallint` `bigint` `float` `double` `decimal` `date` `time` `timestamp` `null` `array` `map`. | | array | `ValueType[]` | A array is a data type that represents a collection of elements. The element type includes `int` `string` `boolean` `tinyint` `smallint` `bigint` `float` `double` `array` `map`. | +### PrimaryKey + +Primary key is a config used to define the primary key in schema, it contains name, columns field. + +``` +primaryKey { + name = id + columns = [id] +} +``` + +| Field | Required | Default Value | Description | +|:--------|:---------|:--------------|-----------------------------------| +| name | Yes | - | The name of the primaryKey | +| columns | Yes | - | The column list in the primaryKey | + +### ConstraintKeys + +Constraint keys is a list of config used to define the constraint keys in schema, it contains constraintName, constraintType, constraintColumns field. + +``` +constraintKeys = [ + { + constraintName = "id_index" + constraintType = KEY + constraintColumns = [ + { + columnName = "id" + sortType = ASC + } + ] + }, + ] +``` + +| Field | Required | Default Value | Description | +|:------------------|:---------|:--------------|-------------------------------------------------------------------------------------------------------------------------------------------| +| constraintName | Yes | - | The name of the constraintKey | +| constraintType | No | KEY | The type of the constraintKey | +| constraintColumns | Yes | - | The column list in the primaryKey, each column should contains constraintType and sortType, sortType support ASC and DESC, default is ASC | + +#### What constraintType supported at now + +| ConstraintType | Description | +|:---------------|:------------| +| KEY | key | Review Comment: What's `KEY` means? ########## seatunnel-api/src/main/java/org/apache/seatunnel/api/table/catalog/schema/ReadonlyConfigParser.java: ########## @@ -0,0 +1,378 @@ +/* Review Comment: From the code, we would maintain two part code for one feature. ########## seatunnel-api/src/test/java/org/apache/seatunnel/api/table/catalog/CatalogTableUtilTest.java: ########## @@ -51,7 +51,7 @@ public void testSimpleSchemaParse() throws FileNotFoundException, URISyntaxExcep String path = getTestConfigFile("/conf/simple.schema.conf"); Config config = ConfigFactory.parseFile(new File(path)); SeaTunnelRowType seaTunnelRowType = - CatalogTableUtil.buildWithConfig(config).getSeaTunnelRowType(); + CatalogTableUtil.buildWithConfig(config).getTableSchema().toPhysicalRowDataType(); Review Comment: Any reason for revert this? I remember `getSeaTunnelRowType()` is a wrapper of `getTableSchema().toPhysicalRowDataType()` ########## seatunnel-connectors-v2/connector-fake/src/test/java/org/apache/seatunnel/connectors/seatunnel/fake/source/FakeDataGeneratorTest.java: ########## @@ -111,7 +111,9 @@ public void testRowDataParse(String conf) throws FileNotFoundException, URISynta Config testConfig = getTestConfigFile(conf); SeaTunnelRowType seaTunnelRowType = - CatalogTableUtil.buildWithConfig(testConfig).getSeaTunnelRowType(); + CatalogTableUtil.buildWithConfig(testConfig) Review Comment: ditto -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@seatunnel.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org