Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?

From: Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net>, Bruce Momjian <bruce(at)momjian(dot)us>, "David E(dot) Wheeler" <david(at)justatheory(dot)com>, Christophe Pettus <christophe(dot)pettus(at)pgexperts(dot)com>, Postgres-Bugs <pgsql-bugs(at)postgresql(dot)org>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] [BUGS] BUG #14825: enum type: unsafe use?
Date: 2018-01-11 23:01:55
Message-ID: CAEepm=3SKxCi=P8UnrZ2yhgA8UdnJvAoq_Oxo4R4kG2Wpf-tZg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Fri, Oct 6, 2017 at 2:45 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Tue, Oct 3, 2017 at 9:38 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>>> Do you have any suggestion as to how we should transmit the blacklist to
>>> parallel workers?
>>
>> How about storing them in the a dshash table instead of dynahash?
>> Similar to how we're now dealing with the shared typmod registry stuff?
>> It should be fairly simple to now simply add a new struct Session member
>> shared_enum_whatevs_table.
>
> Yeah, that approach seems worth exploring.

Andrew Dunstan asked me off-list which README covers that stuff. Erm,
there isn't one, so I was going to write some explanation here to see
if that could help... but after looking at this I'm not sure I agree
it's the right approach anyway.

The reason commit cc5f8136 introduced Session and
SharedRecordTypmodRegistry was that we have some state that is
session-scoped and writable by any worker. In contrast:

1. The enum OID blacklist has transaction scope. If you wanted to
put it into the Session you'd have to explicitly zap it at end of
transaction. Incidentally dshash has no fast reset, though that could
be added, but I'd probably want fast per-transaction cleanup to skip
retail destruction entirely and simply give back all the memory, just
like MemoryContext does. There are other transaction-scoped things
we'll eventually want to share, like ComboCIDs, so I think we'll need
something like that, but no one has been brave enough to propose the
machinery yet.

2. The enum OID blacklist is read-only for workers. They don't
create new blacklisted enums and don't see that they ever will.

So I agree with Tom's suggestion:

On Wed, Oct 4, 2017 at 2:29 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Perhaps serialize the contents into an array in DSM, then rebuild a hash
> table from that in the worker. Robert might have a better idea though.

I'd happily volunteer to write or review a patch to do that. Is there
a rebase of the stuff that got reverted, to build on?

--
Thomas Munro
http://www.enterprisedb.com

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Andres Freund 2018-01-11 23:05:10 Re: BUG #14932: SELECT DISTINCT val FROM table gets stuck in an infinite loop
Previous Message Tom Lane 2018-01-11 22:46:54 Re: [BUGS] Improper const-evaluation of HAVING with grouping sets and subquery pullup

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-01-11 23:11:37 Re: Parameters in user-defined aggregate final functions
Previous Message David Fetter 2018-01-11 22:53:24 Re: Parameters in user-defined aggregate final functions