Hi,
I noticed a couple of comments that were obsoleted by commit
578b229718 which forgot to remove them. Attached fixes that.
--
Amit Langote
EDB: http://www.enterprisedb.com
fix-obsolete-comments.patch
Description: Binary data
n, it calls ExecSetRelationUsesMultiInsert() to turn the flag
on. Also, given the confusion regarding how execPartition.c
manipulates the flag, maybe change ExecFindPartition() to accept a
Boolean parameter multi_insert, which it will pass down to
ExecInitPartitionInfo(), which in turn will call
ExecSetRelationUsesMultiInsert() for a given partition. Of course, if
the logic in ExecSetRelationUsesMultiInsert() determines that
multi-insert can't be used, for the reasons listed in the function,
then the caller will have to live with that decision.
Any other ideas on how to make this work and look better?
--
Amit Langote
EDB: http://www.enterprisedb.com
[1]
https://www.postgresql.org/message-id/d3fbf3bc93b7bcd99ff7fa9ee41e0e20%40postgrespro.ru
On Sat, Oct 3, 2020 at 8:26 PM Amit Langote wrote:
> On Sat, Oct 3, 2020 at 8:15 PM Tomas Vondra
> wrote
> > On Sat, Oct 03, 2020 at 11:42:21AM +0900, Amit Langote wrote:
> > >On Fri, Oct 2, 2020 at 11:32 PM David G. Johnston
> > > wrote:
> > >>
he error message. Note that we do this reverse
conversion only for tuple routing target relations, not all child
result relations, so in that sense it's a bit inconsistent. Maybe we
don't need to be too pedantic about showing the exact same tuple as
the user inserted (that is, one
Thanks for the review.
On Wed, Nov 11, 2020 at 5:55 PM Heikki Linnakangas wrote:
> On 10/11/2020 17:32, Heikki Linnakangas wrote:
> > On 10/11/2020 13:12, Amit Langote wrote:
> >> On second thought, it seems A would amount to merely a cosmetic
> >> adjustment of the
On Wed, Nov 4, 2020 at 11:32 AM Amit Langote wrote:
> On Tue, Nov 3, 2020 at 9:05 PM Heikki Linnakangas wrote:
> > On 03/11/2020 10:27, Amit Langote wrote:
> > > Please check the attached if that looks better.
> >
> > Great, thanks! Yeah, I like that much bette
On Wed, Nov 4, 2020 at 11:32 AM Amit Langote wrote:
> On Tue, Nov 3, 2020 at 9:05 PM Heikki Linnakangas wrote:
> > A) We could change FDW API so that BeginDirectModify takes the same
> > arguments as BeginForeignModify(). That avoids the assumption that it's
> > a
On Tue, Nov 3, 2020 at 9:05 PM Heikki Linnakangas wrote:
> On 03/11/2020 10:27, Amit Langote wrote:
> > Please check the attached if that looks better.
>
> Great, thanks! Yeah, I like that much better.
>
> This makes me a bit unhappy:
>
> >
> >
On Mon, Nov 2, 2020 at 10:53 PM Amit Langote wrote:
> On Mon, Nov 2, 2020 at 10:19 PM Heikki Linnakangas wrote:
> > (/me reads patch further) I presume that's what you referred to in the
> > commit message:
> >
> > > Also, extend this lazy ini
On Mon, Nov 2, 2020 at 10:19 PM Heikki Linnakangas wrote:
> On 30/10/2020 08:13, Amit Langote wrote:
> > /*
> > * Perform WITH CHECK OPTIONS check, if any.
> > */
> > static void
> > ExecProcessWithCheckOptions(ModifyTableState *mtstate,
tuple contents.
> Or push the responsibility down to the table AM, passing the
> AM only the modified columns, and let the AM figure out how to deal with
> the columns that were not modified, hoping that it can do something smart.
That sounds interesting, but maybe a sizable project on its own?
Thanks a lot for taking a look at this, BTW.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Mon, Nov 2, 2020 at 9:51 AM David Rowley wrote:
>
> On Tue, 27 Oct 2020 at 19:40, Amit Langote wrote:
> > Some comments:
>
> Thanks for having a look at this.
>
> I've made some adjustments to those comments and pushed.
Thank you.
--
Amit Langote
EDB: http://www.enterprisedb.com
On Wed, Sep 30, 2020 at 12:34 PM Amit Langote wrote:
> On Thu, Sep 24, 2020 at 2:26 PM Amit Langote wrote:
> > I saw the CF-bot failure too yesterday, although it seems that it's
> > because the bot is trying to apply the patch version meant for v11
> > branch onto HE
by direct modify operations.
0002 refactors ResultRelInfo initialization do be don lazily on first use
I call these v6, because the last version posted on this thread was
v5, even though it went through a couple of iterations on the above
thread. Sorry about the confusion.
--
Amit Langote
EDB
On Wed, Oct 28, 2020 at 4:46 PM Amit Langote wrote:
>
> On Tue, Oct 27, 2020 at 10:23 PM Heikki Linnakangas wrote:
> > This patch looks reasonable to me at a quick glance. I'm a bit worried
> > or unhappy about the impact on FDWs, though. It doesn't seem nice that
don't change authoritative mappings, but maybe can add some
> one-way conversions for the convenience.
Maybe UCS_TO_EUC_JP.pl could do something like the above.
Are there other cases that were fixed like this in the past, either
for euc_jp or sjis?
--
Amit Langote
EDB: http://www.enterprisedb.com
b/src/backend/utils/mb/Unicode/euc_jp_to_utf8.map
- {0xa1dd, 0xe28892},
+ {0xa1dd, 0xefbc8d},
--- a/src/backend/utils/mb/Unicode/utf8_to_euc_jp.map
+++ b/src/backend/utils/mb/Unicode/utf8_to_euc_jp.map
- {0xe28892, 0xa1dd},
+ {0xefbc8d, 0xa1dd},
Can't tell what reason there was to do that, but there must have been
some. Maybe the Japanese character sets prefer full-width hyphen
minus (unicode U+FF0D) over mathematical minus sign (U+2212)?
--
Amit Langote
EDB: http://www.enterprisedb.com
On Wed, Oct 28, 2020 at 12:02 PM Amit Langote wrote:
> On Tue, Oct 27, 2020 at 10:23 PM Heikki Linnakangas wrote:
> > But since this applies on top of the "overhaul update/delete processing"
> > patch, let's tackle that patch set next. Could you rebase that, please?
tion in
> postgres_fdw.
Ah, calling BeginDirectModify() itself lazily sounds like a good idea;
see attached updated 0001 to see how that looks. While updating that
patch, I realized that the ForeignScan.resultRelation that we
introduced in 178f2d560d will now be totally useless. :-(
--
Amit Langote
EDB: htt
On Tue, Oct 27, 2020 at 10:23 PM Heikki Linnakangas wrote:
> On 23/10/2020 12:37, Amit Langote wrote:
> > To explain these numbers a bit, "overheaul update/delete processing"
> > patch improves the performance of that benchmark by allowing the
> > updates to use
On Sun, Oct 25, 2020 at 10:06 AM David Rowley wrote:
> On Fri, 16 Oct 2020 at 03:01, Amit Langote wrote:
> >
> > Going over the last few emails, it seems that David held off from
> > committing the patch, because of the lack of confidence in its
> > robustne
On Fri, Oct 23, 2020 at 4:04 PM Heikki Linnakangas wrote:
> On 22/10/2020 16:49, Amit Langote wrote:
> > On Tue, Oct 20, 2020 at 9:57 PM Amit Langote
> > wrote:
> >> On Mon, Oct 19, 2020 at 8:55 PM Heikki Linnakangas wrote:
> >>> It's probably t
On Thu, Oct 22, 2020 at 11:25 PM Alvaro Herrera wrote:
>
> On 2020-Oct-22, Amit Langote wrote:
>
> > 0001 fixes a thinko of the recent commit 1375422c782 that I discovered
> > when debugging a problem with 0003.
>
> Hmm, how hard is it to produce a test case that fail
On Tue, Oct 20, 2020 at 9:57 PM Amit Langote wrote:
> On Mon, Oct 19, 2020 at 8:55 PM Heikki Linnakangas wrote:
> > It's probably true that there's no performance gain from initializing
> > them more lazily. But the reasoning and logic around the initialization
> >
On Mon, Oct 19, 2020 at 8:55 PM Heikki Linnakangas wrote:
> On 17/10/2020 18:54, Alvaro Herrera wrote:
> > On 2020-Oct-17, Amit Langote wrote:
> >> As I said in my previous email, I don't see how we can make
> >> initializing the map any lazier than it alread
Hi Andy,
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 remove this duplicative logic and return the
>>> generated steps in a list from th
On Mon, Oct 19, 2020 at 8:48 PM Heikki Linnakangas wrote:
> On 19/10/2020 07:54, Amit Langote wrote:
> > On Sun, Oct 18, 2020 at 12:54 AM Alvaro Herrera
> > wrote:
> >> Well, I was thinking on making the ri_PartitionInfo be about
> >> partitioning in general, no
On Sun, Oct 18, 2020 at 12:54 AM Alvaro Herrera wrote:
> On 2020-Oct-17, Amit Langote wrote:
> > Hmm, I don't see ri_PartitionCheckExpr as being a piece of routing
> > information, because it's primarily meant to be used when inserting
> > *directly* into a part
On Fri, Oct 16, 2020 at 11:45 PM Alvaro Herrera wrote:
> On 2020-Oct-16, Amit Langote wrote:
> > On Thu, Oct 15, 2020 at 11:59 PM Heikki Linnakangas wrote:
> > > On Wed, Oct 14, 2020 at 6:04 PM Heikki Linnakangas
> > > wrote:
>
> > > A
lds directly to ResultRelInfo.
If we do that, we'll end up with 3 notations for the same thing across
releases: In v10 and v11, PartitionRoutingInfos members are saved in
arrays in ModifyTableState, totally detached from the partition
ResultRelInfos. In v12 (3f2393edef), we moved them into ResultRelInfo
but chose to add them into a sub-struct (PartitionRoutingInfo), which
in retrospect was not a great decision. Now if we pull them into
ResultRelInfo, we'll have invented the 3rd notation. Maybe that makes
things hard when back-patching bug-fixes?
Attached updated patch.
--
Amit Langote
EDB: http://www.enterprisedb.com
v18-0001-Revise-child-to-root-tuple-conversion-map-manage.patch
Description: Binary data
will prevent add_partial_path() from being the
arbitrator of whether such a path is really the best in terms of cost.
If we can't be confident in that approach, maybe we should consider
making the run-time pruning code cope with nested Appends?
--
Amit Langote
EDB: http://www.enterprisedb.com
On Wed, Oct 14, 2020 at 6:04 PM Heikki Linnakangas wrote:
> On 14/10/2020 09:44, Amit Langote wrote:
> > I like the idea of storing the ResultRelInfo in ForeignScanState, but
> > it would be better if we can document the fact that an FDW may not
> > reliably access unti
pruning for UPDATE/DELETE with [1], initializing all result
relations before initializing the plan tree will mean most of those
ResultRelInfos will be unused, because run-time pruning that occurs
when the plan tree is initialized (and/or when it is executed) may
eliminate most but a f
On Tue, Oct 13, 2020 at 7:13 PM Heikki Linnakangas wrote:
> On 13/10/2020 07:32, Amit Langote wrote:
> > On Tue, Oct 13, 2020 at 1:57 AM Heikki Linnakangas wrote:
> >> It occurred to me that if we do that (initialize the array lazily),
> >> there's very
might be
scanned.
However, we don't currently consult an inheritance parent relation's
stats at all during "base" relation planning, which is why you don't
see the parent relation's statistics reflected in the row count and
costs assigned to its (Append
On Tue, Oct 13, 2020 at 1:57 AM Heikki Linnakangas wrote:
> On 12/10/2020 16:47, Amit Langote wrote:
> > On Mon, Oct 12, 2020 at 8:12 PM Heikki Linnakangas wrote:
> >> 1. We have many different cleanup/close routines now:
> >> ExecCloseResultRelations, ExecCl
On Mon, Oct 12, 2020 at 8:12 PM Heikki Linnakangas wrote:
> On 09/10/2020 11:01, Amit Langote wrote:
> > On Thu, Oct 8, 2020 at 9:35 PM Amit Langote wrote:
> >> On Wed, Oct 7, 2020 at 9:07 PM Heikki Linnakangas wrote:
> >>> On 07/10/2020 12:50, Amit Langote wrot
I think we should remove this duplicative logic and return the
generated steps in a list from this function, which the code in
gen_partprune_steps_internal() then "combines" using an INTERSECT
step. See attached a patch to show what I mean.
--
Amit Langote
EDB: http://www.enterprisedb.com
partprune-code-tweaks.patch
Description: Binary data
On Wed, Oct 7, 2020 at 9:07 PM Heikki Linnakangas wrote:
> On 07/10/2020 12:50, Amit Langote wrote:
> > On Tue, Oct 6, 2020 at 12:45 AM Heikki Linnakangas wrote:
> >> It would be better to set it in make_modifytable(), just
> >> after calling PlanDirectModify().
>
On Thu, Oct 8, 2020 at 2:58 PM Amit Langote wrote:
> On Wed, Oct 7, 2020 at 12:51 AM Tom Lane wrote:
> > Ashutosh Bapat writes:
> > > On Mon, Oct 5, 2020 at 11:59 PM Tom Lane wrote:
> > >> ... we could avoid the growth in eclass members for large partition sets
&g
hat should be bought
> back by having fewer EC members to compare to. In any event, testing
> a working patch will be a lot more illuminating than speculation.
Agreed.
--
Amit Langote
EDB: http://www.enterprisedb.com
[1]
https://www.postgresql.org/message-id/CAApHDvrEXcadNYAAdq6RO0eKZUG6rRHXJGAbpzj8y432gCD9bA%40mail.gmail.com
Append (cost=0.29..83.15 rows=10 width=12)
Subplans Removed: 9
-> Index Scan using bar_1_a_idx on bar_1 (cost=0.29..8.31 rows=1 width=12)
Index Cond: (a = $1)
(4 rows)
As you can see, the cost of the generic plan is 83.13 which is 8.31 *
10, even though only one partition is actually scanned, so the actual
cost is the same as a custom plan. The planner fails to consider that
the those 9 subplans will be removed during execution.
--
Amit Langote
EDB: http://www.enterprisedb.com
Hekki,
Thanks a lot for the review!
On Tue, Oct 6, 2020 at 12:45 AM Heikki Linnakangas wrote:
> On 13/07/2020 08:47, Amit Langote wrote:
> > It's been over 11 months since there was any significant commentary on
> > the contents of the patches themselves, so perhaps I shoul
On Sat, Oct 3, 2020 at 8:15 PM Tomas Vondra wrote
> On Sat, Oct 03, 2020 at 11:42:21AM +0900, Amit Langote wrote:
> >On Fri, Oct 2, 2020 at 11:32 PM David G. Johnston
> > wrote:
> >> On Friday, October 2, 2020, Amit Langote
> >> wrote:
> >>>
> >
On Fri, Oct 2, 2020 at 11:32 PM David G. Johnston
wrote:
> On Friday, October 2, 2020, Amit Langote wrote:
>>
>>
>> Reporter on that thread says that the last update should have failed
>> and I don't quite see a workable alternative to that.
>
>
> To
See attached a patch for that. I'm not terribly
happy with the error and details messages though:
update parent set id = 15;
ERROR: cannot move row being updated to another partition
DETAIL: Moving the row may cause a foreign key involving the source
partition to be violated.
Thoughts
s is a correct direction to
> go.
That's an interesting idea, that is, to try to do this totally outside
the planner. When I was thinking about this a little while ago, I was
trying to find a way to adjust the cost of the plan in the planner
itself by looking at the runtime pruning
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
On Thu, Sep 24, 2020 at 2:26 PM Amit Langote wrote:
> On Thu, Sep 24, 2020 at 1:54 PM Michael Paquier wrote:
> > On Thu, Sep 24, 2020 at 04:25:24AM +0900, Etsuro Fujita wrote:
> > > Sorry, my thought was vague. To store xmin/xmax/cmin/cmax into a
> > > given
On Tue, Sep 29, 2020 at 2:01 AM Tom Lane wrote:
> Amit Langote writes:
> > On Fri, Sep 25, 2020 at 12:02 AM Tom Lane wrote:
> >> Well, I agree with Peter to that extent, but my opinion is that *none*
> >> of these cases ought to be errors. What we're doing her
On Fri, Sep 25, 2020 at 12:02 AM Tom Lane wrote:
> [ cc'ing Peter, since his opinion seems to have got us here in the first
> place ]
>
> Amit Langote writes:
> > On Thu, Sep 24, 2020 at 7:19 AM Tom Lane wrote:
> >> However, while I was looking at
On Thu, Sep 24, 2020 at 7:30 PM Etsuro Fujita wrote:
> On Thu, Sep 24, 2020 at 2:47 PM Amit Langote wrote:
> > On Thu, Sep 24, 2020 at 4:25 AM Etsuro Fujita
> > wrote:
> > > Yeah, but for the other issue, I started thinking that we should just
> > > forbid refe
n the
DETAIL message what the expression's collation is, like as follows:
create table test_part_coll_cast2 partition of test_part_coll_posix
for values from (name 's') to ('z');
+ERROR: collation of partition bound value for column "a" does not
match partition key collation "POSIX"
+LINE 1: ...ion of test_part_coll_posix for values from (name 's') to ('...
+ ^
+DETAIL: The collation of partition bound value is "C".
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
[1]
https://www.postgresql.org/message-id/04661508-b6f5-177e-6f6b-c4cb8426b9f0%402ndquadrant.com
partition-bound-collation-check.patch
Description: Binary data
On Thu, Sep 24, 2020 at 4:25 AM Etsuro Fujita wrote:
> On Wed, Sep 23, 2020 at 10:12 PM Amit Langote wrote:
> > On Sun, Sep 20, 2020 at 11:41 PM Etsuro Fujita
> > wrote:
> > > On Mon, Sep 14, 2020 at 10:45 PM Amit Langote
> > > wrote:
> > &
it seems that it's
because the bot is trying to apply the patch version meant for v11
branch onto HEAD branch. I just checked that the patches apply
cleanly to their respective branches.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
Hi Alvaro,
Sorry I totally failed to see the v2 you had posted and a couple of
other emails where you mentioned the issues I brought up.
On Thu, Sep 24, 2020 at 12:23 AM Alvaro Herrera
wrote:
> On 2020-Sep-23, Amit Langote wrote:
> I suspect that we don't really need this defensive
On Wed, Sep 23, 2020 at 10:22 PM Ashutosh Bapat
wrote:
> On Wed, 23 Sep 2020 at 14:41, Amit Langote wrote:
> Setting this CF entry as "RFC". Thanks.
Great, thanks for your time on this.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
Fujita-san,
On Sun, Sep 20, 2020 at 11:41 PM Etsuro Fujita wrote:
> On Mon, Sep 14, 2020 at 10:45 PM Amit Langote wrote:
> > On Mon, Sep 14, 2020 at 5:56 PM Etsuro Fujita
> > wrote:
> > > IIUC, I think two issues are discussed in the thread [1]: (a) there is
> >
prologue of
> partition_rbound_cmp() so I am not sure if this comment is really required.
> For example, we aren't adding a comment here
> + overlap_location = ((PartitionRangeDatum *)
> + list_nth(spec->upperdatums, -cmpval - 1))->location;
In the attached updated patch, I have tried
able route (2) to d_listp2, the
detached partition. It's the bug mentioned above that causes the
executor's partition descriptor build to falsely fail to include the
detached partition.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Fri, Sep 18, 2020 at 12:30 PM Amit Kapila wrote:
> On Thu, Sep 17, 2020 at 6:46 PM Amit Langote wrote:
> >
> > Thanks for taking a look.
> >
> > On Thu, Sep 17, 2020 at 9:20 PM Amit Kapila wrote:
> > > On Thu, Sep 17, 2020 a
Thanks for taking a look.
On Thu, Sep 17, 2020 at 9:20 PM Amit Kapila wrote:
> On Thu, Sep 17, 2020 at 3:22 PM Amit Langote wrote:
> >
>
> /*-
> * relation.c
> - *PostgreSQL logical replicatio
Hi,
$subect needs fixing, because right now it says just:
/*-
* relation.c
*PostgreSQL logical replication
Attached find a patch to expand that a little bit.
--
Amit Langote
EnterpriseDB: http
ngeBound *probe, bool *is_equal, int32
> *cmpval)
>
> Please update the prologue explaining the new argument.
Done. Actually, I noticed that *is_equal was unused in this
function's only caller. *cmpval == 0 already gives that, so removed
is_equal parameter.
Attached updated vers
On Thu, Sep 17, 2020 at 3:40 AM Tom Lane wrote:
> Amit Langote writes:
> > Updated patch attached.
>
> Pushed with a little bit of fooling about.
Thank you.
> After looking at the
> git history, I saw that the Assert we were wondering about used to
> be just "As
On Thu, Sep 10, 2020 at 6:57 PM Andrey V. Lepikhov
wrote:
> On 9/9/20 5:51 PM, Amit Langote wrote:
> > On Wed, Sep 9, 2020 at 6:42 PM Alexey Kondratov
> > wrote:
> >> And InitResultRelInfo() may set ri_usesMultiInsert to false by default,
> >> since it's
On Wed, Sep 16, 2020 at 2:41 AM Tom Lane wrote:
> Amit Langote writes:
> > On Tue, Sep 15, 2020 at 7:28 AM Tom Lane wrote:
> >> AFAICS, it is utterly silly for InitResultRelInfo to be forcing
> >> a partition qual to be computed when we might not n
On Tue, Sep 15, 2020 at 10:47 PM Tom Lane wrote:
> Amit Langote writes:
> > On Tue, Sep 15, 2020 at 9:09 AM Tom Lane wrote:
> >> I wrote a quick patch for this part. It seems pretty safe and probably
> >> could be back-patched without fear.
>
> > The patc
like most of the places reading it are
> just interested in non-nullness; can't those be nuked from
> orbit in favor of testing rel->rd_rel->relispartition?
Yeah, makes sense. Please see attached a patch to do that.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
remove-ri_PartitionCheck.patch
Description: Binary data
lhassubclass,
> which is not only simpler and less likely to need future changes, but
> is able to save a scan of pg_inherits in a lot more cases.)
+1
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Mon, Sep 14, 2020 at 5:56 PM Etsuro Fujita wrote:
> On Mon, Sep 14, 2020 at 3:53 PM Amit Langote wrote:
> > On Sat, Sep 12, 2020 at 5:42 AM Alvaro Herrera
> > wrote:
> > > I noticed that this bugfix has stalled, probably because the other
> > > bugfix has a
However, given that we have to pass the *new* tuple to the projection,
not the old one, we will need a "clean" way to transfer its (the new
tuple's) system columns into the source partition's tuple slot. The
sketch I gave upthread of what that could look like was not like
Hello,
On Fri, Aug 7, 2020 at 9:26 PM Amit Langote wrote:
> On Tue, Aug 4, 2020 at 3:15 PM Amit Langote wrote:
> > On Sat, Aug 1, 2020 at 4:46 AM Robert Haas wrote:
> > > On Fri, Jun 26, 2020 at 8:36 AM Amit Langote
> > > wrote:
> > > > 0001 and 0002
ultRelInfo() to a separate
> routine.
Interesting idea. Maybe better to have a separate routine like Alexey says.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Tue, May 12, 2020 at 10:28 AM Amit Langote wrote:
>
> On Tue, May 12, 2020 at 8:51 AM Bruce Momjian wrote:
> > On Fri, May 8, 2020 at 12:07:09PM +0900, Amit Langote wrote:
> > > I have attached a patch with my suggestions above.
> >
> > OK, I slightly m
On Wed, Sep 9, 2020 at 7:41 AM Alvaro Herrera wrote:
> On 2020-Sep-08, Amit Langote wrote:
>
> > Yeah, we need to make sure that ExecPartitionCheck gets a slot whose
> > TupleDesc matches the partition's. Actually we do have such dedicated
> > slots for pa
Hi Alexey,
On Tue, Sep 8, 2020 at 6:29 PM Alexey Kondratov
wrote:
> On 2020-09-08 10:34, Amit Langote wrote:
> > Any thoughts on the taking out the refactoring changes out of the main
> > patch as I suggested?
> >
>
> +1 for splitting the patch. It was rather diffic
> v.7 (in attachment) fixes this problem.
> I also accepted Amit's suggestion to rename all fdwapi routines such as
> ForeignCopyIn to *ForeignCopy.
Any thoughts on the taking out the refactoring changes out of the main
patch as I suggested?
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
h.
Attached is the latest patch including those changes. Also, I have
merged your earlier "fixes" patch and updated the isolation test to
exercise a case with sub-partitioned default partition, as well as
varying attribute numbers. Patch applies as-is to both HEAD and v13
branches, but
alid as any other ResultRelInfo as far I can see. I said
"minimally valid" because a fully-valid partition ResultRelInfo is one
made by ExecInitPartitionInfo(), which I avoided to use here thinking
that would be overkill.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Thu, Sep 3, 2020 at 6:50 PM Amit Langote wrote:
>
> Hi,
>
> Starting a new thread to discuss a bug related to $subject that Hao Wu
> reported on thread titled "ALTER TABLE .. DETACH PARTITION
> CONCURRENTLY" [1]. I have been able to reproduce the bug using steps
&
sed on the new feature.
Subject: default partition and concurrent attach partition
https://www.postgresql.org/message-id/CA%2BHiwqFqBmcSSap4sFnCBUEL_VfOMmEKaQ3gwUhyfa4c7J_-nA%40mail.gmail.com
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
first and then the
attach by session 1, then you will correctly get this error from the
attach command:
ERROR: updated partition constraint for default partition
"tpart_default" would be violated by some row
Attached is a patch to fix things on the insert side.
--
Amit L
On Mon, Aug 24, 2020 at 4:18 PM Amit Langote wrote:
> I would
Oops, thought I'd continue writing, but hit send before actually doing
that. Please ignore.
I have some comments on v6, which I will share later this week.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
Hi Andrey,
On Fri, Aug 21, 2020 at 9:19 PM Andrey Lepikhov
wrote:
> On 8/7/20 2:14 PM, Amit Langote wrote:
> > I was playing around with v5 and I noticed an assertion failure which
> > I concluded is due to improper setting of ri_usesBulkModify. You can
> > reproduc
On Thu, Aug 20, 2020 at 10:50 AM Amit Langote wrote:
On Thu, Aug 20, 2020 at 3:06 AM Alvaro Herrera
wrote:
> > On 2020-Aug-19, Amit Langote wrote:
> > > On Thu, Aug 6, 2020 at 4:25 PM kato-...@fujitsu.com
> > > wrote:
> > > > On Wednesday, August 5, 2020 9:
Hi,
On Thu, Aug 20, 2020 at 3:06 AM Alvaro Herrera wrote:
> On 2020-Aug-19, Amit Langote wrote:
> > On Thu, Aug 6, 2020 at 4:25 PM kato-...@fujitsu.com
> > wrote:
> > > On Wednesday, August 5, 2020 9:43 AM I wrote:
> > > > I'll report the result before
ng rebuilt
repeatedly as would happen before this commit in Kato-san's case,
because it moves RelationBuildPartitionDesc out of the relcache flush
code path.
So, the OOM situation that Kato-san original reported should not occur
as of v13.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Fri, Aug 7, 2020 at 10:45 PM Etsuro Fujita wrote:
> On Mon, Aug 3, 2020 at 4:39 PM Amit Langote wrote:
> > On Mon, Aug 3, 2020 at 2:54 PM Amit Langote wrote:
> > > By the way, you'll need two adjustments to even get this example
> > > working, one of which i
On Tue, Aug 4, 2020 at 3:15 PM Amit Langote wrote:
> On Sat, Aug 1, 2020 at 4:46 AM Robert Haas wrote:
> > On Fri, Jun 26, 2020 at 8:36 AM Amit Langote
> > wrote:
> > > 0001 and 0002 are preparatory patches.
> >
> > I read through these patches a bit but it
On Mon, Aug 3, 2020 at 8:38 PM Amit Langote wrote:
> On Wed, Jul 29, 2020 at 5:36 PM Andrey V. Lepikhov
> wrote:
> > > Will send more comments after reading the v5 patch.
> > >
> > Ok. I'll be waiting for the end of your review.
>
> Sorry about the la
On Fri, Aug 7, 2020 at 1:21 PM Justin Pryzby wrote:
> On Fri, Aug 07, 2020 at 01:13:51PM +0900, Amit Langote wrote:
> > On Fri, Aug 7, 2020 at 1:05 PM Justin Pryzby wrote:
> > > On Fri, Aug 07, 2020 at 12:16:11PM +0900, Amit Langote wrote:
> > > > Curiously, Just
On Fri, Aug 7, 2020 at 1:05 PM Justin Pryzby wrote:
>
> On Fri, Aug 07, 2020 at 12:16:11PM +0900, Amit Langote wrote:
> > Curiously, Justin mentioned upthread that the crash occurred during
> > BIND of a prepared query, so it better had been that a custom plan was
> > b
;s no proof that it isn't
wrong. For example, I can see that pinfo->relid_map[pinfo->nparts]
can be accessed with my patch which is not correct.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Thu, Aug 6, 2020 at 2:30 AM Tom Lane wrote:
> Amit Langote writes:
> > The attached patch should fix that.
>
> I don't like this patch at all though; I do not think it is being nearly
> careful enough to ensure that it's matched the surviving relation OIDs
&g
efix(), which leads to an assertion failure.
That analysis is spot on.
> Attached is a patch for fixing this issue.
I have looked at the patch and played around with it using the
regression tests you've added recently. I was not able to find any
results that looked surprising.
Thanks again.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
On Wed, Aug 5, 2020 at 10:04 AM Justin Pryzby wrote:
> On Wed, Aug 05, 2020 at 09:53:44AM +0900, Amit Langote wrote:
> > On Wed, Aug 5, 2020 at 9:52 AM Amit Langote wrote:
> > > On Wed, Aug 5, 2020 at 9:32 AM Justin Pryzby wrote:
> > > > On Wed, Aug 05, 2020 at
On Wed, Aug 5, 2020 at 9:52 AM Amit Langote wrote:
> On Wed, Aug 5, 2020 at 9:32 AM Justin Pryzby wrote:
> > On Wed, Aug 05, 2020 at 09:26:20AM +0900, Amit Langote wrote:
> > > On Wed, Aug 5, 2020 at 12:11 AM Justin Pryzby
> > > wrote:
> > > >
> >
}
Assert(pd_idx == pinfo->nparts);
}
where it crashes due to having relid_map[] and partdesc->oids[] that
look like this:
(gdb) p *pinfo->relid_map@pinfo->nparts
$3 = {0, 74106, 74109}
(gdb) p *partdesc->oids@partdesc->nparts
$6 = {74103, 74106, 74109, 74112}
The 0 in relid_map matches with nothing in partdesc->oids with the
loop ending without moving forward in the relid_map array, causing the
Assert to fail.
The attached patch should fix that.
--
Amit Langote
EnterpriseDB: http://www.enterprisedb.com
PartitionedRelPruneInfo-relid_map-pruned-parts-zero-fix.patch
Description: Binary data
On Sat, Aug 1, 2020 at 4:46 AM Robert Haas wrote:
> On Fri, Jun 26, 2020 at 8:36 AM Amit Langote wrote:
> > 0001 and 0002 are preparatory patches.
>
> I read through these patches a bit but it's really unclear what the
> point of them is. I think they need better com
801 - 900 of 2399 matches
Mail list logo
