Kafka Connect: Split route field path once instead of per record

[wombatu-kun]

For static routing with iceberg.tables.route-field and for dynamic routing, SinkWriter re-split the dotted route field path with Splitter.on('.').splitToList on every record, although the route field is fixed for the writer's lifetime. This small allocation cleanup splits the path once in the SinkWriter constructor and passes the pre-split List<String> to RecordUtils.extractFromRecordValue. The package-private String overload of that method is removed, so TestRecordUtils now exercises the production signature. Behavior is unchanged; a non-JMH microbenchmark put the saving at roughly 50-130 ns per routed record, which is small next to record conversion and writing.

Tests: the new TestSinkWriter.dynamicRouteNestedField routes on a two-segment path (route.table) and fails if the path is not split; the existing TestSinkWriter and TestRecordUtils cases cover both routing modes.

---

AI Disclosure

• Model: Claude Opus 4.8, Claude Opus 5.5
• Platform/Tool: Claude Code
• Human Oversight: fully reviewed
• Prompt Summary: Split the Kafka Connect route field path once at construction instead of per record, then address review feedback.

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

[wombatu-kun]

no stale

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

[wombatu-kun]

@laskoviymishka you merged #16657 and #16606 in this same connect/data package - would you have a moment for this one? Same shape as #16657: per-record work hoisted out of the hot path, 2 files, CI green.

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

[laskoviymishka]

This is a clean win — splitting the route path once in the constructor instead of per record is the right call, and reading tablesRouteField() once is safe since the config is immutable for the writer's lifetime. Behavior looks identical across the null / static / dynamic paths and the existing tests cover it; the one gap is that ROUTE_FIELD is single-segment, so a SinkWriter test with a nested path like a.b would pin the pre-split path end to end — optional.

I'd also reframe the description from a perf fix to a small allocation cleanup: the saving is ~50–130ns against much heavier per-record conversion and write work, and there's no JMH behind the numbers. Nothing here blocks merge.

[laskoviymishka]

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.

[wombatu-kun]

Dropped in 23e1b00, without a replacement: the null contract is already stated where the field is read, by the // route to all tables branch on the static path and the checkNotNull message on the dynamic one.

[laskoviymishka]

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.

[wombatu-kun]

Dropped the String overload in 23e1b00, and TestRecordUtils now splits with Splitter.on('.').splitToList(...), so it exercises the production signature. Since the split now happens in the SinkWriter constructor, I also added TestSinkWriter.dynamicRouteNestedField, which routes on a nested route.table field and fails if the constructor stops splitting.

[wombatu-kun]

@laskoviymishka both of your comments are addressed in 23e1b00 (replies inline), and CI is green. Could you merge it when you have a moment? Thanks!

[laskoviymishka]

Thanks! This is good to merge now