On 2019/04/04 4:45, Jesper Pedersen wrote:
> On 4/3/19 1:52 PM, Alvaro Herrera wrote:
>> Pushed, many thanks Amit and Jesper for reviewing.
>>
>
> Thank you for working on this feature.
+1, thank you.
Regards,
Amit
On 4/3/19 1:52 PM, Alvaro Herrera wrote:
Pushed, many thanks Amit and Jesper for reviewing.
Thank you for working on this feature.
Best regards,
Jesper
Pushed, many thanks Amit and Jesper for reviewing.
--
Álvaro Herrerahttps://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Apr 2, 2019 at 11:07 AM Tom Lane wrote:
> The problem this patch is running into is that we'd like to make the
> validity of dropping a table partition depend on whether there are
> individual rows in some other table that FK-reference rows in the target
> partition. That's just
Alvaro Herrera writes:
> On 2019-Mar-29, Robert Haas wrote:
>> I don't know why dependency.c doesn't handle this internally. If I
>> say that I want to delete a list of objects, some of which can't be
>> dropped without dropping certain other things, dependency.c ought to
>> be able to suspend
Hi,
On 4/1/19 4:03 PM, Alvaro Herrera wrote:
I'm satisfied with this patch now, so I intend to push early tomorrow.
Passes check-world, and my tests.
Best regards,
Jesper
Hi Alvaro,
On 2019/04/02 5:03, Alvaro Herrera wrote:
> On 2019-Mar-29, Jesper Pedersen wrote:
>
>> I ran my test cases for this feature, and havn't seen any issues.
>>
>> Therefore I'm marking 1877 as Ready for Committer. If others have additional
>> feedback feel free to switch it back.
>
>
On 2019-Mar-29, Jesper Pedersen wrote:
> I ran my test cases for this feature, and havn't seen any issues.
>
> Therefore I'm marking 1877 as Ready for Committer. If others have additional
> feedback feel free to switch it back.
Thanks!
I found two issues today. One, server side, is that
Hi,
On 3/29/19 12:29 PM, Alvaro Herrera wrote:
On 2019-Mar-29, Jesper Pedersen wrote:
Maybe the "(" / ")" in the CASCADE description should be removed from
ref/drop_table.sgml as part of this patch.
I'm not sure what text you propose to remove?
Just the attached.
Should catalogs.sgml be
On 2019-Mar-29, Jesper Pedersen wrote:
> Thanks !
>
> Maybe the "(" / ")" in the CASCADE description should be removed from
> ref/drop_table.sgml as part of this patch.
I'm not sure what text you propose to remove?
> Should catalogs.sgml be updated for this case ?
I'm not adding any new
Hi,
On 3/29/19 11:22 AM, Alvaro Herrera wrote:
On 2019-Mar-29, Jesper Pedersen wrote:
Could expand a bit on the change to DEPENDENCY_INTERNAL instead of
DEPENDENCY_PARTITION_PRI / DEPENDENCY_PARTITION_SEC ?
The PARTITION dependencies work in a way that doesn't do what we want.
Admittedly,
On 2019-Mar-29, Jesper Pedersen wrote:
> Could expand a bit on the change to DEPENDENCY_INTERNAL instead of
> DEPENDENCY_PARTITION_PRI / DEPENDENCY_PARTITION_SEC ?
The PARTITION dependencies work in a way that doesn't do what we want.
Admittedly, neither does INTERNAL, but at least it's less
Hi Alvaro,
On 3/28/19 2:59 PM, Alvaro Herrera wrote:
I ended up revising the dependencies that we give to the constraint in
the partition -- instead of giving it partition-type dependencies, we
give it an INTERNAL dependency. Now when you request to drop the
partition, it says this:
create
On 2019-Mar-29, Robert Haas wrote:
> On Wed, Mar 20, 2019 at 11:58 AM Alvaro Herrera
> wrote:
> > constraint is dropped. I can only think of ugly data structure to
> > support this, and adding enough hooks in dependency.c to support this is
> > going to be badly received.
>
> I don't know why
On Thu, Mar 28, 2019 at 2:59 PM Alvaro Herrera wrote:
> I ended up revising the dependencies that we give to the constraint in
> the partition -- instead of giving it partition-type dependencies, we
> give it an INTERNAL dependency. Now when you request to drop the
> partition, it says this:
>
>
On Wed, Mar 20, 2019 at 11:58 AM Alvaro Herrera
wrote:
> constraint is dropped. I can only think of ugly data structure to
> support this, and adding enough hooks in dependency.c to support this is
> going to be badly received.
I don't know why dependency.c doesn't handle this internally. If I
On 2019-Mar-18, Alvaro Herrera wrote:
> A pretty silly bug remains here. Watch:
>
> create table pk (a int primary key) partition by list (a);
> create table pk1 partition of pk for values in (1);
> create table fk (a int references pk);
> insert into pk values (1);
> insert into fk values (1);
Hi,
On 2019/03/27 8:27, Alvaro Herrera wrote:
> On 2019-Mar-26, Alvaro Herrera wrote:
>
>> Thanks for the thorough testing and bug analysis! It was spot-on. I've
>> applied your two proposed fixes, as well as added a new test setup that
>> covers both these bugs. The attached set is rebased
Hi,
On 3/26/19 5:39 PM, Alvaro Herrera wrote:
As I said before, I'm thinking of getting rid of the whole business of
checking partitions on the referenced side of an FK at DROP time, and
instead jut forbid the DROP completely if any FKs reference an ancestor
of that partition.
Will that allow
On 2019-Mar-26, Alvaro Herrera wrote:
> Thanks for the thorough testing and bug analysis! It was spot-on. I've
> applied your two proposed fixes, as well as added a new test setup that
> covers both these bugs. The attached set is rebased on 7c366ac969ce.
Attached is rebased on 126d63122232.
Hello Amit
On 2019-Mar-26, Amit Langote wrote:
> + Oid objectClass = getObjectClass(thisobj);
>
> I guess you meant to use ObjectClass, not Oid here.
Absolutely.
> Tested 0002 a bit more and found some problems.
Thanks for the thorough testing and bug
Hi Amit,
On 3/26/19 2:06 AM, Amit Langote wrote:
Wouldn't you get the same numbers on HEAD too? IOW, I'm not sure how the
patch here, which seems mostly about getting DDL in order to support
foreign keys on partitioned tables, would have affected the result of this
benchmark. Can you clarify
Hi Alvaro,
On 2019/03/22 6:54, Alvaro Herrera wrote:
> Here's v7;
Needs rebasing on top of 940311e4bb3.
0001:
+ Oid objectClass = getObjectClass(thisobj);
I guess you meant to use ObjectClass, not Oid here.
Tested 0002 a bit more and found some problems.
Hi Jesper,
On 2019/03/22 22:01, Jesper Pedersen wrote:
> Hi Alvaro,
>
> On 3/21/19 6:18 PM, Alvaro Herrera wrote:
>> On 2019-Mar-21, Jesper Pedersen wrote:
>>> pgbench -M prepared -f select.sql
>>>
>>> I'm seeing 82.64% spent in GetCachedPlan(). plan_cache_mode is auto.
>>
>> Hmm, I can't
Hi Alvaro,
On 3/21/19 6:18 PM, Alvaro Herrera wrote:
On 2019-Mar-21, Jesper Pedersen wrote:
pgbench -M prepared -f select.sql
I'm seeing 82.64% spent in GetCachedPlan(). plan_cache_mode is auto.
Hmm, I can't reproduce this at all ... I don't even see GetCachedPlan
in the profile. Do
Hi Alvaro,
On 3/21/19 4:49 PM, Alvaro Herrera wrote:
I think the attached is a better solution, which I'll go push shortly.
I see you pushed this, plus 0002 as well.
Thanks !
Best regards,
Jesper
Hi Jesper
On 2019-Mar-21, Jesper Pedersen wrote:
> running
>
> pgbench -M prepared -f select.sql
>
> I'm seeing 82.64% spent in GetCachedPlan(). plan_cache_mode is auto.
Hmm, I can't reproduce this at all ... I don't even see GetCachedPlan
in the profile. Do you maybe have some
On 2019-Mar-18, Alvaro Herrera wrote:
> A pretty silly bug remains here. Watch:
>
> create table pk (a int primary key) partition by list (a);
> create table pk1 partition of pk for values in (1);
> create table fk (a int references pk);
> insert into pk values (1);
> insert into fk values (1);
On 2019-Mar-21, Alvaro Herrera wrote:
> I figured this out. It's actually a bug in pg11, whereby we're setting
> a dependency wrongly. If you try to do pg_describe_object() the
> pg_depend entries for tables set up the way the regression test does it,
> it'll fail with a "cache lookup failed
On 2019-Mar-18, Alvaro Herrera wrote:
> > I'm getting a failure in the pg_upgrade test:
> >
> > --
> > +-- Name: pk5 pk5_pkey; Type: CONSTRAINT; Schema: regress_fk; Owner:
> > jpedersen
> > +--
> > +
> > +ALTER TABLE ONLY regress_fk.pk5
> > +ADD CONSTRAINT pk5_pkey PRIMARY KEY (a);
> > +
>
Hi Alvaro,
On 3/18/19 6:02 PM, Alvaro Herrera wrote:
I spent a few hours studying this and my conclusion is the opposite of
yours: we should make addFkRecurseReferencing the recursive one, and
CloneFkReferencing a non-recursive caller of that. So we end up with
both addFkRecurseReferenced and
On 2019-Mar-14, Robert Haas wrote:
> On Thu, Mar 14, 2019 at 3:36 PM Alvaro Herrera
> wrote:
> > In any case, since the RI
> > queries are run via SPI, any unnecessary partitions should get pruned by
> > partition pruning based on each partition's constraint. So I'm not so
> > certain that
On 2019-Mar-19, Amit Langote wrote:
> On Tue, Mar 19, 2019 at 8:49 PM Alvaro Herrera
> wrote:
> > On 2019-Mar-19, Amit Langote wrote:
> >
> > > Will it suffice or be OK if we skipped invoking the pre-drop callback for
> > > objects that are being "indirectly" dropped? I came up with the
On Tue, Mar 19, 2019 at 8:49 PM Alvaro Herrera wrote:
> On 2019-Mar-19, Amit Langote wrote:
>
> > Will it suffice or be OK if we skipped invoking the pre-drop callback for
> > objects that are being "indirectly" dropped? I came up with the attached
> > patch to resolve this problem, if that idea
On 2019-Mar-19, Amit Langote wrote:
> Will it suffice or be OK if we skipped invoking the pre-drop callback for
> objects that are being "indirectly" dropped? I came up with the attached
> patch to resolve this problem, if that idea has any merit. We also get
> slightly better error message as
Hi,
Thanks for updating the patch. I'll reply to other parts separately.
On 2019/03/19 7:02, Alvaro Herrera wrote:
> A pretty silly bug remains here. Watch:
>
> create table pk (a int primary key) partition by list (a);
> create table pk1 partition of pk for values in (1);
> create table fk
On 2019-Feb-28, Amit Langote wrote:
> I'd like to hear your thoughts on some suggestions to alter the structure
> of the reorganized code around foreign key addition/cloning. With this
> patch adding support for foreign keys to reference partitioned tables, the
> code now has to consider various
Hi Jesper
On 2019-Mar-01, Jesper Pedersen wrote:
> I'm getting a failure in the pg_upgrade test:
>
> --
> +-- Name: pk5 pk5_pkey; Type: CONSTRAINT; Schema: regress_fk; Owner:
> jpedersen
> +--
> +
> +ALTER TABLE ONLY regress_fk.pk5
> +ADD CONSTRAINT pk5_pkey PRIMARY KEY (a);
> +
> +
> +--
Hi Amit
On 2019-Mar-18, Amit Langote wrote:
> On 2019/03/15 2:31, Alvaro Herrera wrote:
> > Once I was finished, fixed bugs and tested it, I realized that that was
> > a stupid thing to have done -- because THOSE ARE DIFFERENT CONSTRAINTS.
> > When you say "fk (a) references pk1" you're saying
Hi,
On 2019/03/15 2:31, Alvaro Herrera wrote:
> Once I was finished, fixed bugs and tested it, I realized that that was
> a stupid thing to have done -- because THOSE ARE DIFFERENT CONSTRAINTS.
> When you say "fk (a) references pk1" you're saying that all the values
> in fk(a) must appear in pk1.
On Thu, Mar 14, 2019 at 3:36 PM Alvaro Herrera wrote:
> Well, I suppose that can be implemented as an optimization on top of
> what we have, but I think that we should first get this feature right,
> and later we can see about improving it.
Sure, not arguing with that.
> In any case, since the
On 2019-Mar-14, Robert Haas wrote:
> On Thu, Mar 14, 2019 at 1:40 PM Alvaro Herrera
> wrote:
> > When you say "fk (a) references pk1" you're saying that all the values
> > in fk(a) must appear in pk1. OTOH when you say "fk references pk" you
> > mean that the values might appear anywhere in
On Thu, Mar 14, 2019 at 1:40 PM Alvaro Herrera wrote:
> Once I was finished, fixed bugs and tested it, I realized that that was
> a stupid thing to have done -- because THOSE ARE DIFFERENT CONSTRAINTS.
This made me laugh.
> When you say "fk (a) references pk1" you're saying that all the values
On 2019-Feb-28, Amit Langote wrote:
Hello
> In the following case:
>
> create table pk (a int primary key) partition by list (a);
> create table pk1 (a int primary key);
> create table fk (a int references pk1);
>
> -- adds FK referencing pk1 via ATAddForeignKeyConstraint recursion
> alter
Hi Alvaro,
On 2/28/19 1:28 PM, Alvaro Herrera wrote:
Rebased to current master. I'll reply later to handle the other issues
you point out.
Applying with hunks.
With 0003 using
export CFLAGS="-DCOPY_PARSE_PLAN_TREES -O0 -fno-omit-frame-pointer" &&
CC="ccache gcc" ./configure --prefix
On 2019-Feb-28, Amit Langote wrote:
> Hi Alvaro,
>
> I looked at the latest patch and most of the issues/bugs that I was going
> to report based on the late January version of the patch seem to have been
> taken care of; sorry that I couldn't post sooner which would've saved you
> some time.
Hi Alvaro,
I looked at the latest patch and most of the issues/bugs that I was going
to report based on the late January version of the patch seem to have been
taken care of; sorry that I couldn't post sooner which would've saved you
some time. The patch needs to be rebased on top of ff11e7f4b9
Here's another rebase. The code is structured somewhat differently from
the previous one, mostly because of the backpatched bugfixes, but also
because of the changes in dependency handling.
I think there's also at least one more bugfix to backpatch (included in
0003 here), related to whether
On Tue, Jan 22, 2019 at 06:45:48PM -0300, Alvaro Herrera wrote:
> Yes, very soon -- right now, in fact :-)
This needs a rebase. Moved to next CF, waiting on author.
--
Michael
signature.asc
Description: PGP signature
Hi Alvaro,
On 2018/12/01 4:12, Alvaro Herrera wrote:
> Here's a more credible version of this patch series.
Are you going to post rebased patches soon?
Thanks,
Amit
Hi,
On 1/7/19 1:07 PM, Jesper Pedersen wrote:
While I'm still looking at 0004 - should we have a test that updates one
of the constraints of fk to another partition in pk ?
In addition:
* Document pre_drop_class_check()
* I think index_get_partition() would be more visible in partition.c
*
Hi,
On 11/30/18 2:12 PM, Alvaro Herrera wrote:
Here's a more credible version of this patch series.
The patch series applies with hunks, and passes check-world.
No comments for 0001, 0002, 0003 and 0005.
While I'm still looking at 0004 - should we have a test that updates one
of the
Here's a more credible version of this patch series.
0001 refactors some duplicative code that interprets a pg_constraint row
for a foreign key back into a Constraint node. Only what is necessary
for current features is read.
0002 moves some code that I added in 3de241dba86f to
>
>
> > 1. it seems that we will continue to to per-row RI checks for inserts and
> > updates. However, there already exists a bulk check in
> RI_Initial_Check().
> > Could we modify this bulk check to do RI checks on a per-statement basis
> > rather than a per-row basis?
>
> One of the goals when
On 2018-Nov-05, Corey Huinker wrote:
> This is an important and much needed feature!
I agree :-)
> Based on my extremely naive reading of this code, I have two perhaps
> equally naive questions:
>
> 1. it seems that we will continue to to per-row RI checks for inserts and
> updates. However,
On Fri, Nov 2, 2018 at 7:42 PM Alvaro Herrera
wrote:
> Here's a patch to allow partitioned tables to be referenced by foreign
> keys. Current state is WIP, but everything should work; see below for
> the expected exception.
>
> The design is very simple: have one pg_constraint row for each
Oh, I forgot to mention one thing. When creating a constraint, an index
OID is normally given. I'm not sure what is this for. In the patch
it's a little convoluted to get the correct index OID, so I'm just
passing InvalidOid. Things work nonetheless. I wonder if we shouldn't
just do away with
Here's a patch to allow partitioned tables to be referenced by foreign
keys. Current state is WIP, but everything should work; see below for
the expected exception.
The design is very simple: have one pg_constraint row for each partition
on each side, each row pointing to the topmost table on
58 matches
Mail list logo
