SEPURI-SAI-KRISHNA commented on PR #12990:
URL: https://github.com/apache/seatunnel/pull/12990#issuecomment-6423996667
Reviewed, @JeremyXin. This addresses all four notes, and I checked it by
running your head `8f827ed71` rather than by reading it.
**Measured against your branch.** I compiled your `ChainedMapTransform` and
loaded it ahead of the built module, then fed it the two streams I had kept:
```text
serialVersionUID = 8769738606872706499
pre-#12804 stream -> deserialize OK, map() returned
SeaTunnelRow{tableId=test, kind=+I, fields=[x]}
current-dev stream -> InvalidClassException: stream 4659105814088680487,
local 8769738606872706499
```
So the UID is restored with no explicit pin, the rebuild really does repair
the instance rather than only surviving the read, and the unsupported window is
exactly the one you scoped out. Your `readObject` order is the one that works:
`defaultReadObject()`, assign, then refresh.
**The fixture test is a real regression test, not decoration.** On `dev` the
field is non-transient, so the class computes `4659105814088680487` while the
committed fixture carries `8769738606872706499`. Reading it without your fix
cannot do anything but throw, so the test has to go red if the change is
reverted.
**The E2E config does exercise the chained object,** which is the part that
quietly tests nothing if it is wrong. `CopyFieldMultiCatalogTransform extends
AbstractMultiCatalogMapTransform`, which overrides `composeTransforms` to
return `new ChainedMapTransform(transforms)` once there is more than one, so
`ALL_MATCH` with two entries on `test.xyz` builds the chain and not the bare
transform. The private `buildTransform` also sets `currentCatalogTable =
transform.getProducedCatalogTable()` between rules, so the second rule really
sees `name2`.
**One suspicion I chased and cleared,** since a reviewer will raise it:
`AbstractMultiCatalogTransform:53` is `protected final transient ReadonlyConfig
transformLevelConfig`, null after deserialization and not reassignable. It is
read only through `buildTransform`, and `setInputCatalogTables` delegates to
the inner transforms and calls `refreshOutputCatalogTables()` without
rebuilding, so nothing reads it post-construction. The comment you added in
#12938 is accurate.
The generator under `src/test/resources` follows the
`GenerateLegacyLLMFixture.java` precedent already on `dev`, so it is consistent
rather than novel.
**One thing to action:** `gh pr checks` reports no checks on this branch, so
`Build` has not run yet. Worth getting it green before asking for a merge.
--
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]
