First draft of back-branch release notes is done

Started by Tom Lanealmost 3 years ago13 messages
#1Tom Lane
tgl@sss.pgh.pa.us

... at

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=f282b026787da69d88a35404cf62f1cc21cfbb7c

As usual, please send corrections/comments by Sunday.

regards, tom lane

#2Jonathan S. Katz
jkatz@postgresql.org
In reply to: Tom Lane (#1)
Re: First draft of back-branch release notes is done

On Feb 4, 2023, at 10:24 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

... at

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=f282b026787da69d88a35404cf62f1cc21cfbb7c

As usual, please send corrections/comments by Sunday.

While reviewing for the release announcement, I noticed this (abbreviated as I’m on my mobile):

“Prevent clobbering of cached parsetrees…Bad things could happen if…”

While I chuckled over the phrasing, I’m left to wonder what the “bad things” are, in case I
need to check an older version to see if I’m susceptible to this bug.

Thanks,

Jonathan

#3Alvaro Herrera
alvherre@alvh.no-ip.org
In reply to: Tom Lane (#1)
Re: First draft of back-branch release notes is done

On 2023-Feb-03, Tom Lane wrote:

... at

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=f282b026787da69d88a35404cf62f1cc21cfbb7c

As usual, please send corrections/comments by Sunday.

Fix edge-case data corruption in shared tuplestores (Dmitry Astapov)

If the final chunk of a large tuple being written out to disk was
exactly 32760 bytes, it would be corrupted due to a fencepost bug.
This is a hazard for parallelized plans that require a tuplestore,
such as parallel hash join. The query would typically fail later
with corrupted-data symptoms.

I think this sounds really scary, because people are going to think that
their stored data can get corrupted -- they don't necessarily know what
a "shared tuplestore" is. Maybe "Avoid query failures in parallel hash
joins" as headline?

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"The Gord often wonders why people threaten never to come back after they've
been told never to return" (www.actsofgord.com)

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#3)
Re: First draft of back-branch release notes is done

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

On 2023-Feb-03, Tom Lane wrote:

Fix edge-case data corruption in shared tuplestores (Dmitry Astapov)

I think this sounds really scary, because people are going to think that
their stored data can get corrupted -- they don't necessarily know what
a "shared tuplestore" is. Maybe "Avoid query failures in parallel hash
joins" as headline?

Hmmm ... are we sure it *can't* lead to corruption of stored data,
if this happens during an INSERT or UPDATE plan? I'll grant that
such a case seems pretty unlikely though, as the bogus data
retrieved from the tuplestore would have to not cause a failure
within the query before it can get written out.

Also, aren't shared tuplestores used in more places than just
parallel hash join? I mentioned that as an example, not an
exhaustive list of trouble spots.

regards, tom lane

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jonathan S. Katz (#2)
Re: First draft of back-branch release notes is done

"Jonathan S. Katz" <jkatz@postgresql.org> writes:

On Feb 4, 2023, at 10:24 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

“Prevent clobbering of cached parsetrees…Bad things could happen if…”

While I chuckled over the phrasing, I’m left to wonder what the “bad things” are, in case I
need to check an older version to see if I’m susceptible to this bug.

Fair. I was trying to avoid committing to specific consequences.
The assertion failure seen in the original report (#17702) wouldn't
occur for typical users, but they might see crashes or "unexpected node
type" failures. Maybe we can say that instead.

regards, tom lane

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#4)
Re: First draft of back-branch release notes is done

I wrote:

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

I think this sounds really scary, because people are going to think that
their stored data can get corrupted -- they don't necessarily know what
a "shared tuplestore" is. Maybe "Avoid query failures in parallel hash
joins" as headline?

Maybe less scary if we make it clear we're talking about a temporary file?

<para>
Fix edge-case corruption of temporary data within shared tuplestores
(Dmitry Astapov)
</para>

<para>
If the final chunk of a large tuple being written out to a temporary
file was exactly 32760 bytes, it would be corrupted due to a
fencepost bug. This is a hazard for parallelized plans that require
a tuplestore, such as parallel hash join. The query would typically
fail later with corrupted-data symptoms.
</para>

regards, tom lane

#7Thomas Munro
thomas.munro@gmail.com
In reply to: Tom Lane (#4)
Re: First draft of back-branch release notes is done

On Mon, Feb 6, 2023 at 8:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Alvaro Herrera <alvherre@alvh.no-ip.org> writes:

On 2023-Feb-03, Tom Lane wrote:

Fix edge-case data corruption in shared tuplestores (Dmitry Astapov)

I think this sounds really scary, because people are going to think that
their stored data can get corrupted -- they don't necessarily know what
a "shared tuplestore" is. Maybe "Avoid query failures in parallel hash
joins" as headline?

Hmmm ... are we sure it *can't* lead to corruption of stored data,
if this happens during an INSERT or UPDATE plan? I'll grant that
such a case seems pretty unlikely though, as the bogus data
retrieved from the tuplestore would have to not cause a failure
within the query before it can get written out.

Agreed. I think you have to be quite unlucky to hit this in the first
place (very large tuples with very particular alignment), and then
you'd be highly likely to fail in some way due to corrupted tuple
size, making permanent corruption extremely unlikely.

Also, aren't shared tuplestores used in more places than just
parallel hash join? I mentioned that as an example, not an
exhaustive list of trouble spots.

Shared file sets (= a directory of temp files with automatic cleanup)
are used by more things, but shared tuplestores (= a shared file set
with a tuple-oriented interface on top, to support partial scan) are
currently only used by PHJ.

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Thomas Munro (#7)
Re: First draft of back-branch release notes is done

Thomas Munro <thomas.munro@gmail.com> writes:

On Mon, Feb 6, 2023 at 8:57 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Also, aren't shared tuplestores used in more places than just
parallel hash join? I mentioned that as an example, not an
exhaustive list of trouble spots.

Shared file sets (= a directory of temp files with automatic cleanup)
are used by more things, but shared tuplestores (= a shared file set
with a tuple-oriented interface on top, to support partial scan) are
currently only used by PHJ.

Oh, okay. I'll change it to say "corruption within parallel hash
join", then, and not use the word "tuplestore" at all.

regards, tom lane

#9Jonathan S. Katz
jkatz@postgresql.org
In reply to: Tom Lane (#5)
Re: First draft of back-branch release notes is done

On 2/5/23 3:01 PM, Tom Lane wrote:

"Jonathan S. Katz" <jkatz@postgresql.org> writes:

On Feb 4, 2023, at 10:24 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

“Prevent clobbering of cached parsetrees…Bad things could happen if…”

While I chuckled over the phrasing, I’m left to wonder what the “bad things” are, in case I
need to check an older version to see if I’m susceptible to this bug.

Fair. I was trying to avoid committing to specific consequences.
The assertion failure seen in the original report (#17702) wouldn't
occur for typical users, but they might see crashes or "unexpected node
type" failures. Maybe we can say that instead.

I did a quick readthrough of #17702. Your proposal sounds reasonable.

Based on that explanation and reading #17702, I'm still not sure if this
will make the cut in the release announcement itself, but +1 for
modifying it in the release notes.

Thanks,

Jonathan

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Jonathan S. Katz (#9)
Re: First draft of back-branch release notes is done

"Jonathan S. Katz" <jkatz@postgresql.org> writes:

On 2/5/23 3:01 PM, Tom Lane wrote:

Fair. I was trying to avoid committing to specific consequences.
The assertion failure seen in the original report (#17702) wouldn't
occur for typical users, but they might see crashes or "unexpected node
type" failures. Maybe we can say that instead.

I did a quick readthrough of #17702. Your proposal sounds reasonable.

Based on that explanation and reading #17702, I'm still not sure if this
will make the cut in the release announcement itself, but +1 for
modifying it in the release notes.

The notes now say

<para>
Prevent clobbering of cached parsetrees for utility statements in
SQL functions (Tom Lane, Daniel Gustafsson)
</para>

<para>
If a SQL-language function executes the same utility command more
than once within a single calling query, it could crash or report
strange errors such as <quote>unrecognized node type</quote>.
</para>

regards, tom lane

#11Jonathan S. Katz
jkatz@postgresql.org
In reply to: Tom Lane (#10)
Re: First draft of back-branch release notes is done

On 2/5/23 9:39 PM, Tom Lane wrote:

<para>
Prevent clobbering of cached parsetrees for utility statements in
SQL functions (Tom Lane, Daniel Gustafsson)
</para>

<para>
If a SQL-language function executes the same utility command more
than once within a single calling query, it could crash or report
strange errors such as <quote>unrecognized node type</quote>.
</para>

regards, tom lane

+1. Thanks!

Jonathan

#12Justin Pryzby
pryzby@telsasoft.com
In reply to: Tom Lane (#1)
Re: First draft of back-branch release notes is done

On Fri, Feb 03, 2023 at 02:32:39PM -0500, Tom Lane wrote:

... at

https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=f282b026787da69d88a35404cf62f1cc21cfbb7c

As usual, please send corrections/comments by Sunday.

It's of no concern, but I was curious why this one wasn't included:

commit 72aea955d49712a17c08748aa9abcbcf98c32fc5
Author: Thomas Munro <tmunro@postgresql.org>
Date: Fri Jan 6 16:38:46 2023 +1300

Fix pg_truncate() on Windows.

Commit 57faaf376 added pg_truncate(const char *path, off_t length), but
"length" was ignored under WIN32 and the file was unconditionally
truncated to 0.

There was no live bug, since the only caller passes 0.

Fix, and back-patch to 14 where the function arrived.

#13Tom Lane
tgl@sss.pgh.pa.us
In reply to: Justin Pryzby (#12)
Re: First draft of back-branch release notes is done

Justin Pryzby <pryzby@telsasoft.com> writes:

It's of no concern, but I was curious why this one wasn't included:

commit 72aea955d49712a17c08748aa9abcbcf98c32fc5
Author: Thomas Munro <tmunro@postgresql.org>
Date: Fri Jan 6 16:38:46 2023 +1300

Fix pg_truncate() on Windows.

Commit 57faaf376 added pg_truncate(const char *path, off_t length), but
"length" was ignored under WIN32 and the file was unconditionally
truncated to 0.

There was no live bug, since the only caller passes 0.

I concluded that due to the lack of live bug, this would not be of
interest to end users. The back-patch was just for future-proofing.

regards, tom lane