luoyuxia commented on code in PR #3509:
URL: https://github.com/apache/fluss/pull/3509#discussion_r3459346057
##########
fluss-client/src/main/java/org/apache/fluss/client/lookup/PrimaryKeyLookuper.java:
##########
@@ -81,7 +81,7 @@ public PrimaryKeyLookuper(
tableInfo.getTableConfig(),
tableInfo.isDefaultBucketKey());
this.bucketKeyEncoder =
- tableInfo.isDefaultBucketKey()
+ tableInfo.isDefaultBucketKey() && lakeFormat !=
DataLakeFormat.HUDI
Review Comment:
I think we should avoid putting the Hudi-specific `lakeFormat !=
DataLakeFormat.HUDI` check in the callers.
The reason for the special case makes sense: for Hudi, the primary key
encoder must be lossless (`CompactedKeyEncoder`), while the bucket key encoder
still needs to follow Hudi's hash-based bucket encoding. So even when
`isDefaultBucketKey` is true, the bucket encoder cannot safely reuse the
primary key encoder.
But this is really a `KeyEncoder` capability/strategy decision, not
something `PrimaryKeyLookuper` or `UpsertWriterImpl` should know about. Could
we move this into the `KeyEncoder` factory instead? For example:
```java
KeyEncoder.ofBucketKeyEncoder(
rowType,
bucketKeys,
tableConfig,
isDefaultBucketKey,
primaryKeyEncoder); and let KeyEncoder internally decide whether
the primary key encoder can be reused for bucket routing. That would keep the
Hudi exception in one place and avoid future callers accidentally missing the
same condition.
--
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]
