zjw1111 commented on PR #224:
URL: https://github.com/apache/paimon-cpp/pull/224#issuecomment-5450741187
Thanks for streamlining the realtime test coverage. Could you make two
follow-up cleanups before merge?
1. Please centralize the remaining PK realtime transport-schema construction
in tests through `RealtimePrimaryKeyLayout::CreateSchema`. The production
write/read paths already use this helper, but the special-field prefix is still
assembled manually in:
- `src/paimon/core/operation/key_value_file_store_write_test.cpp`
- `src/paimon/core/realtime/primary_key_realtime_store_test.cpp`
(`TransportSchema`, `NestedTransportSchema`, and the nested-projection schema)
- `test/inte/realtime_write_inte_test.cpp` (`ReadPkSequences`)
These test-local copies can drift from `RealtimePrimaryKeyLayout`. Please
construct only the value fields at those sites and let `CreateSchema` add
`_VALUE_KIND`, `_SEQUENCE_NUMBER`, and `_REALTIME_OFFSET`. Tests that
intentionally mutate a helper-created schema to exercise `ValidateSchema`
should remain as they are.
2. Please synchronize the PR description with the current head:
- It still says that `SpecialFields::PreparedKeyValueSchema` and shared
prepared-field indexes define the transport schema, but those symbols no longer
exist; the current API is `RealtimePrimaryKeyLayout::CreateSchema` /
`ValidateSchema` with the layout indexes.
- It says that integration tests cover prepare-commit, commit, write, and
refresh failure recovery with context/writer recreation and external WAL
replay. Commit `813c6444` removed the fault-injection helpers and the related
recovery integration tests, so this coverage claim, and any test totals
affected by that cleanup, should be updated. The failure-recovery behavior may
remain documented as a caller contract if intended, but it should not be
presented as integration coverage that is still in this PR.
--
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]
