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

Reply via email to