xiangfu0 commented on PR #19307: URL: https://github.com/apache/pinot/pull/19307#issuecomment-5557218367
On "can we reduce the tests added?" — pushed `8ebcf1fa8c`. `ForwardIndexCreatorFactoryTest` goes from 34 to 25 invocations: the `compressionStatsEnabled` flag is no longer a data-provider dimension (it doubled every row, and the only thing it pins for V7 — that V7 never reports legacy compression stats — is now asserted once in a targeted spot), and the 10k-doc multi-chunk round trip gets its own small provider instead of running every pipeline at that scale. I kept four tests deliberately, since each pins behavior a regression could break silently rather than adding round-trip coverage: `testMalformedV7FileIsRejected` (its rows map one-to-one onto distinct validation branches, and the `rejectedAtRead` column pins the load-time-vs-first-read split created by the lazy frame validation), `testLegacyVersionSevenStillUsesLegacyReader` (legacy fixed-byte writers accept arbitrary version ints >= 4, so this guards against misrouting existing segments into the V7 reader), `testLegacyCompressionStatsRemainOptIn` (this PR moved the creator to the `FixedByteChunkWriter` interface and made stats `instanceof`-branched), and `testV7WriterAndReaderGuards`. Happy to cut further if you would rather trade some of that for a smaller diff. _🤖 Addressed by [Claude Code](https://claude.com/claude-code)_ -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
