| From: | Japin Li <japinli(at)hotmail(dot)com> |
|---|---|
| To: | zengman <zengman(at)halodbtech(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: [PATCH] Fix minor issues in astreamer_zstd.c |
| Date: | 2026-01-10 07:53:52 |
| Message-ID: | SYAPR01MB3038F2C5900F07E36667385CB683A@SYAPR01MB3038.ausprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sat, 10 Jan 2026 at 15:03, Japin Li <japinli(at)hotmail(dot)com> wrote:
> On Sat, 10 Jan 2026 at 12:59, "zengman" <zengman(at)halodbtech(dot)com> wrote:
>> Hi all,
>>
>> While reviewing the zstd code, I noticed two minor issues in astreamer_zstd.c that appear to be errors:
>>
>> First, the file header comment for astreamer_zstd_compressor incorrectly references "lz4 compression" instead of "zstd compression" — this looks like a copy-paste error from astreamer_lz4.c.
>> Second, in the astreamer_zstd_decompressor_finalize function, it seems
>> there may be an error in parameter passing: the function uses the full
>> buffer capacity (bbs_buffer.maxlen) when calling astreamer_content,
>> rather than the actual length of decompressed data (zstd_outBuf.pos).
>>
>> I’ve attached two patches to address these potential issues:
>> 0001: Fixes the incorrect comment in the astreamer_zstd.c header
>
> +1
>
>> 0002: Corrects the parameter passed to astreamer_content in astreamer_zstd_decompressor_finalize
>
> Not sure about this — LZ4 and GZIP do the same thing.
>
> Based on the code coverage reports in [0] and [1], it appears that this case
> is already covered by existing tests for both LZ4 and GZIP.
>
> [0] https://coverage.postgresql.org/src/fe_utils/astreamer_lz4.c.gcov.html
> [1] https://coverage.postgresql.org/src/fe_utils/astreamer_gzip.c.gcov.html
>
After re-reading the LZ4 and GZIP code, I agree — the parameter is indeed wrong.
It looks like decompressor_finalize has the same issue in both implementations.
Would you mind taking a look?
diff --git a/src/fe_utils/astreamer_gzip.c b/src/fe_utils/astreamer_gzip.c
index e8d62f754ca..4ccec1a5a7f 100644
--- a/src/fe_utils/astreamer_gzip.c
+++ b/src/fe_utils/astreamer_gzip.c
@@ -349,7 +349,7 @@ astreamer_gzip_decompressor_finalize(astreamer *streamer)
*/
astreamer_content(mystreamer->base.bbs_next, NULL,
mystreamer->base.bbs_buffer.data,
- mystreamer->base.bbs_buffer.maxlen,
+ mystreamer->bytes_written,
ASTREAMER_UNKNOWN);
astreamer_finalize(mystreamer->base.bbs_next);
diff --git a/src/fe_utils/astreamer_lz4.c b/src/fe_utils/astreamer_lz4.c
index bcee7e088de..570e5305874 100644
--- a/src/fe_utils/astreamer_lz4.c
+++ b/src/fe_utils/astreamer_lz4.c
@@ -401,7 +401,7 @@ astreamer_lz4_decompressor_finalize(astreamer *streamer)
*/
astreamer_content(mystreamer->base.bbs_next, NULL,
mystreamer->base.bbs_buffer.data,
- mystreamer->base.bbs_buffer.maxlen,
+ mystreamer->bytes_written,
ASTREAMER_UNKNOWN);
astreamer_finalize(mystreamer->base.bbs_next);
--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | John Naylor | 2026-01-10 08:46:12 | Re: Correct comment wording in extension.c |
| Previous Message | Kirill Reshke | 2026-01-10 07:41:26 | Re: GIN pageinspect support for entry tree and posting tree |