Обсуждение: Patch for bug #17056 fast default on non-plain table
Here's a patch I propose to apply to fix this bug (See <https://www.postgresql.org/message-id/flat/759e997e-e1ca-91cd-84db-f4ae963fada1%40dunslane.net#b1cf11c3eb1f450bed97c79ad473909f>) cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Вложения
Andrew Dunstan <andrew@dunslane.net> writes: > Here's a patch I propose to apply to fix this bug (See > <https://www.postgresql.org/message-id/flat/759e997e-e1ca-91cd-84db-f4ae963fada1%40dunslane.net#b1cf11c3eb1f450bed97c79ad473909f>) If I'm reading the code correctly, your change in RelationBuildTupleDesc is scribbling directly on the disk buffer, which is surely not okay. I don't understand why you need that at all given the other defenses you added ... but if you need it, you have to modify the tuple AFTER copying it. regards, tom lane
On 6/17/21 11:05 AM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> Here's a patch I propose to apply to fix this bug (See >> <https://www.postgresql.org/message-id/flat/759e997e-e1ca-91cd-84db-f4ae963fada1%40dunslane.net#b1cf11c3eb1f450bed97c79ad473909f>) > If I'm reading the code correctly, your change in RelationBuildTupleDesc > is scribbling directly on the disk buffer, which is surely not okay. > I don't understand why you need that at all given the other defenses > you added ... but if you need it, you have to modify the tuple AFTER > copying it. OK, will fix. I think we do need it (See Andres' comment in the bug thread). It should be a fairly simple fix. Thanks for looking. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
On 6/17/21 11:13 AM, Andrew Dunstan wrote: > On 6/17/21 11:05 AM, Tom Lane wrote: >> Andrew Dunstan <andrew@dunslane.net> writes: >>> Here's a patch I propose to apply to fix this bug (See >>> <https://www.postgresql.org/message-id/flat/759e997e-e1ca-91cd-84db-f4ae963fada1%40dunslane.net#b1cf11c3eb1f450bed97c79ad473909f>) >> If I'm reading the code correctly, your change in RelationBuildTupleDesc >> is scribbling directly on the disk buffer, which is surely not okay. >> I don't understand why you need that at all given the other defenses >> you added ... but if you need it, you have to modify the tuple AFTER >> copying it. > > OK, will fix. I think we do need it (See Andres' comment in the bug > thread). It should be a fairly simple fix. > > revised patch attached. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Вложения
Andrew Dunstan <andrew@dunslane.net> writes:
> revised patch attached.
OK. One other point is that in HEAD, you only need the hunk that
prevents atthasmissing from becoming incorrectly set. The hacks
to cope with it being already wrong are only needed in the back
branches. Since we already forced initdb for beta2, there will
not be any v14 installations in which pg_attribute contains
a wrong value.
regards, tom lane
On 6/17/21 1:45 PM, Tom Lane wrote: > Andrew Dunstan <andrew@dunslane.net> writes: >> revised patch attached. > OK. One other point is that in HEAD, you only need the hunk that > prevents atthasmissing from becoming incorrectly set. The hacks > to cope with it being already wrong are only needed in the back > branches. Since we already forced initdb for beta2, there will > not be any v14 installations in which pg_attribute contains > a wrong value. > > Good point. Should I replace the relcache.c changes in HEAD with an Assert? Or just skip them altogether? cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Andrew Dunstan <andrew@dunslane.net> writes:
> On 6/17/21 1:45 PM, Tom Lane wrote:
>> OK. One other point is that in HEAD, you only need the hunk that
>> prevents atthasmissing from becoming incorrectly set.
> Good point. Should I replace the relcache.c changes in HEAD with an
> Assert? Or just skip them altogether?
I wouldn't bother touching relcache.c.
regards, tom lane