DanielCarter-stack commented on PR #10472:
URL: https://github.com/apache/seatunnel/pull/10472#issuecomment-3870272646
<!-- code-pr-reviewer -->
<!-- cpr:pr_reply_v2_parts {"group": "apache/seatunnel#10472", "part": 1,
"total": 1} -->
### Issue 2: CHAR type length not validated, may lead to inconsistent
behavior
**Location**: `SapHanaTypeConverter.java:199-212`
**Related context**:
- Constant definition: `SapHanaTypeConverter.java:100` (MAX_NVARCHAR_LENGTH)
- Caller: `SapHanaTypeMapper.java:38-58` (extracted from JDBC
ResultSetMetaData)
**Issue description**:
The current implementation directly uses the length value returned by the
JDBC Driver without validating its reasonableness:
```java
case HANA_VARCHAR:
case HANA_CHAR:
case HANA_ALPHANUM:
...
builder.dataType(BasicType.STRING_TYPE);
if (typeDefine.getLength() == null || typeDefine.getLength() == 0) {
builder.columnLength(MAX_LOB_LENGTH);
} else {
builder.columnLength(typeDefine.getLength()); // ← Length range not
validated
}
break;
```
**Root cause**:
1. SAP HANA's CHAR type has a maximum length limit (typically 2000 bytes)
2. The JDBC Driver may return unreasonable values (such as 0, negative
numbers, extremely large values)
3. Different versions of SAP HANA may have different limits
**Potential risks**:
**Risk 1: Improper handling of oversized values**
- **Scenario**: Some JDBC Drivers return abnormal precision values (such as
Integer.MAX_VALUE)
- **Actual behavior**: `columnLength` is set to an extremely large value
- **Impact**:
- SeaTunnel may internally allocate buffers based on this value (if the
length is 2^31, it will cause OOM)
- When generating DDL, `CHAR(2147483647)` may be generated, causing
database errors
**Risk 2: Length is 0 or negative**
- **Scenario**: CHAR type has no length definition (rare but possible)
- **Current behavior**: When `length == 0`, it is set to `MAX_LOB_LENGTH`
- **Problem**: Inconsistent logic (`length == null` is also `MAX_LOB_LENGTH`)
- **Suggestion**: Handle uniformly or add warning logs
**Risk 3: Cross-version compatibility**
- **Scenario**: Different versions of SAP HANA have different limits on the
maximum length of CHAR
- **Impact**: The code has no version awareness, which may lead to abnormal
behavior in certain versions
**Scope of impact**:
**Direct impact**:
- `SapHanaTypeConverter.convert()` method
- `SapHanaCreateTableSqlBuilder.buildColumnSql()` method (uses reconvert
result)
**Indirect impact**:
- Memory management (if the length is abnormally large)
- DDL generation (table creation failure)
**Impact surface**: Single Connector (SAP HANA)
**Severity**: Low (rarely encountered in actual scenarios)
**Improvement suggestions**:
```java
// Add CHAR type length constant
public static final long MAX_CHAR_LENGTH = 2000; // SAP HANA CHAR maximum
length
// Add validation in convert() method
case HANA_VARCHAR:
case HANA_CHAR:
case HANA_ALPHANUM:
case HANA_CLOB:
case HANA_NCLOB:
case HANA_TEXT:
case HANA_BINTEXT:
builder.dataType(BasicType.STRING_TYPE);
if (typeDefine.getLength() == null || typeDefine.getLength() == 0) {
builder.columnLength(MAX_LOB_LENGTH);
} else {
// Validate length range
long length = typeDefine.getLength();
if (length < 0) {
log.warn("Invalid negative length {} for column {}, using
default",
length, typeDefine.getName());
builder.columnLength(MAX_LOB_LENGTH);
} else if (length > MAX_CHAR_LENGTH) {
log.warn("CHAR length {} exceeds maximum {}, truncating to {}",
length, MAX_CHAR_LENGTH, MAX_CHAR_LENGTH);
builder.columnLength(MAX_CHAR_LENGTH);
} else {
builder.columnLength(length);
}
}
break;
// Add similar validation in reconvert() method
case STRING:
if (column.getColumnLength() == null
|| column.getColumnLength() <= 0) {
// Unspecified or invalid length
builder.columnType(HANA_NVARCHAR);
builder.dataType(HANA_NVARCHAR);
builder.length(MAX_NVARCHAR_LENGTH);
} else if (column.getColumnLength() <= MAX_NVARCHAR_LENGTH) {
// Short string
builder.columnType(HANA_NVARCHAR);
builder.dataType(HANA_NVARCHAR);
builder.length(column.getColumnLength());
} else {
// Long string
builder.columnType(HANA_CLOB);
builder.dataType(HANA_CLOB);
}
break;
```
**Rationale**:
- Defensive programming: even if the JDBC Driver returns abnormal values,
they can be handled correctly
- Provide clear error prompts (through logs)
- Complies with actual SAP HANA limitations
- Consistent with the handling of other types (such as DECIMAL) (already has
precision/scale validation)
---
### Issue 3: Insufficient unit test coverage, missing edge case tests
**Location**: `SapHanaTypeConverterTest.java:272-285`
**Related context**:
- Test method: `testConvertChar()` (line 215)
- Integration test: `JdbcHanaIT.java:74-75` (CHAR field definition)
**Issue description**:
The current unit test only covers the single case of CHAR(10):
```java
@Test
public void testConvertChar() {
// ... other type tests ...
// Only test CHAR(10)
typeDefine = BasicTypeDefine.builder()
.name("test")
.columnType("CHAR")
.dataType("CHAR")
.length(10L)
.build();
column = SapHanaTypeConverter.INSTANCE.convert(typeDefine);
Assertions.assertEquals(typeDefine.getName(), column.getName());
Assertions.assertEquals(BasicType.STRING_TYPE, column.getDataType());
Assertions.assertEquals(typeDefine.getLength(),
column.getColumnLength());
Assertions.assertEquals(typeDefine.getColumnType(),
column.getSourceType());
}
```
**Root cause**:
Test cases do not cover various edge scenarios of the CHAR type.
**Potential risks**:
**Risk 1: Default length behavior not tested**
- **Scenario**: SAP HANA allows `CHAR` (without length), defaulting to 1
- **Current**: `typeDefine.setLength(null)` or `length=1L` not tested
- **Impact**: If the JDBC Driver's handling of lengthless CHAR is
inconsistent, bugs may be introduced
**Risk 2: Maximum length boundary not tested**
- **Scenario**: `CHAR(2000)` (SAP HANA CHAR maximum length)
- **Current**: Boundary values not tested
- **Impact**: Boundary conditions may trigger unexpected truncation or
overflow
**Risk 3: Handling of length 0 not tested**
- **Scenario**: Some JDBC Drivers may return `length=0` to indicate
"unspecified"
- **Current**: In code, `length == 0` is treated as `MAX_LOB_LENGTH`, but
not tested
- **Impact**: Behavior may not meet expectations
**Risk 4: Space padding semantics not tested**
- **Scenario**: CHAR(10) storing "abc" should be padded to "abc "
- **Current**: Integration test uses "char_value_10" (exactly 10 characters,
no padding)
- **Impact**: Cannot verify whether space padding is correctly preserved
**Scope of impact**:
**Direct impact**:
- `SapHanaTypeConverterTest.java`
**Indirect impact**:
- Code quality (reduced test coverage)
- Regression risk (refactoring may break CHAR support)
**Impact surface**: Single test class
**Severity**: Medium
**Improvement suggestions**:
```java
@Test
public void testConvertChar() {
// ... existing tests ...
// Test CHAR(10) - already exists
typeDefine = BasicTypeDefine.builder()
.name("test")
.columnType("CHAR")
.dataType("CHAR")
.length(10L)
.build();
column = SapHanaTypeConverter.INSTANCE.convert(typeDefine);
Assertions.assertEquals(BasicType.STRING_TYPE, column.getDataType());
Assertions.assertEquals(10L, column.getColumnLength());
// New: Test default length CHAR (length=1)
typeDefine = BasicTypeDefine.builder()
.name("test_default")
.columnType("CHAR")
.dataType("CHAR")
.length(1L)
.build();
column = SapHanaTypeConverter.INSTANCE.convert(typeDefine);
Assertions.assertEquals(BasicType.STRING_TYPE, column.getDataType());
Assertions.assertEquals(1L, column.getColumnLength());
// New: Test CHAR without length (length=null)
typeDefine = BasicTypeDefine.builder()
.name("test_no_length")
.columnType("CHAR")
.dataType("CHAR")
.length(null)
.build();
column = SapHanaTypeConverter.INSTANCE.convert(typeDefine);
Assertions.assertEquals(BasicType.STRING_TYPE, column.getDataType());
Assertions.assertEquals(Integer.MAX_VALUE, column.getColumnLength());
// New: Test maximum length CHAR(2000)
typeDefine = BasicTypeDefine.builder()
.name("test_max_length")
.columnType("CHAR")
.dataType("CHAR")
.length(2000L)
.build();
column = SapHanaTypeConverter.INSTANCE.convert(typeDefine);
Assertions.assertEquals(BasicType.STRING_TYPE, column.getDataType());
Assertions.assertEquals(2000L, column.getColumnLength());
// New: Test CHAR with length 0 (should use MAX_LOB_LENGTH)
typeDefine = BasicTypeDefine.builder()
.name("test_zero_length")
.columnType("CHAR")
.dataType("CHAR")
.length(0L)
.build();
column = SapHanaTypeConverter.INSTANCE.convert(typeDefine);
Assertions.assertEquals(BasicType.STRING_TYPE, column.getDataType());
Assertions.assertEquals(Integer.MAX_VALUE, column.getColumnLength());
}
// New: Test CHAR type preservation in reconvert method (if implementing fix
for issue 1)
@Test
public void testReconvertStringFromChar() {
Column column = PhysicalColumn.builder()
.name("test")
.dataType(BasicType.STRING_TYPE)
.columnLength(100L)
.sourceType("CHAR(100)") // Key: preserve original type
.build();
BasicTypeDefine typeDefine =
SapHanaTypeConverter.INSTANCE.reconvert(column);
// Should preserve CHAR type instead of converting to NVARCHAR
Assertions.assertEquals("CHAR(100)", typeDefine.getColumnType());
Assertions.assertEquals("CHAR", typeDefine.getDataType());
Assertions.assertEquals(100L, typeDefine.getLength());
}
```
**Integration test improvements** (`JdbcHanaIT.java`):
```java
// Add test data in initTestData() method
Pair<String[], List<SeaTunnelRow>> initTestData() {
// ...
List<SeaTunnelRow> rows = new ArrayList<>();
for (int i = 0; i < 100; i++) {
// Add space padding test
SeaTunnelRow row = new SeaTunnelRow(new Object[] {
// ... other fields ...
"short", // CHAR_VALUE_10 (test: "short" < 10,
should be padded to "short ")
"a", // CHAR_VALUE (test: single character)
// ...
});
rows.add(row);
}
return Pair.of(fieldNames, rows);
}
// Add new test method to verify space padding
@Test
public void testCharSpacePadding() throws Exception {
// Insert short string
String insertSql = "INSERT INTO TEST.ALLDATATYPES (INT_VALUE,
CHAR_VALUE_10) VALUES (1, 'abc')";
try (Statement stmt = connection.createStatement()) {
stmt.execute(insertSql);
}
// Read and verify
String selectSql = "SELECT CHAR_VALUE_10 FROM TEST.ALLDATATYPES WHERE
INT_VALUE = 1";
try (Statement stmt = connection.createStatement();
ResultSet rs = stmt.executeQuery(selectSql)) {
Assertions.assertTrue(rs.next());
String value = rs.getString("CHAR_VALUE_10");
// CHAR(10) stores "abc" should be padded to "abc " (7 spaces)
Assertions.assertEquals("abc ", value);
Assertions.assertEquals(10, value.length());
}
}
```
**Rationale**:
- Improve test coverage and cover edge cases
- Verify CHAR's special semantics (space padding)
- Prevent future refactoring from breaking CHAR support
- Complies with Apache project testing standards (high coverage requirements)
---
### Issue 4: Missing NCHAR type support, may cause internationalization
scenarios to fail
**Location**: `SapHanaTypeConverter.java:54-60`
**Related context**:
- CHAR type constant: `HANA_CHAR` (line 57)
- SAP HANA documentation: supports NCHAR (National Character, supports
Unicode)
**Issue description**:
SAP HANA supports the NCHAR type (National Character CHAR, used for storing
Unicode characters), but the current implementation does not include it:
```java
// -------------------------string----------------------------
public static final String HANA_VARCHAR = "VARCHAR";
public static final String HANA_NVARCHAR = "NVARCHAR"; // ← Supports
NVARCHAR
public static final String HANA_CHAR = "CHAR"; // ← Supports CHAR
// Missing: public static final String HANA_NCHAR = "NCHAR"; // ← NCHAR not
supported
public static final String HANA_ALPHANUM = "ALPHANUM";
public static final String HANA_SHORTTEXT = "SHORTTEXT";
```
**Root cause**:
- The PR only focuses on the CHAR type and does not consider NCHAR
- The relationship between NCHAR and CHAR is similar to that between
NVARCHAR and VARCHAR
- NCHAR is commonly used in internationalization applications to store
multilingual characters
**Potential risks**:
**Risk 1: Internationalization application data reading failure**
- **Scenario**: Some SAP systems use NCHAR to store Chinese, Japanese, and
other multilingual fields
- **Current behavior**: NCHAR type triggers `Unsupported type: NCHAR` error
- **Impact**: Unable to synchronize tables containing NCHAR fields
**Risk 2: Character set inconsistency**
- **Scenario**: The user's reason for using NCHAR is the need for Unicode
support
- **Current**: When converting to STRING_TYPE, character set information may
be lost
- **Impact**: Some special characters (such as emoji) may display abnormally
**Risk 3: Inconsistent behavior with NVARCHAR**
- **Scenario**: NVARCHAR is properly supported (lines 213-217), but NCHAR is
not
- **Problem**: Asymmetric logic (NVARCHAR has special handling, NCHAR does
not)
- **Impact**: User confusion
**Scope of impact**:
**Direct impact**:
- `SapHanaTypeConverter.convert()` method
- SAP HANA users in internationalization scenarios
**Indirect impact**:
- Multilingual data synchronization tasks
- SAP ERP/CRM system integration (SAP systems widely use NCHAR)
**Impact surface**: Single Connector (SAP HANA)
**Severity**: Medium (significant impact on internationalization
applications)
**Improvement suggestions**:
```java
// Add NCHAR constant
// -------------------------string----------------------------
public static final String HANA_VARCHAR = "VARCHAR";
public static final String HANA_NVARCHAR = "NVARCHAR";
public static final String HANA_CHAR = "CHAR";
public static final String HANA_NCHAR = "NCHAR"; // ← New
public static final String HANA_ALPHANUM = "ALPHANUM";
public static final String HANA_SHORTTEXT = "SHORTTEXT";
// Update shouldAppendLength list
public static final List<String> shouldAppendLength =
Arrays.asList(
HANA_BINARY,
HANA_VARBINARY,
HANA_VARCHAR,
HANA_NVARCHAR,
HANA_CHAR,
HANA_NCHAR, // ← New
HANA_ALPHANUM,
HANA_SHORTTEXT);
// Add handling in convert() method (similar to NVARCHAR)
case HANA_VARCHAR:
case HANA_CHAR:
case HANA_ALPHANUM:
case HANA_CLOB:
case HANA_NCLOB:
case HANA_TEXT:
case HANA_BINTEXT:
builder.dataType(BasicType.STRING_TYPE);
if (typeDefine.getLength() == null || typeDefine.getLength() == 0) {
builder.columnLength(MAX_LOB_LENGTH);
} else {
builder.columnLength(typeDefine.getLength());
}
break;
case HANA_NCHAR: // ← New (similar to NVARCHAR handling)
case HANA_NVARCHAR:
case HANA_SHORTTEXT:
builder.dataType(BasicType.STRING_TYPE);
builder.columnLength(TypeDefineUtils.charTo4ByteLength(typeDefine.getLength()));
break;
// Add handling in reconvert() method
case STRING:
// If sourceType is NCHAR, preserve it first
if (StringUtils.isNotBlank(column.getSourceType())
&& column.getSourceType().startsWith("NCHAR")) {
String originalType = column.getSourceType();
builder.columnType(originalType);
builder.dataType("NCHAR");
if (column.getColumnLength() != null) {
builder.length(column.getColumnLength() / 4); // 4 bytes to
character count
}
} else if (column.getColumnLength() == null
|| column.getColumnLength() <= MAX_NVARCHAR_LENGTH) {
builder.columnType(HANA_NVARCHAR);
builder.dataType(HANA_NVARCHAR);
...
} else {
builder.columnType(HANA_CLOB);
builder.dataType(HANA_CLOB);
}
break;
```
**Unit test**:
```java
@Test
public void testConvertNchar() {
// Test NCHAR(10)
BasicTypeDefine typeDefine = BasicTypeDefine.builder()
.name("test")
.columnType("NCHAR")
.dataType("NCHAR")
.length(10L)
.build();
Column column = SapHanaTypeConverter.INSTANCE.convert(typeDefine);
Assertions.assertEquals(BasicType.STRING_TYPE, column.getDataType());
Assertions.assertEquals(40, column.getColumnLength()); // 10 * 4
(UTF-32)
Assertions.assertEquals("NCHAR", column.getSourceType());
}
```
**Integration test**:
```java
// Add NCHAR field in JdbcHanaIT.java
private static final String CREATE_SOURCE_SQL =
"CREATE TABLE %s (\n"
+ "INT_VALUE INT PRIMARY KEY, \n"
+ "CHAR_VALUE CHAR(10), \n"
+ "NCHAR_VALUE NCHAR(10), \n" // ← New
+ "NVARCHAR_VALUE NVARCHAR(10), \n"
+ "...\n";
```
**Rationale**:
- Maintain completeness of type support (CHAR/NCHAR, VARCHAR/NVARCHAR)
- Support internationalization scenarios (SAP systems widely use NCHAR)
- Keep consistent with NVARCHAR handling logic
- Simple implementation (can reuse NVARCHAR code)
**Priority**: Medium (recommended to supplement in the next PR)
---
### Issue 5: Documentation not updated, users may not know CHAR type is
supported
**Location**: Project documentation (CHANGELOG, user documentation,
connector documentation)
**Related context**:
- PR description: "To fix saphana not support char field type #10471"
- PR check list: "If necessary, please update the documentation"
**Issue description**:
The current PR has not updated any documentation, leading to:
**Problem 1: CHANGELOG not updated**
- **Impact**: Users cannot know from the version release log that the CHAR
type is supported
- **Scenario**: After users upgrade the version, they may still think CHAR
is not supported, thus avoiding use or seeking workarounds
**Problem 2: Connector documentation not updated**
- **Impact**: New users do not know that the SAP HANA Connector supports the
CHAR type
- **Scenario**: When users view the documentation, they may mistakenly think
CHAR is not supported, thus choosing other types or giving up use
**Problem 3: Migration guide not updated**
- **Impact**: When users migrate from other tools to SeaTunnel, they are
unclear about the compatibility of the CHAR type
- **Scenario**: When migrating from SAP Data Services or other ETL tools,
users worry that CHAR fields cannot be synchronized
**Scope of impact**:
**Direct impact**:
- Project documentation (CHANGELOG.md, connector documentation)
**Indirect impact**:
- User experience (users may not know about this feature)
- Community feedback (may receive duplicate issues/feature requests)
**Impact surface**: Entire project (documentation)
**Severity**: Medium (does not affect functionality, but affects usability)
**Improvement suggestions**:
**1. Update CHANGELOG.md** (required)
Add to `docs/en/CHANGELOG.md` or the project's changelog file:
```markdown
## [Unreleased]
### SeaTunnel Connector
#### JDBC
* [FEATURE] Add CHAR type support for SAP HANA connector (#10472)
```
**2. Update connector documentation** (recommended)
Add to `docs/en/connector-v2/source/Jdbc.md` or SAP HANA-specific
documentation:
```markdown
## SAP HANA
### Supported Data Types
| SAP HANA Type | SeaTunnel Type | Description |
|---------------|----------------|-------------|
| ... | ... | ... |
| CHAR | STRING | Fixed-length character string |
| NCHAR | STRING | Fixed-length national character string (Unicode) |
| VARCHAR | STRING | Variable-length character string |
| NVARCHAR | STRING | Variable-length national character string |
| ... | ... | ... |
**Notes**:
- CHAR type is now supported in version 2.x.x+
- When reading CHAR columns, trailing spaces are preserved (as per SQL
standard)
- When creating tables with SaveMode, CHAR columns may be mapped to NVARCHAR
(see reconvert logic)
```
**3. Update migration guide** (optional)
If the project has a migration guide (such as migrating from other ETL
tools), add:
```markdown
## Migrating from SAP Data Services
### Type Mapping
| SAP Data Services | SeaTunnel (SAP HANA) | Notes |
|-------------------|----------------------|-------|
| CHAR | CHAR | Supported since v2.x.x |
| ...
```
**4. Add version compatibility description** (optional)
Explain in `incompatible-changes.md` (if there are incompatible changes):
```markdown
## Version 2.x.x
### SAP HANA Connector
- CHAR type now supported. No breaking changes.
```
**Rationale**:
- Documentation is an important part of user experience
- Apache projects have clear requirements for documentation (mentioned in
the PR template)
- Users learn about features through documentation; missing documentation
makes features "invisible"
- Reduce duplicate feature requests (issue #10471 has been resolved, but
other users may raise the same question)
**Priority**: Medium (recommended to supplement documentation updates after
merging the PR)
---
### Issue 6: CHAR values in E2E tests do not test space padding semantics
**Location**: `JdbcHanaIT.java:183-184`
**Related context**:
- Test data definition: `initTestData()` method (lines 142-209)
- CHAR field: `CHAR_VALUE_10` (line 75)
**Issue description**:
The current E2E test uses CHAR test values that exactly fill the length,
without verifying space padding semantics:
```java
// Current test data
"c", // CHAR_VALUE (default length 1, exactly filled)
"char_value_10", // CHAR_VALUE_10 (length exactly 10, no space padding)
```
**Root cause**:
- SAP HANA's CHAR type is a fixed-length string, and short strings are
right-padded with spaces
- For example: CHAR(10) storing "abc" → actually stored as "abc " (7
spaces)
- The current test case does not verify this key semantics
**Potential risks**:
**Risk 1: Space padding loss not detected**
- **Scenario**: If the code incorrectly trims spaces (such as automatic
processing by some frameworks)
- **Current test**: Cannot detect (because the test value itself has no
spaces)
- **Impact**: Data inconsistency (source table "abc " → target table
"abc")
**Risk 2: Compatibility issues with other systems**
- **Scenario**: Synchronizing from SAP HANA to other databases (such as
Oracle, MySQL)
- **Oracle**: CHAR also pads spaces, consistent with HANA ✅
- **MySQL**: CHAR also pads spaces, but automatically trims on retrieval ⚠️
- **Impact**: When synchronizing across databases, inconsistent space
handling may cause tests to pass but production to fail
**Risk 3: User code depends on space padding**
- **Scenario**: Some legacy systems (such as COBOL interfaces) depend on the
fixed length of CHAR
- **Example**: CHAR(5) storing postal code "12345" and "123 " (2 spaces at
the end) have different meanings
- **Current test**: Cannot verify whether this semantics is preserved
**Scope of impact**:
**Direct impact**:
- Effectiveness of `JdbcHanaIT.java` test
- End-to-end verification of CHAR type
**Indirect impact**:
- Data consistency in production environment
- Integration with other systems
**Impact surface**: SAP HANA Connector's E2E tests
**Severity**: Medium
**Improvement suggestions**:
```java
@Override
Pair<String[], List<SeaTunnelRow>> initTestData() {
String[] fieldNames = new String[] {
"INT_VALUE",
"VARCHAR_VALUE",
// ...
"CHAR_VALUE", // Default length CHAR
"CHAR_VALUE_10", // CHAR(10)
"CHAR_VALUE_PADDING_TEST", // ← New: for testing space padding
// ...
};
List<SeaTunnelRow> rows = new ArrayList<>();
for (int i = 0; i < 100; i++) {
SeaTunnelRow row = new SeaTunnelRow(new Object[] {
i,
"v",
// ...
"c", // CHAR_VALUE (length 1, exactly filled)
"char10", // CHAR_VALUE_10 (length 6 < 10, should be
padded to "char10 ")
"abc", // CHAR_VALUE_PADDING_TEST (length 3 < 10,
should be padded to "abc ")
// ...
});
rows.add(row);
}
return Pair.of(fieldNames, rows);
}
// Add new fields in CREATE_SOURCE_SQL
private static final String CREATE_SOURCE_SQL =
"CREATE TABLE %s (\n"
+ "INT_VALUE INT PRIMARY KEY, \n"
+ "...\n"
+ "CHAR_VALUE CHAR, \n"
+ "CHAR_VALUE_10 CHAR(10), \n"
+ "CHAR_VALUE_PADDING_TEST CHAR(10), \n" // ← New
+ "...\n";
// Add new test method to verify space padding
@SneakyThrows
@Test
public void testCharSpacePadding() {
// Insert short string
String insertSql = "INSERT INTO TEST.ALLDATATYPES (INT_VALUE,
CHAR_VALUE_10, CHAR_VALUE_PADDING_TEST) "
+ "VALUES (999, 'short', 'abc')";
try (Statement stmt = connection.createStatement()) {
stmt.execute(insertSql);
}
// Verify through JDBC read (should have padding)
String selectSql = "SELECT CHAR_VALUE_10, CHAR_VALUE_PADDING_TEST FROM
TEST.ALLDATATYPES WHERE INT_VALUE = 999";
try (Statement stmt = connection.createStatement();
ResultSet rs = stmt.executeQuery(selectSql)) {
Assertions.assertTrue(rs.next());
String char10 = rs.getString("CHAR_VALUE_10");
Assertions.assertEquals("short ", char10); // "short" (5) + 5
spaces = 10
Assertions.assertEquals(10, char10.length());
String paddingTest = rs.getString("CHAR_VALUE_PADDING_TEST");
Assertions.assertEquals("abc ", paddingTest); // "abc" (3) +
7 spaces = 10
Assertions.assertEquals(10, paddingTest.length());
}
// Verify through SeaTunnel sync (space padding should be preserved)
// ... run E2E sync job ...
// Verify target table data
// ... query target table, assert space padding is preserved ...
}
```
**Additional test scenarios**:
```java
// Test CHAR space trim behavior (if trim option is configured)
@Test
public void testCharWithTrimOption() {
// Some users may want automatic space trimming
// If trim configuration is added in the future, this test verifies its
behavior
// Insert value with spaces
String insertSql = "INSERT INTO TEST.ALLDATATYPES (INT_VALUE,
CHAR_VALUE_10) VALUES (998, 'test ')";
// ... execute ...
// If trim is enabled, should get "test"
// If trim is disabled, should get "test " (preserve spaces)
// Verify behavior matches configuration
}
```
**Rationale**:
- The core characteristic of CHAR is fixed length and space padding, which
is the key difference from VARCHAR
- Tests should verify the key semantics of the type, not just "it works"
- Prevent future refactoring from accidentally breaking space padding
behavior
- Provide clear CHAR semantics documentation (through test cases)
**Priority**: Medium (recommended to supplement in a subsequent PR)
--
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]
