fix: cap internal planner-statistics blob types at the preview size (#106) - #109
Merged
Merged
Conversation
…106) extract.rs's binary_blob_wrapper! macro -- used for PgMcvList, PgDependencies, PgNdistinct, and the two BRIN summary types -- had its own independently-drifted copy of the untruncated BLOB: encoding that #87 fixed for ordinary BYTEA columns. Its doc comment even claimed this matched the builtin, but the builtin's equivalent macro (binary_wrapper! in extract/advanced_types.rs) actually calls the builtin's own encode_blob (the truncated encoder), not an untruncated one -- so the plugin had drifted into a second, separate divergence. Switched the macro to use the shared crate::utils::blob::encode_blob (added in #87) instead of inlining its own base64 encoding, and corrected the stale doc comment. TDD: added internal_statistics_blob_types_truncate_a_large_buffer_to_the_preview_cap in extract_tests.rs (a synthetic 20 KB buffer, since real pg_statistic_ext_data values are typically well under 10 KB in practice -- confirmed live against a real CREATE STATISTICS run, which produced a 78-byte value, far too small to exercise truncation). Confirmed the new test fails against the pre-fix code (20480 vs expected 10240) before the fix, passes after. The existing internal_statistics_blob_types_decode_and_accept_correctly test (small, live-captured buffers) continues to pass unchanged, since those values are far under the cap. Verified live against a real PostgreSQL instance: a genuine pg_statistic_ext_data.stxdndistinct value (from an actual CREATE STATISTICS ... ON a, b, c run) still decodes correctly through the fixed macro with no error.
aesslinger
added
the
prerelease:rc
Version suggestionBased on this PR's title (
This is informational only — no tag or release is created automatically yet. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
extract.rs'sbinary_blob_wrapper!macro — used forPgMcvList,PgDependencies,PgNdistinct,PgBrinBloomSummary,PgBrinMinmaxMultiSummary— had its own independently-drifted copy of the untruncatedBLOB:encoding that BYTEA read path encodes the full base64 instead of a truncated preview (builtin caps at 10 KB) #87 fixed for ordinary BYTEA columns. Its doc comment even claimed this matched the builtin, but the builtin's equivalent macro (binary_wrapper!inextract/advanced_types.rs) actually calls the builtin's ownencode_blob(the truncated encoder), not an untruncated one — so this repo had drifted into a second, separate divergence from the same root cause BYTEA read path encodes the full base64 instead of a truncated preview (builtin caps at 10 KB) #87 addressed.crate::utils::blob::encode_blob(added in BYTEA read path encodes the full base64 instead of a truncated preview (builtin caps at 10 KB) #87) instead of inlining its own base64 encoding, and corrected the stale doc comment.Fixes #106 (filed as a follow-up while working on #87).
Test plan
internal_statistics_blob_types_truncate_a_large_buffer_to_the_preview_captoextract_tests.rs— a synthetic 20 KB buffer, since realpg_statistic_ext_datavalues are typically well under 10 KB in practice (confirmed live below). Confirmed the new test fails against the pre-fix code (20480 vs expected 10240) before the fix, passes after.internal_statistics_blob_types_decode_and_accept_correctlytest (small, live-captured buffers) continues to pass unchanged, since those values are far under the cap.cargo test --lib— 321 passedcargo clippy --all-targets -- -D warnings— cleancargo fmt --all -- --check— cleanCREATE STATISTICS ... ON a, b, cand confirmed the realpg_statistic_ext_data.stxdndistinctvalue (78 bytes) still decodes correctly through the fixed macro with no error — since real values are typically small, this confirms no regression on the common path while the unit test covers the large-value truncation the issue is actually about.