Re: GSoC 2017: Foreign Key Arrays

From: Mark Rofail <markm(dot)rofail(at)gmail(dot)com>
To: Andreas Karlsson <andreas(at)proxel(dot)se>
Cc: Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, David Steele <david(at)pgmasters(dot)net>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Erik Rijkers <er(at)xs4all(dot)nl>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, pgsql-hackers-owner(at)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Hans-Jürgen Schönig <hs(at)cybertec(dot)at>
Subject: Re: GSoC 2017: Foreign Key Arrays
Date: 2017-11-13 11:32:38
Message-ID: CAJvoCuvFePiuw7p_VeTZ=OXeT9MifC0PEc6PqieV69MCrqYwLQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>
> >2) To me it was not obvious that ON DELETE CASCADE would delete
>>
> the whole rows rather than delete the members from the array, and
>> this kind of misunderstanding can lead to pretty bad surprises in
>> production. I am leaning towards not supporting CASCADE.
>>
>> I would say so too, maybe we should remove ON DELETE CASCADE until we
>> have supported all remaining actions.
>
> Delete Cascade support is now removed

> == The @>> operator
>> I would argue that allocating an array of datums and building an array
>> would have the same complexity
>>
>
> I am not sure what you mean here. Just because something has the same
> complexity does not mean there can't be major performance differences.

I have spend a lot of time working on this operator and would like to
benefit from it. How should I go about this ? Start a new patch ?

> = New stuff
>
Everything is now working correctly

== Functional review
>
> >1) MATCH FULL does not seem to care about NULLS in arrays. In the
> example below I expected both inserts into the referring table to fail.
>
>
> It seems in your example the only failed case was: INSERT INTO fk VALUES
> (NULL, '{1}');
> which shouldn't work, can you clarify this?

I think that if you use MATH FULL the query should fail if you have a NULL
> in the array.

== Code review
>
> >I think the code in RI_Initial_Check() would be cleaner if you
> used "CROSS JOIN LATERAL unnest(col)" rather than having unnest() in
> the target list. This way you would not need to rename all columns
> and the code paths for the array case could look more like the code
> path for the normal case.
>
> Can you clarify what you mean a bit more?

I think the code would look cleaner if you generate the following query:
> SELECT fk.x, fk.ys FROM ONLY t2 fk CROSS JOIN LATERAL
> pg_catalog.unnest(ys) a2 (v) LEFT OUTER JOIN ONLY t1 pk ON pk.x = fk.x AND
> pk.y = a2.v WHERE [...]
> rather than:
> SELECT fk.k1, fk.ak2 FROM (SELECT x k1, pg_catalog.unnest(ys) k2, ys ak2
> FROM ONLY t2) fk LEFT OUTER JOIN ONLY t1 pk ON pk.x = fk.k1 AND pk.y =
> fk.k2 WHERE [...]

So the two main issues we remain to resolve are MATCH FULL and the
RI_Initial_Check() query refactoring. The problem is that I am not one of
the original authors and have not touched this part of the code.
I understand the problem but it will take some time for me to understand
how to resolve everything.

Best Regards,
Mark Rofail

Attachment Content-Type Size
Array-ELEMENT-foreign-key-v5.3(fixed).patch text/x-patch 119.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Golub 2017-11-13 12:15:00 Re: proposal: schema variables
Previous Message Alexander Korotkov 2017-11-13 10:13:23 Re: Fix bloom WAL tap test