github-actions[bot] commented on code in PR #69083:
URL: https://github.com/apache/doris/pull/69083#discussion_r4274646965
##########
fe/be-java-extensions/paimon-scanner/src/main/java/org/apache/doris/paimon/PaimonWriteSchema.java:
##########
@@ -163,13 +214,135 @@ GenericRow tableRow(InternalRow columnValues) {
row.setRowKind(toRowKind(value));
continue;
}
+ // Only an inserted row is padded; its row kind, if any, is the
first input column. An UPDATE or
+ // DELETE row carries the key Doris read back from the table,
which has to stay byte-equal to the
+ // stored key for Paimon to match it, padded or not.
+ if (charPadders[i] != null && value != null && row.getRowKind() ==
RowKind.INSERT) {
+ value = charPadders[i].pad(value);
Review Comment:
[P1] Normalize CHAR keys before MERGE matching and cardinality checks
Padding here changes primary-key identity after
`ConnectorChangelogPlanBuilder.addCardinalityChecks` has validated it. With an
empty `(c CHAR(3), v INT)` primary-key target, MERGE source rows `(CAST('k' AS
CHAR(3)), 1)` and `(CAST('k ' AS CHAR(3)), 2)` using `ON t.c = s.c WHEN NOT
MATCHED THEN INSERT (c,v) VALUES (s.c,s.v)` follow this reduced plan:
```text
JNI sink: pad(c)
AssertTrue(count <= 1)
Window(count PARTITION BY c)
Project(op=INSERT, c=s.c, v=s.v)
Join(target, source, t.c=s.c)
```
Doris preserves these short CHAR values and compares their bytes, so both
window counts are 1. This branch then turns both keys into `'k '`, and
Paimon's deduplicate writer commits one surviving row instead of the required
duplicate-insert error. A source `'k'` can likewise overwrite an existing `'k
'` target through a NOT MATCHED-only MERGE. Normalize keys before
matching/checking while preserving matched legacy keys, or reject this CHAR-key
MERGE combination until it is supported; add both regressions.
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonWriteBinding.java:
##########
@@ -126,10 +153,13 @@ static Map<String, String>
resolveStaticPartition(FileStoreTable table, Connecto
}
String value = Objects.requireNonNull(castValues.get(key),
() -> "missing the cast value of static partition column "
+ key);
- if (table.rowType().getField(canonicalName).type().getTypeRoot()
- == DataTypeRoot.TIMESTAMP_WITH_LOCAL_TIME_ZONE) {
+ DataType type = table.rowType().getField(canonicalName).type();
+ if (type.getTypeRoot() ==
DataTypeRoot.TIMESTAMP_WITH_LOCAL_TIME_ZONE) {
value = toSdkLocalTime(canonicalName, value);
}
+ if (charPadding && type.getTypeRoot() == DataTypeRoot.CHAR) {
+ value = padChar(value, ((CharType) type).getLength());
Review Comment:
[P2] Mirror the rowkind.field exemption in static partition padding
The writer now leaves the column named by `rowkind.field` unpadded, but this
branch still pads it when it is a CHAR partition column. For a valid table `(id
INT, rk CHAR(3))` with primary key `(id,rk)`, partition key `rk`, `bucket=1`
and `rowkind.field=rk`, `INSERT OVERWRITE TABLE t PARTITION(rk='+I') SELECT 1`
puts `'+I '` in the FE overwrite filter while the BE writes partition `'+I'`.
Paimon rejects that file as outside the specified partition; a SELECT that
returns no rows at runtime instead succeeds without clearing the existing
`'+I'` partition. Apply the writer's control-field exemption here too, and
cover nonempty and empty static overwrites. This is distinct from the earlier
token-padding finding: its BE fix is present, but the FE overwrite filter now
disagrees with it.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
