morningman commented on code in PR #68874:
URL: https://github.com/apache/doris/pull/68874#discussion_r4239737376
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/PluginDrivenInsertCommandContext.java:
##########
@@ -87,13 +94,63 @@ public void
setStaticPartitionSpecFromExpressions(Map<String, Expression> partit
// Binary partition keys must bypass character decoding,
including empty and non-UTF-8 bytes.
spec.put(entry.getKey(), literal instanceof VarBinaryLiteral
? "0x" + literal.toString() :
literal.getStringValue());
+ literals.put(entry.getKey(), literal);
if (entry.getValue() instanceof NullLiteral) {
nullKeys.add(entry.getKey());
}
}
}
this.staticPartitionSpec = spec;
this.staticPartitionNullKeys = nullKeys;
+ this.staticPartitionLiterals = literals;
+ }
+
+ public void setBoundTargetSchema(List<Column> boundTargetSchema) {
+ this.boundTargetSchema = boundTargetSchema;
+ }
+
+ /**
+ * Casts the static partition values to the types of their columns in the
bound target schema; see
+ * {@code ConnectorWriteHandle#getCastStaticPartitionSpec}. Each value is
the one BindSink materializes into
+ * the rows, after constant folding, but a value that does not fit its
type fails here instead of becoming
+ * NULL.
+ */
+ public Map<String, String> castStaticPartitionSpec() {
+ Preconditions.checkState(staticPartitionLiterals.isEmpty() ||
!boundTargetSchema.isEmpty(),
+ "static partition values are cast before the sink is bound to
a target schema");
+ Map<String, String> spec = new LinkedHashMap<>();
+ for (Map.Entry<String, Literal> entry :
staticPartitionLiterals.entrySet()) {
+ if (staticPartitionNullKeys.contains(entry.getKey())) {
+ continue;
+ }
+ Literal value = entry.getValue();
+ Optional<Column> column = boundTargetSchema.stream()
+ .filter(candidate ->
candidate.getName().equalsIgnoreCase(entry.getKey()))
+ .findFirst();
+ if (column.isPresent()) {
+ value = writtenValue(value,
DataType.fromCatalogType(column.get().getType()));
+ }
+ spec.put(entry.getKey(), value.getStringValue());
Review Comment:
Agreed, fixed in cc5818eadaa, by rejecting the value rather than keeping its
bytes: Paimon's string form cannot name every binary partition.
Paimon's static overwrite turns each spec value into the partition key with
`TypeUtils.castFromString`, which for BINARY / VARBINARY takes the UTF-8 bytes
of the string: `"0x4142"` becomes the six bytes `30 78 34 31 34 32`, `"AB"`
becomes `41 42`, and BINARY(n) is not padded. So neither spelling Doris has
names the partition: the cast values gave `[B@...`, and the spec as written
gives `0x4142`, which is what master passes, so master names the wrong
partition here too. Only a value whose bytes are valid UTF-8 could be named at
all, so instead of a partial mapping:
- `PaimonWriteBinding` rejects a static overwrite that gives a binary
partition column a value other than NULL, before the values are cast (a
`VARBINARY(2)` column no longer fails first with `Cast from 4142 to
VARBINARY(2) not supported`). SQL NULL still names the `partition.default-name`
partition.
- The cast values spell a binary value as the spec as written does, `0x` and
its bytes in hex, so they never carry an array identity.
On a local file-system Paimon catalog, with a `BYTES` partition column and
partitions `AB` and `CD` written through the Paimon API, and the table read
back through the Paimon API (the table uses `partition.legacy-name=false`; with
the default legacy names Paimon names the directory after `byte[].toString()`
and cannot read the table back itself):
| Statement | 454af099d67 | cc5818eadaa |
|---|---|---|
| `INSERT OVERWRITE t PARTITION (p = X'4142') SELECT 10 LIMIT 0` |
`COMMITTED` (an OVERWRITE snapshot); partition `AB` still holds its row |
fails: `Static overwrite of binary partition column 'p' is not supported`; no
snapshot |
With cc5818eadaa, `INSERT OVERWRITE t PARTITION (p = X'4142') VALUES (10)`
fails the same way, and `INSERT OVERWRITE t PARTITION (p = NULL) SELECT 10
LIMIT 0` commits and leaves `AB` and `CD` as they were.
Tests: `PaimonWriteBindingTest.staticOverwriteRejectsABinaryPartitionValue`
(BYTES, VARBINARY(2), BINARY(2); the handle fails if asked for the cast values,
and the case pins Paimon reading `"0x4142"` as its six UTF-8 bytes), the NULL
case now also on a BYTES column, and
`PluginDrivenTableSinkBindingTest.staticPartitionBinaryValueKeepsItsBytesWhenCast`.
Dropping the check, or going back to `getStringValue()` for a cast binary
value, makes exactly the matching new case fail.
--
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]
