DanielLeens commented on PR #12578: URL: https://github.com/apache/seatunnel/pull/12578#issuecomment-6210075745
Thanks @hutiefang76 and @SEZ9 for the careful follow-up. A small clarification on the two test cases listed in the latest comment, since I think both are already covered at the current head `ef53a70c6f72` (I re-read `SeaTunnelRowTest` there): - Cross-overload on a single row: `testSetFieldInvalidatesCachedBytesSize` (`SeaTunnelRowTest.java:253-277`) already does both directions. The "typed" row calls `getBytesSize(rowType)`, then `setField`, then asserts the recomputed value through the untyped `getBytesSize()` and again through the typed overload. The "untyped" row does the reverse (`getBytesSize()` first, then `setField`, then `getBytesSize(rowType)`). That is exactly the collector -> metrics flow. - The documented limitation: `testDirectFieldMutationDoesNotInvalidateCachedBytesSize` (`:280-294`) pins that a write through `getFields()` keeps the stale cached value until a top-level `setField`, and `testNestedFieldMutationDoesNotInvalidateParentCachedBytesSize` (`:297-316`) pins the same for a nested row. These match the "Row Data Contract" paragraph in the EN/ZH architecture guides. @SEZ9, if I am misreading and you have a different assertion in mind, please point at it and I am happy to look again. Otherwise I do not think anything more is needed in the tests, and my conclusion from the last review is unchanged: no code-side blocker. The only open item is still CI. At the time of writing, `ef53a70c` has no `Build` run in the fork (the apache-side `Notify test workflow` reports it could not find one). @hutiefang76, please start the **Build** workflow manually from your fork's Actions tab on this branch (or push an empty commit); the status job should pick it up within about 15 minutes. Once it is green I am happy with this from my side, and the final approval still needs a maintainer with write access. -- 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]
