> On Mon, May 17, 2021 at 3:07 PM houzj.f...@fujitsu.com
> wrote:
> >
> > Hi
> >
> > After 86dc900, In " src/include/nodes/pathnodes.h ", I noticed that it
> > uses the word " partitioned UPDATE " in the comment above struct
> RowIdentityVarInfo.
> >
> > But, it seems " inherited UPDATE " is used
Hi,
On Mon, May 17, 2021 at 3:07 PM houzj.f...@fujitsu.com
wrote:
>
> Hi
>
> After 86dc900, In " src/include/nodes/pathnodes.h ",
> I noticed that it uses the word " partitioned UPDATE " in the comment above
> struct RowIdentityVarInfo.
>
> But, it seems " inherited UPDATE " is used in the rest
Hi
After 86dc900, In " src/include/nodes/pathnodes.h ",
I noticed that it uses the word " partitioned UPDATE " in the comment above
struct RowIdentityVarInfo.
But, it seems " inherited UPDATE " is used in the rest of places.
Is it better to keep them consistent by using " inherited UPDATE " ?
B
On Wed, Mar 31, 2021 at 9:54 PM Amit Langote wrote:
> I reran some of the performance tests I did earlier (I've attached the
> modified test running script for reference):
For archives' sake, noticing a mistake in my benchmarking script, I
repeated the tests. Apparently, all pgbench runs were per
On Thu, 1 Apr 2021 at 15:09, Amit Langote wrote:
> Note that the patch over there doesn't do anything about
> AcquireExecutorLocks() bottleneck, as there are some yet-unsolved race
> conditions that were previously discussed here:
>
> https://www.postgresql.org/message-id/flat/CAKJS1f_kfRQ3ZpjQyHC
On Thu, Apr 1, 2021 at 12:58 AM Tom Lane wrote:
> Amit Langote writes:
> > On Tue, Mar 30, 2021 at 1:51 PM Tom Lane wrote:
> >> Here's a v13 patchset that I feel pretty good about.
>
> > Thanks. After staring at this for a day now, I do too.
>
> Thanks for looking! Pushed after some more docs-
On Wed, Mar 31, 2021 at 1:24 PM Tom Lane wrote:
> I agree that we have some existing behavior that's related to this, but
> it's still messy, and I couldn't find any evidence that suggested that the
> runtime lookup costs anything. Typical subplans are going to deliver
> long runs of tuples from
Robert Haas writes:
> On Tue, Mar 30, 2021 at 12:51 AM Tom Lane wrote:
>> Maybe that could be made more robust, but the other problem
>> is that the EXPLAIN output is just about unreadable; nobody will
>> understand what "(0)" means.
> I think this was an idea that originally came from me, promp
On Tue, Mar 30, 2021 at 12:51 AM Tom Lane wrote:
> Maybe that could be made more robust, but the other problem
> is that the EXPLAIN output is just about unreadable; nobody will
> understand what "(0)" means.
I think this was an idea that originally came from me, prompted by
what we already do fo
Amit Langote writes:
> On Tue, Mar 30, 2021 at 1:51 PM Tom Lane wrote:
>> Here's a v13 patchset that I feel pretty good about.
> Thanks. After staring at this for a day now, I do too.
Thanks for looking! Pushed after some more docs-fiddling and a final
read-through. I think the only code cha
On Tue, Mar 30, 2021 at 1:51 PM Tom Lane wrote:
> Here's a v13 patchset that I feel pretty good about.
Thanks. After staring at this for a day now, I do too.
> My original thought for replacing the "fake variable" design was to
> add another RTE holding the extra variables, and then have setref
Amit Langote writes:
> On Wed, Mar 31, 2021 at 11:56 AM Tom Lane wrote:
>> ... I've complained before that apply_scanjoin_target_to_paths
>> is brute-force and needs to be rewritten, but I don't really want to
>> undertake that task right now.
> I remember having *unsuccessfully* tried to make
>
On Wed, Mar 31, 2021 at 11:56 AM Tom Lane wrote:
> I noticed something else interesting. If you try an actually-useful
> UPDATE, ie one that has to do some computation in the target list,
> you can get a plan like this if it's a partitioned table:
>
> EXPLAIN (verbose, costs off) UPDATE parent SE
I noticed something else interesting. If you try an actually-useful
UPDATE, ie one that has to do some computation in the target list,
you can get a plan like this if it's a partitioned table:
EXPLAIN (verbose, costs off) UPDATE parent SET f2 = f2 + 1;
QUERY PLAN
On Wed, Mar 31, 2021 at 7:13 AM Tom Lane wrote:
> I wrote:
> > However, I then tried a partitioned equivalent of the 6-column case
> > (script also attached), and it looks like
> > 6 columns 16551 19097 15637 18201
> > which is really noticeably worse, 16% or so.
>
> ... and on the third
I wrote:
> However, I then tried a partitioned equivalent of the 6-column case
> (script also attached), and it looks like
> 6 columns 16551 19097 15637 18201
> which is really noticeably worse, 16% or so.
... and on the third hand, that might just be some weird compiler-
and platform-sp
I wrote:
> ... I also tried variants
> of that involving updating two columns of a 6-column table and of a
> 10-column table, figuring that those cases might be a bit more
> representative of typical usage (see attached scripts).
Argh, I managed to attach the wrong file for the 10-column test
case
I wrote:
> I've not made any attempt to do performance testing on this,
> but I think that's about the only thing standing between us
> and committing this thing.
I think the main gating condition for committing this is "does it
make things worse for simple non-partitioned updates?". The need
for
On Mon, Mar 29, 2021 at 11:41 PM Tom Lane wrote:
> Amit Langote writes:
> > On Sun, Mar 28, 2021 at 1:30 AM Tom Lane wrote:
> >> I wanted to give a data dump of where I am. I've reviewed and
> >> nontrivially modified 0001 and the executor parts of 0002, and
> >> I'm fairly happy with the state
Amit Langote writes:
> On Sun, Mar 28, 2021 at 1:30 AM Tom Lane wrote:
>> I wanted to give a data dump of where I am. I've reviewed and
>> nontrivially modified 0001 and the executor parts of 0002, and
>> I'm fairly happy with the state of that much of the code now.
> Thanks a lot for that work
On Sun, Mar 28, 2021 at 1:30 AM Tom Lane wrote:
> Amit Langote writes:
> > Attached updated version of the patch. I have forgotten to mention in
> > my recent posts on this thread one thing about 0001 that I had
> > mentioned upthread back in June. That it currently fails a test in
> > postgres
On Sun, Mar 28, 2021 at 3:11 AM Tom Lane wrote:
> I wrote:
> > ... which is what forced you to remove or lobotomize several regression
> > test cases. Now admittedly, that just moves the state of play for
> > cross-partition updates into postgres_fdw partitions from "works
> > sometimes" to "work
I wrote:
> ... which is what forced you to remove or lobotomize several regression
> test cases. Now admittedly, that just moves the state of play for
> cross-partition updates into postgres_fdw partitions from "works
> sometimes" to "works never". But I don't like the idea that we'll
> be taking
Amit Langote writes:
> On Fri, Mar 26, 2021 at 3:07 AM Tom Lane wrote:
>> I think the reason is that the parser
>> initially builds all INSERT ... SELECT cases with the SELECT as an
>> explicit subquery, giving rise to a SubqueryScan node just below the
>> ModifyTable, which will project exactly
On Fri, Mar 26, 2021 at 3:07 AM Tom Lane wrote:
> Robert Haas writes:
> > - Until I went back and found your earlier remarks on this thread, I
> > was confused as to why you were replacing a JunkFilter with a
> > ProjectionInfo. I think it would be good to try to add some more
> > explicit commen
Robert Haas writes:
> - Until I went back and found your earlier remarks on this thread, I
> was confused as to why you were replacing a JunkFilter with a
> ProjectionInfo. I think it would be good to try to add some more
> explicit comments about that design choice, perhaps as header comments
> f
On Thu, Mar 25, 2021 at 4:22 AM Tom Lane wrote:
> I wrote:
> > Yeah, it's on my to-do list for this CF, but I expect it's going to
> > take some concentrated study and other things keep intruding :-(.
>
> I've started to look at this seriously,
Thanks a lot.
> and I wanted to make a note
> about
I wrote:
> Yeah, it's on my to-do list for this CF, but I expect it's going to
> take some concentrated study and other things keep intruding :-(.
I've started to look at this seriously, and I wanted to make a note
about something that I think we should try to do, but there seems
little hope of sh
Robert Haas writes:
> I spent some time studying this patch this morning. As far as I can
> see, 0001 is a relatively faithful implementation of the design Tom
> proposed back in early 2019. I think it would be nice to either get
> this committed or else decide that we don't want it and what we're
On Wed, Mar 3, 2021 at 9:39 AM Amit Langote wrote:
> Just noticed that a test added by the recent 927f453a941 fails due to
> 0002. We no longer allow moving a row into a postgres_fdw partition
> if it is among the UPDATE's result relations, whereas previously we
> would if the UPDATE on that part
Robert Haas writes:
> As to why it causes us pain, I don't have a full picture of that.
> Target list construction is one problem: we build all these target
> lists for intermediate notes during planning and they're long enough
> -- if the user has a bunch of columns -- and planning is cheap enoug
On Fri, Feb 5, 2021 at 12:06 PM Tom Lane wrote:
> You do realize that we're just copying Datums from one level to the
> next? For pass-by-ref data, the Datums generally all point at the
> same physical data in some disk buffer ... or if they don't, it's
> because the join method had a good reason
Robert Haas writes:
> It's a bit annoying that we percolate things up the tree the way we do
> at all. I realize this is far afield from the topic of this thread.
> But suppose that I join 5 tables and select a subset of the table
> columns in the output. Suppose WLOG the join order is A-B-C-D-E.
On Thu, Feb 4, 2021 at 10:14 PM Amit Langote wrote:
> I guess it would depend on how many of those hidden columns there need
> to be (in addition to the existing "ctid" hidden column) and how many
> levels of the plan tree they would need to climb through when bubbling
> up.
My memory is a bit fu
On Thu, Feb 4, 2021 at 10:41 PM Robert Haas wrote:
> On Thu, Feb 4, 2021 at 4:33 AM Amit Langote wrote:
> > To be clear, the new refetch in ExecModifyTable() is to fill in the
> > unchanged columns in the new tuple. If we rejigger the
> > table_tuple_update() API to receive a partial tuple (esse
On Thu, Feb 4, 2021 at 4:33 AM Amit Langote wrote:
> So would zheap refetch a tuple using the "ctid" column in the plan's
> output tuple and then use some other columns from the fetched tuple to
> actually do the update?
Yes.
> To be clear, the new refetch in ExecModifyTable() is to fill in the
On Wed, Jan 27, 2021 at 4:42 AM Robert Haas wrote:
> On Fri, Oct 30, 2020 at 6:26 PM Heikki Linnakangas wrote:
> > Yeah, you need to access the old tuple to update its t_ctid, but
> > accessing it twice is still more expensive than accessing it once. Maybe
> > you could optimize it somewhat by ke
On Fri, Oct 30, 2020 at 6:26 PM Heikki Linnakangas wrote:
> Yeah, you need to access the old tuple to update its t_ctid, but
> accessing it twice is still more expensive than accessing it once. Maybe
> you could optimize it somewhat by keeping the buffer pinned or
> something. Or push the responsi
On 29/10/2020 15:03, Amit Langote wrote:
Rebased over the recent executor result relation related commits.
ModifyTablePath didn't get the memo that a ModifyTable can only have one
subpath after these patches. Attached patch, on top of your v5 patches,
cleans that up.
- Heikki
diff --git a/s
On Sat, Oct 31, 2020 at 7:26 AM Heikki Linnakangas wrote:
> On 31/10/2020 00:12, Tom Lane wrote:
> > Heikki Linnakangas writes:
> >> But if you do:
> >
> >> postgres=# explain verbose update tab set a = 1, b = 2;
> >> QUERY PLAN
> >> -
On 31/10/2020 00:12, Tom Lane wrote:
Heikki Linnakangas writes:
But if you do:
postgres=# explain verbose update tab set a = 1, b = 2;
QUERY PLAN
-
Update on public.ta
Heikki Linnakangas writes:
> But if you do:
> postgres=# explain verbose update tab set a = 1, b = 2;
> QUERY PLAN
> -
> Update on public.tab (cost=0.00..269603.27 rows=0 w
On 30/10/2020 23:10, Tom Lane wrote:
Heikki Linnakangas writes:
I also did some quick performance testing with a simple update designed
as a worst-case scenario:
vacuum tab; update tab set b = b, a = a;
In this case, the patch fetches the old tuple, but it wouldn't really
need to, because
Heikki Linnakangas writes:
> I also did some quick performance testing with a simple update designed
> as a worst-case scenario:
> vacuum tab; update tab set b = b, a = a;
> In this case, the patch fetches the old tuple, but it wouldn't really
> need to, because all the columns are updated. Co
On 29/10/2020 15:03, Amit Langote wrote:
On Sun, Oct 4, 2020 at 11:44 AM Amit Langote wrote:
On Fri, Sep 11, 2020 at 7:20 PM Amit Langote wrote:
Here are the commit messages of the attached patches:
[PATCH v3 1/3] Overhaul how updates compute a new tuple
I tried to assess the performance i
Hi,
On Thu, Oct 1, 2020 at 1:32 PM Michael Paquier wrote:
>
> On Fri, Sep 11, 2020 at 07:20:56PM +0900, Amit Langote wrote:
> > I have been working away at this and have updated the patches for many
> > cosmetic and some functional improvements.
>
> Please note that this patch set fails to apply.
On Fri, Sep 11, 2020 at 07:20:56PM +0900, Amit Langote wrote:
> I have been working away at this and have updated the patches for many
> cosmetic and some functional improvements.
Please note that this patch set fails to apply. Could you provide a
rebase please?
--
Michael
signature.asc
Descrip
On Wed, May 13, 2020 at 9:21 AM Amit Langote wrote:
>
> Maybe I am misunderstanding you, but the more the rows to update, the
> more overhead we will be paying with the new approach.
Yes, that's right. How much is that compared to the current planning
overhead. How many rows it takes for that ove
On Thu, May 14, 2020 at 7:55 AM David Rowley wrote:
> On Wed, 13 May 2020 at 19:02, Amit Langote wrote:
> > > As for which ResultRelInfos to initialize, couldn't we just have the
> > > planner generate an OidList of all the ones that we could need.
> > > Basically, all the non-pruned partitions.
On Wed, 13 May 2020 at 19:02, Amit Langote wrote:
> > As for which ResultRelInfos to initialize, couldn't we just have the
> > planner generate an OidList of all the ones that we could need.
> > Basically, all the non-pruned partitions.
>
> Why would replacing list of RT indexes by OIDs be better?
On Tue, May 12, 2020 at 10:57 PM Tom Lane wrote:
> Amit Langote writes:
> > On Tue, May 12, 2020 at 5:25 AM Robert Haas wrote:
> >> Ah, that makes sense. If we can invent dummy columns on the parent
> >> rel, then most of what I was worrying about no longer seems very
> >> worrying.
>
> > IIUC,
On Wed, May 13, 2020 at 8:52 AM David Rowley wrote:
> On Wed, 13 May 2020 at 00:54, Ashutosh Bapat
> wrote:
> >
> > On Mon, May 11, 2020 at 8:11 PM Amit Langote
> > wrote:
> > > > Per row overhead would be incurred for every row whereas the plan time
> > > > overhead is one-time or in case of a
On Tue, May 12, 2020 at 9:54 PM Ashutosh Bapat
wrote:
> On Mon, May 11, 2020 at 8:11 PM Amit Langote wrote:
> > > Per row overhead would be incurred for every row whereas the plan time
> > > overhead is one-time or in case of a prepared statement almost free.
> > > So we need to compare it esp. w
On Wed, 13 May 2020 at 00:54, Ashutosh Bapat
wrote:
>
> On Mon, May 11, 2020 at 8:11 PM Amit Langote wrote:
> > > Per row overhead would be incurred for every row whereas the plan time
> > > overhead is one-time or in case of a prepared statement almost free.
> > > So we need to compare it esp. w
Amit Langote writes:
> On Tue, May 12, 2020 at 5:25 AM Robert Haas wrote:
>> Ah, that makes sense. If we can invent dummy columns on the parent
>> rel, then most of what I was worrying about no longer seems very
>> worrying.
> IIUC, the idea is to have "dummy" columns in the top parent's
> relta
On Tue, May 12, 2020 at 5:25 AM Robert Haas wrote:
> On Mon, May 11, 2020 at 4:22 PM Tom Lane wrote:
> > Robert Haas writes:
> > > If the parent is RTI 1, and the children are RTIs 2..6, what
> > > varno/varattno will we use in RTI 1's tlist to represent a column that
> > > exists in both RTI 2
On Mon, May 11, 2020 at 8:11 PM Amit Langote wrote:
> > Per row overhead would be incurred for every row whereas the plan time
> > overhead is one-time or in case of a prepared statement almost free.
> > So we need to compare it esp. when there are 2000 partitions and all
> > of them are being upd
On Mon, May 11, 2020 at 4:22 PM Tom Lane wrote:
> Robert Haas writes:
> > If the parent is RTI 1, and the children are RTIs 2..6, what
> > varno/varattno will we use in RTI 1's tlist to represent a column that
> > exists in both RTI 2 and RTI 3 but not in RTI 1, 4, 5, or 6?
>
> Fair question. We
Robert Haas writes:
> If the parent is RTI 1, and the children are RTIs 2..6, what
> varno/varattno will we use in RTI 1's tlist to represent a column that
> exists in both RTI 2 and RTI 3 but not in RTI 1, 4, 5, or 6?
Fair question. We don't have any problem representing the column
as it exists
On Mon, May 11, 2020 at 2:48 PM Tom Lane wrote:
> > There doesn't seem to be any execution-time
> > problem with such a representation, but there might be a planning-time
> > problem with building it,
>
> Possibly. We manage to cope with not-all-alike children now, of course,
> but I think it mig
Robert Haas writes:
> I believe that you'd want to have happen here is for each child to
> emit the row identity columns that it knows about, and emit NULL for
> the others. Then when you do the Append you end up with a row format
> that includes all the individual identity columns, but for any
>
On Mon, May 11, 2020 at 8:58 AM Ashutosh Bapat
wrote:
> What happens if there's a mixture of foreign and local partitions or
> mixture of FDWs? Injecting junk columns from all FDWs in the top level
> target list will cause error because those attributes won't be
> available everywhere.
I think th
Hi Ashutosh,
Thanks for chiming in.
On Mon, May 11, 2020 at 9:58 PM Ashutosh Bapat
wrote:
> On Fri, May 8, 2020 at 7:03 PM Amit Langote wrote:
> > I do think that doing this would be worthwhile even if we may be
> > increasing ModifyTable's per-row overhead slightly, because planning
> > overhe
On Fri, May 8, 2020 at 7:03 PM Amit Langote wrote:
>
> Here is a sketch for implementing the design that Tom described here:
> https://www.postgresql.org/message-id/flat/357.1550612935%40sss.pgh.pa.us
>
> In short, we would like to have only one plan for ModifyTable to get
> tuples out of to updat
64 matches
Mail list logo
