From: | Amit Langote <amitlangote09(at)gmail(dot)com> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Subject: | Re: Eliminating SPI from RI triggers - take 2 |
Date: | 2022-07-13 11:59:30 |
Message-ID: | CA+HiwqEXHrrhVO3QaNps-NkdXjpHWXSWrrGc5=FzjaBkYCoUZw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Jul 9, 2022 at 1:15 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 1, 2022 at 2:23 AM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
> > So, I hacked together a patch (attached 0001) that invents an "RI
> > plan" construct (struct RIPlan) to replace the use of an "SPI plan"
> > (struct _SPI_plan).
> >
> > With that in place, I decided to rebase my previous patch [1] to use
> > this new interface and the result is attached 0002.
>
Thanks for taking a look at this. I'll try to respond to other points
in a separate email, but I wanted to clarify something about below:
> I find my ego slightly wounded by the comment that "the partition
> descriptor machinery has a hack that assumes that the queries
> originating in this module push the latest snapshot in the
> transaction-snapshot mode." It's true that the partition descriptor
> machinery gives different answers depending on the active snapshot,
> but, err, is that a hack, or just a perfectly reasonable design
> decision?
I think my calling it a hack of "partition descriptor machinery" is
not entirely fair (sorry), because it's talking about the following
comment in find_inheritance_children_extended(), which describes it as
being a hack, so I mentioned the word "hack" in my comment too:
/*
* Cope with partitions concurrently being detached. When we see a
* partition marked "detach pending", we omit it from the returned set
* of visible partitions if caller requested that and the tuple's xmin
* does not appear in progress to the active snapshot. (If there's no
* active snapshot set, that means we're not running a user query, so
* it's OK to always include detached partitions in that case; if the
* xmin is still running to the active snapshot, then the partition
* has not been detached yet and so we include it.)
*
* The reason for this hack is that we want to avoid seeing the
* partition as alive in RI queries during REPEATABLE READ or
* SERIALIZABLE transactions: such queries use a different snapshot
* than the one used by regular (user) queries.
*/
That bit came in to make DETACH CONCURRENTLY produce sane answers for
RI queries in some cases.
I guess my comment should really have said something like:
HACK: find_inheritance_children_extended() has a hack that assumes
that the queries originating in this module push the latest snapshot
in transaction-snapshot mode.
> An alternative might be for PartitionDirectoryLookup to take
> a snapshot as an explicit argument rather than relying on the global
> variable to get that information from context. I generally feel that
> we rely too much on global variables where we should be passing around
> explicit parameters, so if you're just arguing that explicit
> parameters would be better here, then I agree and just didn't think of
> it. If you're arguing that making the answer depend on the snapshot is
> itself a bad idea, I don't agree with that.
No, I'm not arguing that using a snapshot there is wrong and haven't
really thought hard about an alternative.
I tend to agree passing a snapshot explicitly might be better than
using ActiveSnapshot stuff for this.
--
Thanks, Amit Langote
EDB: http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Dagfinn Ilmari Mannsåker | 2022-07-13 11:59:50 | Re: Building PostgreSQL in external directory is broken? |
Previous Message | Peter Eisentraut | 2022-07-13 11:52:06 | Re: [RFC] building postgres with meson -v9 |