morningman commented on code in PR #68874:
URL: https://github.com/apache/doris/pull/68874#discussion_r4238696002
##########
fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonWriteBinding.java:
##########
@@ -78,26 +82,66 @@ static FileStoreTable configureTableForWrite(FileStoreTable
table, boolean overw
return table.copy(Collections.singletonMap(dynamicOverwriteKey,
Boolean.FALSE.toString()));
}
- private static Map<String, String> resolveStaticPartition(FileStoreTable
table,
- Map<String, String> requested, Set<String> nullKeys) {
+ /**
+ * The static partition as Paimon's static overwrite parses it: SQL NULL
becomes the table's
+ * {@code partition.default-name}, and every other value is the one the
written rows carry, cast to the
+ * column type.
+ */
+ static Map<String, String> resolveStaticPartition(FileStoreTable table,
ConnectorWriteHandle writeHandle) {
Map<String, String> canonicalNames = new
TreeMap<>(String.CASE_INSENSITIVE_ORDER);
for (String partitionKey : table.partitionKeys()) {
canonicalNames.put(partitionKey, partitionKey);
}
String defaultPartitionName =
CoreOptions.fromMap(table.options()).partitionDefaultName();
+ Map<String, String> castValues =
writeHandle.getCastStaticPartitionSpec();
Map<String, String> result = new LinkedHashMap<>();
Review Comment:
Agreed, fixed in 454af099d67. Only a static overwrite reads the static
partition: the commit passes it to `withOverwrite`, while an INSERT writes each
row to the partition its own values name. `PaimonWriteBinding.create` now
resolves the static partition, and so asks fe-core for the cast values, only
for an overwrite; an INSERT is planned as it was before this PR. branch-4.x
casts for an INSERT as well, but nothing there depends on it.
On a local file-system Paimon catalog, default session:
| Statement | b9874e4334a | 454af099d67 |
|---|---|---|
| `INSERT INTO t PARTITION (dt = '2024-13-45') VALUES (1)`, `dt DATE` |
`date/datetime literal [2024-13-45T00:00:00] is invalid` | writes `dt = NULL`,
as master does |
| `INSERT INTO t PARTITION (p = 'bad') VALUES (1)`, `p INT` | `bad can't
cast to INT in strict mode.` | writes `p = NULL`, as master does |
| `INSERT INTO t PARTITION (p = X'4142') VALUES (1)`, `p VARBINARY(2)` |
`Cast from 4142 to VARBINARY(2) not supported` | passes planning; the commit
then fails the same way as `INSERT INTO t VALUES (1, X'4142')` |
| `INSERT OVERWRITE TABLE t PARTITION (dt = '2024-13-45') VALUES (2)` | `...
is invalid` | `... is invalid` (unchanged) |
The VARBINARY insert fails in both forms for a reason outside this PR: the
partition directory comes out as the byte array's `toString()`
(`p=[B@1a8d2074`), and the commit then reports the file the BE wrote there as
deleted.
`PaimonWriteBindingTest.insertLeavesTheStaticPartitionValuesAlone` builds an
INSERT binding whose handle fails if asked for the cast values; resolving the
static partition of an INSERT makes it fail. The PR description no longer lists
the INSERT rejection as a behavior change.
--
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]
