[ahmedabu98]

Iceberg reads build every Row with Row.Builder.build(). Its per-field checks are rather expensive, costing more than the rest of the conversion put together.

This switches to attachValues, which is how RowCoder builds rows when it decodes. Two values that build() used to fix up are now converted directly: DATETIME to a joda Instant, and binary inside lists and maps to byte[].

Local JMH, M4 Pro, JDK 21	Before	After
Convert one flat record (12 columns)	2,555 ns, 14 KB allocated	130 ns, 136 B
Convert one nested record	3,866 ns, 19.5 KB	481 ns, 1.2 KB
Read 200k flat rows from local Parquet	758 ms	178 ms
Read 200k nested rows from local Parquet	1,055 ms	360 ms

[github-actions]

📋 Review Summary

This PR optimizes Beam Row construction in IcebergUtils.icebergRecordToBeamRow and CdcOutputUtils.outputRow by replacing addValues(...) with attachValues(...) to avoid redundant validation and copying. However, attachValues(...) skips RowUtils.verify type conversion and nullability checks, which introduces critical runtime bugs in IcebergUtils.icebergRecordToBeamRow for non-nullable fields with null values, ITERABLE fields, and DATETIME fields.

🔍 General Feedback

• Row.Builder.attachValues vs. addValues invariants: RowUtils.verify (called by addValues/build, but bypassed by attachValues) does more than just validation: it also normalizes values to the internal Java representations expected by Row getters and RowCoder (e.g., converting ReadableDateTime/DateTime to org.joda.time.Instant, and allowing Iterable instead of requiring List for ITERABLE schema fields).
• Nullability enforcement: icebergRecordValueToBeamFieldValue immediately returns null when an Iceberg value is null without checking !field.getType().getNullable(). Previously, Row.Builder.build() caught this and threw an IllegalArgumentException; with attachValues, null is silently stored in non-nullable fields.
• The change in CdcOutputUtils.outputRow is safe because all values originate from an already-normalized Row or match the exact non-nullable STRING/INT64 metadata types.

---

💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.

[github-actions]

Assigning reviewers:

R: @chamikaramj for label iceberg.
Backup reviewer: Abacn (if the main reviewer is unresponsive, you can escalate to them by tagging their username)

Note: If you would like to opt out of this review, comment assign to next reviewer.

Available commands:

• stop reviewer notifications - opt out of the automated review tooling
• remind me after tests pass - tag the comment author after tests pass
• waiting on author - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)

The PR bot will only process comments in the main thread (not review comments).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (12bbb17) to head (e045973).
⚠️ Report is 68 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #40998       +/-   ##
===========================================
- Coverage   79.83%        0   -79.84%     
===========================================
  Files         494        0      -494     
  Lines       83575        0    -83575     
===========================================
- Hits        66722        0    -66722     
+ Misses      16853        0    -16853     

Flag	Coverage Δ
python	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.