laskoviymishka commented on code in PR #16655:
URL: https://github.com/apache/iceberg/pull/16655#discussion_r4242885691
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/data/SinkWriter.java:
##########
@@ -39,12 +40,16 @@ public class SinkWriter {
private final IcebergWriterFactory writerFactory;
private final Map<String, RecordWriter> writers;
private final Map<TopicPartition, Offset> sourceOffsets;
+ // the route field is fixed per config, so split its dotted path once
instead of per record
Review Comment:
I'd drop this comment — it describes what the PR changed ("instead of per
record"), which stops making sense once the old per-record split is gone, and
the field name already says what it holds. If we want a comment here, the one
non-obvious bit is the null contract: `routeFieldPath == null` means route to
all tables.
##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/data/RecordUtils.java:
##########
@@ -47,9 +47,12 @@
class RecordUtils {
- @SuppressWarnings("unchecked")
static Object extractFromRecordValue(Object recordValue, String fieldName) {
- List<String> fields = Splitter.on('.').splitToList(fieldName);
+ return extractFromRecordValue(recordValue,
Splitter.on('.').splitToList(fieldName));
+ }
+
+ @SuppressWarnings("unchecked")
+ static Object extractFromRecordValue(Object recordValue, List<String>
fields) {
Review Comment:
After this change the only production caller goes through the `List<String>`
overload, so the `String` version is left serving just `TestRecordUtils`.
Either is fine, but I'd pick deliberately: drop the `String` overload and have
the tests split via `Splitter.on('.').splitToList(...)` so they exercise the
production signature, or keep it and treat it as the covered split-then-extract
convenience. It's package-private, so there's no API cost either way.
--
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]
