xiangfu0 commented on PR #19284: URL: https://github.com/apache/pinot/pull/19284#issuecomment-5335951474
Reviewed against the split plan. One thing worth fixing before this merges, since this PR freezes the codecSpec grammar: `CodecSpecParser` accepts leading zeros in numeric args and canonicalization preserves them, so `ZSTD(03)` and `ZSTD(3)` canonicalize to different strings. The canonical form later gets embedded in V7 segment headers and compared for rewrite detection (upcoming PRs in this stack), so two semantically identical specs would compare as different and could trigger spurious rewrites. Suggest rejecting or normalizing leading-zero args in `parseArg`. Minor: `MAX_SPEC_LENGTH` = 64KB is generous for a string destined for a segment header; a 1–4KB cap would bound header bloat. -- 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]
