ccessor functions would be better, I will change the patch
that way.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Wed, Mar 10, 2021 at 6:18 PM Amit Kapila wrote:
> On Mon, Mar 8, 2021 at 7:19 PM Amit Langote wrote:
> > I just read through v25 and didn't find anything to complain about.
>
> Thanks a lot, pushed now! Amit L., your inputs are valuable for this work.
Glad I could be o
On Thu, Mar 11, 2021 at 4:25 AM Tom Lane wrote:
> Amit Langote writes:
> > On Wed, Mar 10, 2021 at 8:37 AM Tom Lane wrote:
> >> Hmm. So, the key point is that the values coming from the partitioned
> >> child table are injected into the test query as parameters, no
On Wed, Mar 10, 2021 at 8:37 AM Tom Lane wrote:
> Amit Langote writes:
> > On Fri, Mar 5, 2021 at 6:00 AM Tom Lane wrote:
> >> This claim seems false on its face:
> >>> All child constraints of a given foreign key constraint can use the
> >>
hat
leads to relids and relids_extra having different lengths, which trips
the Assert in ExecuteTruncateGuts().
--
Amit Langote
EDB: http://www.enterprisedb.com
really want ri_triggers.c having its own copy of
> low-level stuff like the tuple-locking code you copied. Seems
> like a likely maintenance hazard, so maybe some more refactoring
> is needed.
Okay, I will see if there's a way to avoid copying too much code.
--
Amit Langote
EDB: http://www.enterprisedb.com
gt; 0001* patch or if you want to review it further?
I just read through v25 and didn't find anything to complain about.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Mon, Mar 8, 2021 at 9:53 PM Andy Fan wrote:
> On Mon, Mar 8, 2021 at 8:42 PM Amit Langote wrote:
>> On Mon, Mar 8, 2021 at 8:39 PM Andy Fan wrote:
>> > On Mon, Mar 8, 2021 at 3:43 PM Andy Fan wrote:
>> >> My point below is a bit off-topic, but I want to
EATE TABLE p (a int) partitioned by list(a) RESTRICTED". We can add these
> limitation to restricted partitioned relation only.
I think you'd agree that the topics you want to discuss deserve a
separate discussion thread. You may refer to this discussion in that
new thread if you think th
will
do in the generic plan case.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Fri, Mar 5, 2021 at 7:50 AM Ryan Lambert wrote:
> On Wed, Mar 3, 2021 at 11:03 PM Amit Langote wrote:
>>
>> Sorry, this seems to have totally slipped my mind.
>>
>> Attached is the patch I had promised.
>>
>> Also, I have updated the title of the CF
On Fri, Mar 5, 2021 at 6:00 AM Tom Lane wrote:
> Amit Langote writes:
> > Updated patch attached.
>
> This claim seems false on its face:
>
> > All child constraints of a given foreign key constraint can use the
> > same RI query and the resulting plan, that is,
On Fri, Mar 5, 2021 at 10:47 PM Laurenz Albe wrote:
> On Wed, 2021-03-03 at 17:58 +0900, Amit Langote wrote:
> > For example, with the attached PoC patch:
>
> I have incorporated your POC patch and added a regression test.
>
> I didn't test it thoroughly though.
Thanks. Alt
OpExpr and
BoolExpr to begin with?
* Or maybe have you considered generalizing what
build_implied_pruning_quals() does so that other places like
indxpath.c can use the facility?
--
Amit Langote
EDB: http://www.enterprisedb.com
On Tue, Oct 20, 2020 at 9:46 PM Amit Langote wrote:
> On Tue, Oct 20, 2020 at 4:05 PM Andy Fan wrote:
> > On Wed, Oct 14, 2020 at 11:26 AM Andy Fan wrote:
> >> On Mon, Oct 12, 2020 at 4:37 PM Amit Langote
> >> wrote:
> >>> I think we should r
Hi David,
On Wed, Mar 3, 2021 at 10:21 PM David Steele wrote:
> On 12/7/20 10:59 PM, Amit Langote wrote:
> > On Tue, Dec 8, 2020 at 12:04 PM Kyotaro Horiguchi
> > wrote:
> >> At Tue, 8 Dec 2020 01:16:00 +0900, Amit Langote
> >> wrote in
> >>>
On Tue, Mar 2, 2021 at 5:47 PM Laurenz Albe wrote:
> On Tue, 2021-03-02 at 11:23 +0900, Amit Langote wrote:
> > +ALTER TABLE pagg_tab_ml SET (parallel_workers = 0);
> > +EXPLAIN (COSTS OFF)
> > +SELECT a FROM pagg_tab_ml WHERE b = 42;
> > +
to revisit this for quite a while.
+1
Also, I got reminded of this discussion from not so long ago:
https://www.postgresql.org/message-id/flat/16302-e45634e2c0e34e97%40postgresql.org
--
Amit Langote
EDB: http://www.enterprisedb.com
rkers=0 on the parent table shouldn't really
disable a regular (non-parallel-aware) Append running under Gather
even if it does Parallel Append (parallel-aware)? So in this test
case, there should have been a Gather atop Append, with individual
partitions scanned using Parallel Seq Scan where applicab
he attached?
>>
>
> Sorry for the delay.
> I see that the changes were made as described.
> Passes make check and make check-world yet again.
> I'm marking this Ready For Committer unless someone objects.
Thank you Corey for the review.
--
Amit Langote
EDB: http://www.enterprisedb.com
ables, and I can
see that it is actually wrong to access a partitioned table's
parallel_workers through this macro as-is. Although I hadn't tested
that, so thanks for pointing that out.
> Like:
> #define PartitionedTableGetParallelWorkers(relation, defaultpw) \
> xxx
> (PartitionedTableRdOptions *) (relation)->rd_options)->parallel_workers :
> (defaultpw))
I'm thinking it would be better to just modify the existing macro to
check relkind to decide which struct pointer type to cast the value in
rd_options to.
I will post an updated patch later.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Mon, Mar 1, 2021 at 12:38 PM Greg Nancarrow wrote:
> On Fri, Feb 26, 2021 at 5:50 PM Amit Langote wrote:
> >
> > On Fri, Feb 26, 2021 at 3:35 PM Greg Nancarrow wrote:
> > > On Fri, Feb 26, 2021 at 4:07 PM Amit Langote
> > > wrote:
> > > >
the test case
> to pg11. It worked fine for me (with no code changes), but it seems good to
> have it there just to make sure the buildfarm agrees with us on this.
Ah, good call.
--
Amit Langote
EDB: http://www.enterprisedb.com
r on a referencing table.
> I think it is worth adding some explanation in this document. Thus, explaining
> impact on referencing tables here, as it already describes behaviour of
> UPDATE on a partitioned table.
ISTM the description of the case that will now be prevented seems too
obscure
On Fri, Feb 26, 2021 at 3:35 PM Greg Nancarrow wrote:
> On Fri, Feb 26, 2021 at 4:07 PM Amit Langote wrote:
> > The attached patch fixes this, although I am starting to have second
> > thoughts about how we're tracking partitions in this patch. Wondering
> > if we should bi
Hi Greg,
Replying to an earlier email in the thread because I think I found a
problem relevant to the topic that was brought up.
On Wed, Feb 17, 2021 at 10:34 PM Amit Langote wrote:
> On Wed, Feb 17, 2021 at 10:44 AM Greg Nancarrow wrote:
> > Is the use of "table_clo
[2]. While no compromises can be made for the former, whether or not
to go for the latter probably involves some cost-benefit analysis,
something we can probably revisit.
I don't think we're compromising by not adding the partition OIDs when
the insert plan is not parallel, but the benefits of a
On Wed, Feb 24, 2021 at 6:12 PM Drouvot, Bertrand wrote:
> On 2/24/21 9:12 AM, Amit Langote wrote:
> > I've attached a patch with my suggested fixes and also test cases.
> > Please take a look.
>
> I had a look and it looks good to me. Also the new regression tests
k too can
crash similarly:
if (!TupIsNull(newslot) &&
((event == TRIGGER_EVENT_INSERT && insert_new_table) ||
(event == TRIGGER_EVENT_UPDATE && update_new_table)))
I've attached a patch with my suggested fixes and also test cases.
Please take a look.
--
Amit Langote
EDB: http://www.enterprisedb.com
v2-0001-Fix-use-after-tree-bug-with-AfterTriggersTableDat.patch
Description: Binary data
int32 vl_len_; /* varlena header (do not touch directly!) */
+ int parallel_workers; /* max number of parallel workers */
+} PartitionedTableRdOptions;
+
#define HEAP_MIN_FILLFACTOR 10
#define HEAP_DEFAULT_FILLFACTOR 100
--
Amit Langote
EDB: http://www.enterprisedb.com
On Fri, Feb 19, 2021 at 5:04 PM Masahiko Sawada wrote:
> On Mon, Feb 15, 2021 at 10:37 PM Amit Langote wrote:
> > Regarding the patch, I would have liked if it only prevented the
> > update when the foreign key that would be violated by the component
> > delete references th
On Fri, Feb 19, 2021 at 7:38 PM Amit Kapila wrote:
> On Thu, Feb 18, 2021 at 11:05 AM Amit Langote wrote:
> >
> > > > It also occurred to me that we can avoid pointless adding of
> > > > partitions if the final plan won't use parallelism. For that, the
On Fri, Feb 19, 2021 at 11:54 PM Seamus Abshere wrote:
> On Fri, Feb 19, 2021, at 2:30 AM, Amit Langote wrote:
> > Here is an updated version of the Seamus' patch that takes into
> > account these and other comments received on this thread so far.
> > Maybe warrants adding
On Tue, Feb 16, 2021 at 3:05 PM Amit Langote wrote:
> On Tue, Feb 16, 2021 at 1:35 AM Seamus Abshere wrote:
> > Here we go, my first patch... solves
> > https://www.postgresql.org/message-id/7d6fdc20-857c-4cbe-ae2e-c0ff9520e...@www.fastmail.com
>
> Thanks for s
rence page which
has some notes on row movement or somewhere else. Do you have
suggestions?
Attaching rebased version of the patches for HEAD to appease the cfbot.
--
Amit Langote
EDB: http://www.enterprisedb.com
v4-0001-Create-foreign-key-triggers-in-partitioned-tables.patch
Description: Binary dat
On Thu, Feb 18, 2021 at 6:52 PM Amit Langote wrote:
>
> Based on the discussion at:
>
> https://www.postgresql.org/message-id/6929d485-2d2a-da46-3681-4a400a3d794f%40enterprisedb.com
>
> I'm posting the patch for $subject here in this new thread and I'll
> add it to the next
to make some changes to both the
core code and postgres_fdw, which the attached patch implements.
Details are in the commit message.
--
Amit Langote
EDB: http://www.enterprisedb.com
v1-0001-Allow-batching-of-inserts-during-cross-partition-.patch
Description: Binary data
ef struct PartitionedTableRdOptions
{
int32 vl_len_;/* varlena header (do not touch directly!) */
int parallel_workers; /* max number of parallel workers */
} PartitionedTableRdOptions;
--
Amit Langote
EDB: http://www.enterprisedb.com
On Thu, Feb 18, 2021 at 4:36 AM Tomas Vondra
wrote:
> On 2/17/21 9:51 AM, Amit Langote wrote:
> > On Wed, Feb 17, 2021 at 5:46 PM Amit Langote
> > wrote:
> >> Sorry, I hadn't shared enough details of my investigations when I
> >> originally ran into thi
On Thu, Feb 18, 2021 at 10:03 AM Greg Nancarrow wrote:
> On Thu, Feb 18, 2021 at 12:34 AM Amit Langote wrote:
> > All that is to say that we should move our code to add partition OIDs
> > as plan dependencies to somewhere in set_plan_references(), which
> >
ned 0 later,
> triggering an assert.
>
> It's probably better to require GetForeignModifyBatchSize() to always
> return a valid batch size (>= 1). If batching is not supported, just
> return 1.
That makes sense.
How about the attached?
--
Amit Langote
EDB: http://www.enterprisedb.com
ForeignModifyBatchSize-sanity.patch
Description: Binary data
On Wed, Feb 17, 2021 at 10:44 AM Greg Nancarrow wrote:
> On Wed, Feb 17, 2021 at 12:19 AM Amit Langote wrote:
> > On Mon, Feb 15, 2021 at 4:39 PM Greg Nancarrow wrote:
> > > On Sat, Feb 13, 2021 at 12:17 AM Amit Langote
> > > wrote:
> > > > On Thu,
On Wed, Feb 17, 2021 at 5:46 PM Amit Langote wrote:
> On Wed, Feb 17, 2021 at 12:04 AM Tomas Vondra
> wrote:
> > On 2/16/21 10:25 AM, Amit Langote wrote:
> > > On Tue, Feb 16, 2021 at 1:36 AM Tomas Vondra
> > >> Thanks for the patch, it seems fine to me
On Tue, Feb 16, 2021 at 11:02 PM Laurenz Albe wrote:
> On Tue, 2021-02-16 at 16:29 +0900, Amit Langote wrote:
> > > I am +1 on allowing to override the degree of parallelism on a parallel
> > > append. If "parallel_workers" on the partitioned table is an
On Mon, Feb 15, 2021 at 4:39 PM Greg Nancarrow wrote:
> On Sat, Feb 13, 2021 at 12:17 AM Amit Langote wrote:
> > On Thu, Feb 11, 2021 at 4:43 PM Greg Nancarrow wrote:
> > > Actually, I tried adding the following in the loop that checks the
> > > parallel-safety of e
On Tue, Feb 16, 2021 at 1:36 AM Tomas Vondra
wrote:
> On 2/5/21 3:52 AM, Amit Langote wrote:
> > Tsunakwa-san,
> >
> > On Mon, Jan 25, 2021 at 1:21 PM tsunakawa.ta...@fujitsu.com
> > wrote:
> >> From: Amit Langote
> >>> Yes, it can be simpli
Hi,
On Tue, Feb 16, 2021 at 1:06 AM Laurenz Albe wrote:
> On Mon, 2021-02-15 at 17:53 +0900, Amit Langote wrote:
> > On Mon, Feb 15, 2021 at 5:28 PM Seamus Abshere wrote:
> > > It turns out parallel_workers may be a useful reloption for certain uses
> > > of partitio
Hi Seamus,
Please see my reply below.
On Tue, Feb 16, 2021 at 1:35 AM Seamus Abshere wrote:
> On Mon, Feb 15, 2021, at 3:53 AM, Amit Langote wrote:
> > On Mon, Feb 15, 2021 at 5:28 PM Seamus Abshere wrote:
> > > It turns out parallel_workers may be a useful reloptio
Thank you for looking at this.
On Mon, Feb 15, 2021 at 4:12 PM Masahiko Sawada wrote:
> On Wed, Jan 20, 2021 at 7:04 PM Amit Langote wrote:
> > This shows that the way we've made these triggers behave in general
> > can cause some unintended behaviors for foreign keys during
>
end path. */
appendpath = create_append_path(root, rel, NIL, partial_subpaths,
NIL, NULL, parallel_workers,
enable_parallel_append,
-1);
Note that the 'rel' in this code refers to the partitioned table for
which an Append path is being considered, so compute_parallel_worker()
using that 'rel' would use the partitioned table's
rel_parallel_workers as you are trying to do.
--
Amit Langote
EDB: http://www.enterprisedb.com
s:
/*
* Check whether we support copying data out of the specified relation,
* unless the caller also passed a non-NULL data_dest_cb, in which case,
* the callback will take care of it
*/
if (rel != NULL && rel->rd_rel->relkind != RELKIND_RELATION &&
data_dest_cb == NULL)
I just checked that this works or at least doesn't break any newly added tests.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Thu, Feb 11, 2021 at 4:43 PM Greg Nancarrow wrote:
> On Thu, Feb 11, 2021 at 5:33 PM Greg Nancarrow wrote:
> > On Tue, Feb 9, 2021 at 1:04 AM Amit Langote wrote:
> > >
> > > * I think that the concerns raised by Tsunakawa-san in:
> > >
> &g
On Wed, Feb 10, 2021 at 5:50 PM Amit Langote wrote:
> On Wed, Feb 10, 2021 at 5:24 PM Amit Kapila wrote:
> > On Wed, Feb 10, 2021 at 1:00 PM Amit Langote
> > wrote:
> > > On Wed, Feb 10, 2021 at 1:35 PM Greg Nancarrow
> > > wrote:
> > > > It's
On Wed, Feb 10, 2021 at 5:52 PM tsunakawa.ta...@fujitsu.com
wrote:
> From: Amit Langote
> > Just to be clear, I'm not suggesting that we should put effort into
> > making INSERT ... VALUES run in parallel. I'm just raising my concern
> > about embedding the assumption i
On Wed, Feb 10, 2021 at 5:24 PM Amit Kapila wrote:
> On Wed, Feb 10, 2021 at 1:00 PM Amit Langote wrote:
> > On Wed, Feb 10, 2021 at 1:35 PM Greg Nancarrow wrote:
> > > It's parallel UNSAFE because it's not safe or even possible to have a
> > > parallel plan
On Wed, Feb 10, 2021 at 5:03 PM tsunakawa.ta...@fujitsu.com
wrote:
> From: Amit Langote
> > On Wed, Feb 10, 2021 at 1:35 PM Greg Nancarrow
> > wrote:
> > > There's no "second-guessing" involved here.
> > > There is no underlying way of dividing u
On Wed, Feb 10, 2021 at 1:35 PM Greg Nancarrow wrote:
> On Wed, Feb 10, 2021 at 2:39 PM Amit Langote wrote:
> >
> > > The code check that you have identified above ensures that the INSERT
> > > has an underlying SELECT, because the planner won't (and shouldn't
> &
On Tue, Feb 9, 2021 at 10:30 AM Greg Nancarrow wrote:
> On Tue, Feb 9, 2021 at 1:04 AM Amit Langote wrote:
> > That brings me to to this part of the hunk:
> >
> > + /*
> > +* If there is no underlying SELECT, a parallel table-modification
> > +* operati
On Mon, Jan 25, 2021 at 2:23 PM Amit Langote wrote:
> On Tue, Dec 22, 2020 at 5:16 PM Amit Langote wrote:
> > On Mon, Dec 7, 2020 at 3:53 PM Amit Langote wrote:
> > > On Thu, Nov 12, 2020 at 5:04 PM Amit Langote
> > > wrote:
> > > > Attached new 000
s)
It's fair to argue that it would rarely make sense to use PREPARE for
bulk loads, but we need to tighten things up a bit here regardless.
--
Amit Langote
EDB: http://www.enterprisedb.com
v15_delta.diff
Description: Binary data
On Fri, Feb 5, 2021 at 11:01 PM Greg Nancarrow wrote:
> On Fri, Feb 5, 2021 at 11:12 PM Amit Langote wrote:
> > That seems good enough as far as I am concerned. Although either an
> > Assert as follows or a comment why the if (sub_action_ptr) is needed
>
On Fri, Feb 5, 2021 at 6:56 PM Greg Nancarrow wrote:
> On Fri, Feb 5, 2021 at 8:07 PM Amit Langote wrote:
> > > This is one reason for my original approach (though I admit, it was
> > > not optimal) because at least it was reliable and detected the
> > > modify
On Fri, Feb 5, 2021 at 4:53 PM Greg Nancarrow wrote:
> On Fri, Feb 5, 2021 at 5:21 PM Amit Langote wrote:
> > BTW, the original query's cteList is copied into sub_action query but
> > not into rule_action for reasons I haven't looked very closely into,
> > even though we'd l
That may be better.
BTW, the original query's cteList is copied into sub_action query but
not into rule_action for reasons I haven't looked very closely into,
even though we'd like to ultimately set the latter's hasModifyingCTE
to reflect the original query's, right? So we should do the following
at some point before returning:
if (sub_action->hasModifyingCTE)
rule_action->hasModifyingCTE = true;
> I'll do some further checks, because the rewriting is recursive and
> tricky, so don't want to miss any cases ...
Always a good idea.
--
Amit Langote
EDB: http://www.enterprisedb.com
---
>
> And the Basic test passed.
> What do you think ?
That is very close to what I was going to suggest, which is this:
diff --git a/src/backend/rewrite/rewriteHandler.c
b/src/backend/rewrite/rewriteHandler.c
index 0672f497c6..3c4417af98 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -631,6 +631,8 @@ rewriteRuleAction(Query *parsetree,
checkExprHasSubLink((Node *) rule_action->returningList);
}
+ rule_action->hasModifyingCTE |= parsetree->hasModifyingCTE;
+
return rule_action;
}
--
Amit Langote
EDB: http://www.enterprisedb.com
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 re
Tsunakwa-san,
On Mon, Jan 25, 2021 at 1:21 PM tsunakawa.ta...@fujitsu.com
wrote:
> From: Amit Langote
> > Yes, it can be simplified by using a local join to prevent the update of
> > the foreign
> > partition from being pushed to the remote server, for which my example in
On Thu, Feb 4, 2021 at 11:45 PM Tom Lane wrote:
> Amit Langote writes:
> > On Thu, Feb 4, 2021 at 4:24 PM Kohei KaiGai wrote:
> >> I think that MaxHeapAttributeNumber is a senseless restriction for foreign-
> >> tables. How about your opinions?
>
> > My fi
n have at most %d columns",
MaxHeapAttributeNumber)));
So, unless I am terribly wrong, we may have a shot at revisiting the
decision that would have set this limit.
--
Amit Langote
EDB: http://www.enterprisedb.com
xecConstraints(), etc. to live with a partial
tuple but maybe that's doable with some effort. We could even
optimize away evaluating any constraints if none of the constrained
columns are unchanged.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Thu, Feb 4, 2021 at 1:41 PM Tom Lane wrote:
> Amit Langote writes:
> > On Thu, Feb 4, 2021 at 1:08 AM Tom Lane wrote:
> >> Works for me on HEAD (using RHEL8.3, gcc 8.3.1, LLVM 10.0.1).
>
> > Thanks for checking. Must be my LLVM setup I guess:
>
> &g
On Thu, Feb 4, 2021 at 1:08 AM Tom Lane wrote:
>
> Amit Langote writes:
> > Has anybody seen this:
>
> Works for me on HEAD (using RHEL8.3, gcc 8.3.1, LLVM 10.0.1).
Thanks for checking. Must be my LLVM setup I guess:
$ llvm-config --version
7.0.1
$ cat /etc/redhat-release
Ce
at
postmaster.c:1402
#16 0x00791197 in main (argc=5, argv=0x2d863f0) at main.c:209
(gdb)
I can make this happen with PG > v12. Maybe there's something wrong
with my LLVM installation but just thought to ask here just in case.
--
Amit Langote
EDB: http://www.enterprisedb.com
> think it's a bit clearer now.
That is definitely an improvement, thanks.
--
Amit Langote
EDB: http://www.enterprisedb.com
I'd much rather explore getting rid
> of partitioned_rels completely. I'll now have a look at the patch
> you're proposing for that.
I've read Tom's patch (0001) and would definitely vote for that over
having partitioned_rels in Paths anymore.
> Thanks for investigating this and writing the patch.
+1. My apologies as well for failing to notice this thread sooner.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Wed, Jan 27, 2021 at 5:32 PM Kyotaro Horiguchi
wrote:
> At Sun, 24 Jan 2021 20:51:39 +0900, Amit Langote
> wrote in
> > Here's v5.
>
> At Mon, 25 Jan 2021 18:19:56 +0900, Amit Langote
> wrote in
> > > Anybody else want to look this patch over before I mark
Yamada-san,
On Wed, Jan 27, 2021 at 8:51 AM Tatsuro Yamada
wrote:
> On 2021/01/25 18:19, Amit Langote wrote:
> > On Mon, Jan 25, 2021 at 9:24 AM Corey Huinker
> > wrote:
> >> Anybody else want to look this patch over before I mark it Ready For
> >> Committ
On Mon, Jan 25, 2021 at 7:01 PM Amit Langote wrote:
> On Mon, Jan 25, 2021 at 6:06 PM Keisuke Kuroda
> wrote:
> > However, as already mentioned, the problem of memory bloat on DELETE
> > remains.
> > This can be solved by the patch in [1], but I think it is too much to a
g/message-id/flat/cab4b85d-9292-967d-adf2-be0d803c3e23%40nttcom.co.jp_1
Hmm, the patch tries to solve a general problem that SPI plans are not
being shared among partitions whereas they should be. So I don't
think that it's necessarily specific to DELETE. Until we have a
solution like the patch
On Mon, Jan 25, 2021 at 9:24 AM Corey Huinker wrote:
> On Sun, Jan 24, 2021 at 6:51 AM Amit Langote wrote:
>> Here's v5.
>
> v5 patches apply to master.
> Suggested If/then optimization is implemented.
> Suggested case merging is implemented.
> Passes make check and ma
On Wed, Jan 20, 2021 at 7:03 PM Amit Langote wrote:
> On Wed, Jan 20, 2021 at 4:13 PM Peter Eisentraut
> > Could you summarize here what you are trying to do with respect to what
> > was decided before? I'm a bit confused, looking through the patches you
> > have posted.
On Tue, Dec 22, 2020 at 5:16 PM Amit Langote wrote:
> On Mon, Dec 7, 2020 at 3:53 PM Amit Langote wrote:
> > On Thu, Nov 12, 2020 at 5:04 PM Amit Langote
> > wrote:
> > > Attached new 0002 which does these adjustments. I went with
> > > ri_RootTargetDesc
On Sun, Jan 24, 2021 at 2:17 AM Tomas Vondra
wrote:
> On 1/23/21 9:31 AM, Amit Langote wrote:
> > I was looking at this and it looks like we've got a problematic case
> > where postgresGetForeignModifyBatchSize() is called from
> > ExecInitRoutingInfo().
> >
>
be put next to each other (saving one block of
>> code).
Ah, yes. The TM_Updated case used to be handled a bit differently in
earlier unposted versions of the patch, though at some point I
concluded that the special handling was unnecessary, but didn't
realize what you just pointed out. Fixed.
&g
e function
report_trig_details();
create table p2 partition of p for values in (2);
insert into p values (2);
update p set a = 1; -- crashes
So we let's check mtstate->operation == CMD_INSERT in
ExecInitRoutingInfo() to prevent calling GetForeignModifyBatchSize()
in cross-update situations
ked, but yes, the prevailing context is
AfterTriggerTupleContext, a short-lived one that is reset for every
trigger event tuple. I'm still inclined to explicitly free those
objects, so changed like that. While at it, I also changed
mapped_partkey_attnums to root_partattrs for readability.
Attached v4
On Thu, Jan 21, 2021 at 10:42 AM Tomas Vondra
wrote:
> On 1/21/21 2:24 AM, Amit Langote wrote:
> > On Thu, Jan 21, 2021 at 9:56 AM Tomas Vondra
> > wrote:
> >> On 1/21/21 1:17 AM, Zhihong Yu wrote:
> >>> Hi,
> >>> The assignment to resul
st;
resultRelInfo->ri_projectReturning =
ExecBuildProjectionInfo(rlist, econtext, slot, >ps,
resultRelInfo->ri_RelationDesc->rd_att);
resultRelInfo++;
}
}
--
Amit Langote
EDB: http://www.enterprisedb.com
On Wed, Jan 20, 2021 at 4:13 PM Peter Eisentraut
wrote:
> On 2021-01-08 09:54, Amit Langote wrote:
> >>> I don't quite recall if the decision to implement it like this was
> >>> based on assuming that this is what users would like to see happen in
> >>>
On Wed, Jan 20, 2021 at 1:01 AM Tomas Vondra
wrote:
> On 1/19/21 7:23 AM, Amit Langote wrote:
> > On Tue, Jan 19, 2021 at 2:06 PM tsunakawa.ta...@fujitsu.com
> >> Actually, I tried to do it (adding the GetModifyBatchSize() call after
> >> BeginForeignModify(
infinitely even
> if the invariant is not satisfied.
>
> I guess a counter other than i would be better for this purpose.
I have done that in v3. Thanks.
--
Amit Langote
EDB: http://www.enterprisedb.com
as more insidious
> performance implications, but is also far less common, and whose solution
> will likely be very different.
Yeah, we should continue looking into the ways to make referenced-side
RI checks be less bloated.
I've attached the updated patch.
--
Amit Langote
EDB: http://www.enterpris
d table_tuple_lock status: %u", res);
>> ^~~~
>> /home/japin/Codes/postgresql/Debug/../src/backend/utils/adt/ri_triggers.c:419:4:
>> note: here
>> default:
>> ^~~
Thanks, will fix it.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Tue, Jan 19, 2021 at 2:06 PM tsunakawa.ta...@fujitsu.com
wrote:
> From: Amit Langote
> > I apologize in advance for being maybe overly pedantic, but I noticed
> > that, in ExecInitModifyTable(), you decided to place the call outside
> > the loop that goes over resultR
eflags);
}
Maybe it's fine today because we only care about inserts and there's
always only one entry in the resultRelations list in that case, but
that may not remain the case in the future.
--
Amit Langote
EDB: http://www.enterprisedb.com
ncremented
> when a match is found by the inner loop.
I've updated the patch to add the Assert. Thanks for taking a look.
--
Amit Langote
EDB: http://www.enterprisedb.com
v2-0001-Export-get_partition_for_tuple.patch
Description: Binary data
v2-0002-Avoid-using-SPI-for-some-RI-checks.patch
Description: Binary data
On Tue, Jan 19, 2021 at 3:01 AM Pavel Stehule wrote:
> po 18. 1. 2021 v 13:40 odesÃlatel Amit Langote
> napsal:
>> I started with the check that's performed when inserting into or
>> updating the referencing table to confirm that the new row points to a
>> valid row in
k.
The patch seems simple enough to consider for inclusion in v14 unless
of course we stumble into some dealbreaker(s). I will add this to
March CF.
--
Amit Langote
EDB: http://www.enterprisedb.com
[1]
https://www.postgresql.org/message-id/CADkLM%3DcTt_8Fg1Jtij5j%2BQEBOxz9Cuu4DiMDYOwdtktDA
On Sat, Jan 16, 2021 at 2:02 PM Amit Kapila wrote:
> On Fri, Jan 15, 2021 at 6:45 PM Amit Langote wrote:
> > On Fri, Jan 15, 2021 at 9:59 PM Amit Kapila wrote:
> > > We want to do this for Inserts where only Select can be parallel and
> > > Inserts will always be
On Sat, Jan 16, 2021 at 12:00 AM tsunakawa.ta...@fujitsu.com
wrote:
> From: Amit Langote
> > Okay, so maybe not moving the whole logic into the FDW's
> > BeginForeignModify(), but at least if we move this...
> >
> > @@ -441,6 +449,72 @@ ExecInser
601 - 700 of 2273 matches
Mail list logo
