SEZ9 commented on PR #11479: URL: https://github.com/apache/seatunnel/pull/11479#issuecomment-5390959649
Thanks @DanielLeens for the careful re-check — you're right, and I appreciate the correction. **Issue 1 (PR11479-F1): withdrawn.** Your reading of the PUT branch at `30c35a30a438` matches the actual code: a single `values` array is decoded from the after-image, `restorePrimaryKeyColumns` runs on it before the INSERT/UPDATE_AFTER split, and no UPDATE_BEFORE row is emitted from this method at all. My finding described a second `decodeObjects(row.getOldValue()...)` path that doesn't exist here, so the "UPDATE_BEFORE still emits null PK" regression does not apply. I'm dropping it as a blocker. On the remaining items (Issues 2–8), nothing changes from my side yet — they still stand as open asks: - **F2 (blocking-ish ordering):** please confirm whether `RowKey.decode(...).getHandle()` in the snapshot deserializer can throw on short common-handle keys before the restore call runs, or add a test that covers a sub-8-byte encoded handle. - **F3:** the all-or-nothing restore in `CommonHandleDecoder` can overwrite PK values already present in the row value; a per-column null check (only restore when the slot is null) would resolve this. - **F4:** add validation before the fixed `skipBytes(11)` — key length and a check that the key is actually a common-handle record key. - **F5:** class-level Javadoc on `CommonHandleDecoder` and a comment explaining the `RECORD_KEY_PREFIX_LENGTH = 11` constant. - **F6:** tests for a streaming PUT with non-empty `oldValue` and for a `pkHandle` / non-clustered-PK table where the guard must skip. - **F7:** null-guard `TiTableInfo.getIndices()` in `primaryIndex()`. - **F8 (non-blocking):** consider caching the primary index lookup and avoiding the duplicate `toByteArray()` copies per record. No need to re-verify all of them independently before pushing — happy to review fixes for F2–F7 plus the F5 docs in one pass. Thanks again for keeping the review honest on Issue 1. <!-- streview-comment:511 --> -- 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]
