SEZ9 commented on PR #12578: URL: https://github.com/apache/seatunnel/pull/12578#issuecomment-5964633209
Thanks for the follow-up in ec14f3a — this covers the points from the previous round. - Conditional clearing: good, skipping the write when the cached size is already zero removes the unconditional barrier on the row-construction path. Please just confirm the guard is a single read of the cached field before the conditional write, so a fresh row pays only a read on each `setField`. - Docs/Javadoc: having the contract on `setField` itself plus the EN/ZH row docs is the right place for it. Could you confirm what happened to the prose paragraph under "Source Contract" in the architecture guide — was it removed, moved, or cross-referenced to the new Javadoc? Either is fine, as long as it is no longer the sole carrier of the contract and its placement matches its scope. - Tests: the same-row cross-overload, `getFields()` and nested-child cases plus removal of the duplicate fixture address the test findings. One ask: please make sure the `getFields()` and nested-child cases assert the documented limitation explicitly (i.e., the cached size is observably stale after such a mutation until a top-level `setField`), rather than only asserting that the top-level invalidation path works. - JMH: agreed, the overlapping intervals mean no perf claim should be made; stating it as a non-regression is sufficient. Once you confirm the three items above I'm happy to take another look. <!-- streview-comment:1482 --> -- 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]
