DanielCarter-stack commented on PR #10413:
URL: https://github.com/apache/seatunnel/pull/10413#issuecomment-3815848910
<!-- code-pr-reviewer -->
<!-- cpr:pr_reply_v2_parts {"group": "apache/seatunnel#10413", "part": 1,
"total": 1} -->
### Issue 1: Incorrect Java type mapping for row type in documentation
**Location**:
- `docs/en/introduction/concepts/schema-feature.md:96`
- `docs/zh/introduction/concepts/schema-feature.md:96`
**Modified code**:
```markdown
-| row | `org.apache.seatunnel.api.table.type.SeaTunnelRow` | Row type, can
be nested. |
+| row | `org.apache.seatunnel.api.table.type.SeaTunnelRowType` | Row type,
can be nested. |
```
**Related context**:
- Type definition:
`seatunnel-api/src/main/java/org/apache/seatunnel/api/table/type/SeaTunnelRowType.java:25`
- Value class definition:
`seatunnel-api/src/main/java/org/apache/seatunnel/api/table/type/SeaTunnelRow.java:28`
- Interface definition:
`seatunnel-api/src/main/java/org/apache/seatunnel/api/table/type/SeaTunnelDataType.java:23`
- Composite type interface:
`seatunnel-api/src/main/java/org/apache/seatunnel/api/table/type/CompositeType.java:22`
**Problem description**:
In the documentation table, the "Value type in Java" column for the `row`
type incorrectly contains `SeaTunnelRow` (runtime value class), when it should
contain `SeaTunnelRowType` (type definition class).
This is confirmed through code analysis:
```java
// SeaTunnelRowType implements CompositeType<SeaTunnelRow>
public class SeaTunnelRowType implements CompositeType<SeaTunnelRow> {
@Override
public Class<SeaTunnelRow> getTypeClass() {
return SeaTunnelRow.class; // Return value class, but itself is a
type definition
}
}
```
This is consistent with the pattern of other types in the table:
- `map` type → `MapType<K, V>` (type definition), `getTypeClass()` →
`Map.class` (value class)
- `array` type → `ArrayType<T, E>` (type definition), `getTypeClass()` →
`T[].class` (value class)
- `row` type → `SeaTunnelRowType` (type definition), `getTypeClass()` →
`SeaTunnelRow.class` (value class)
**Potential risks**:
- Risk 1: Developers might incorrectly assume that configuring the `row`
type in the schema will directly map to `SeaTunnelRow` objects, leading to type
conversion errors
- Risk 2: Developers developing custom Connectors might misuse the type
system API
- Risk 3: Documentation readers might become confused about SeaTunnel's type
system
**Impact scope**:
- **Direct impact**: All users and developers consulting the Schema
documentation
- **Indirect impact**: May mislead developers developing custom Connectors
or type converters based on the documentation
- **Affected area**: Documentation readers, does not affect runtime code
**Severity**: MAJOR
**Improvement suggestion**:
The current PR has correctly fixed this issue, no further improvement is
needed.
**Rationale**:
1. The modification aligns with the actual design of the type system
2. Both Chinese and English documentation are modified synchronously to
maintain consistency
3. Consistent with usage in other documentation (such as S3File connector
documentation)
### Issue 2: Documentation table header may cause confusion
**Location**:
- `docs/en/introduction/concepts/schema-feature.md:79`
- `docs/zh/introduction/concepts/schema-feature.md:79`
**Problem description**:
The second column header in the documentation table is "Value type in Java",
which may lead to reader misunderstanding.
For the `row` type:
- **Value filled in table**: `SeaTunnelRowType` (type definition class)
- **Actual runtime value type**: `SeaTunnelRow` (value class)
The header "Value type in Java" may lead readers to understand it as "Java
type of runtime values", but in fact this column should be understood as "Java
type representation corresponding to SeaTunnel type".
**Potential risks**:
- Risk 1: Readers may confuse type definitions and value classes
- Risk 2: For the `row` type, readers may not understand why the table fills
`SeaTunnelRowType` instead of `SeaTunnelRow`
**Impact scope**:
- **Direct impact**: Documentation readers
- **Indirect impact**: May lead to misunderstanding of the type system
- **Affected area**: Documentation readers
**Severity**: MINOR
**Improvement suggestion**:
Consider changing the table header to a clearer expression:
```markdown
| Data type | SeaTunnel DataType in Java | Description |
```
Or add an explanatory note:
```markdown
| Data type | Type definition in Java | Description |
```
**Rationale**:
This more accurately reflects that the types filled in the table are
SeaTunnel's type definition classes, not runtime value classes. However, this
is only an improvement suggestion, not a blocking issue.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
