| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
| Cc: | pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com> |
| Subject: | Re: astreamer_lz4: fix bug of output pointer advancement in decompressor |
| Date: | 2026-03-04 17:16:55 |
| Message-ID: | 1531718.1772644615@sss.pgh.pa.us |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> writes:
> There have been a couple of LZ4-related patches recently, so I spent some time playing with the LZ4 path and found a bug in astreamer_lz4_decompressor_content().
Yup, that's clearly wrong. I failed to reproduce a crash with the
test hack you suggested, but no matter. Pushed with some cosmetic
editorialization.
The track record of all this client-side-compression logic is
really quite awful :-(. Another thing that I'm looking askance
at is the error handling, or rather lack of it:
ret = LZ4F_decompress(mystreamer->dctx,
next_out, &out_size,
next_in, &read_size, NULL);
if (LZ4F_isError(ret))
pg_log_error("could not decompress data: %s",
LZ4F_getErrorName(ret));
... continue on our merry way ...
I suspect whoever wrote this thought pg_log_error is equivalent
to elog(ERROR), but it's not; it just prints a message. It seems
highly unlikely to me that continuing onwards will result in a
good outcome. I'm a bit inclined to s/pg_log_error/pg_fatal/
throughout these files, at least in places where there's no
visible effort to handle the error.
regards, tom lane
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-03-04 17:46:50 | Re: Shutdown indefinitely stuck due to unflushed FPI_FOR_HINT record |
| Previous Message | Jacob Champion | 2026-03-04 17:01:06 | Re: pgsql: Change default value of default_toast_compression to "lz4", when |