SEZ9 commented on PR #12578: URL: https://github.com/apache/seatunnel/pull/12578#issuecomment-6229065970
Thanks for the follow-up on the tests. If `SeaTunnelRowTest` at `ef53a70c6f72` contains the cases as described — `testSetFieldInvalidatesCachedBytesSize` covering typed → `setField` → untyped and the reverse on the same row, plus `testDirectFieldMutationDoesNotInvalidateCachedBytesSize` and `testNestedFieldMutationDoesNotInvalidateParentCachedBytesSize` pinning the `getFields()` / nested-row non-invalidation behaviour — that covers the cross-overload and documented-limitation cases I asked for. I will confirm against the diff rather than take it as settled here, but I have no further test-side requests. Two earlier points were not addressed in the latest comment, so a quick confirmation on each would help: 1. **Volatile write in `setField`** – is the write still unconditional at `ef53a70c`? If so, please either explain why it is needed for correctness or guard it so only the cache invalidation touches the volatile field. 2. **Javadoc on `setField`** – the "Row Data Contract" paragraph in the architecture guides is a reasonable home for the prose, but does `setField` itself now carry a short Javadoc stating that it invalidates the cached size and that mutations via `getFields()` / nested rows do not? That is the method people will actually read, so I'd like the contract to live there as well. Once those two are confirmed I have nothing further from my side. <!-- streview-comment:1823 --> -- 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]
