Re: SQL:2011 application time

On 3/19/24 04:02, Peter Eisentraut wrote:
> On 16.03.24 22:37, Paul A Jungwirth wrote:
>> Here is a new patch series addressing the last few feedback emails
>> from Peter & Jian He. It mostly focuses on the FKs patch, trying to
>> get it really ready to commit,
>
> I have committed the test changes (range and date format etc.).
>
> The FOREIGN KEY patch looks okay to me now. Maybe check if any of the subsequent comments from jian
> should be applied.

Okay, specifics below.

> I think we could also handle multiranges in a hardcoded way? Ranges and multiranges are hardcoded
> concepts anyway. It's just when we move to arbitrary types supporting containment, then it gets a
> bit more complicated.
>
> What would a patch that adds just multiranges on the FK side, but without the full pluggable gist
> support, look like?

Attached a separate patch extending FKs to multiranges only. I'd still love to support arbitrary
types eventually but it's not part of the patches here now.

>> I don't see any drawbacks from supporting inferred REFERENCES with
>> temporal tables, so my vote is to break from the standard here, and
>> *not* apply that follow-up patch. Should I add some docs about that?
>> Also skipping the patch will cause some annoying merge conflicts, so
>> let me know if that's what you choose and I'll handle them right away.
>
> I agree we can allow this.

Great, thanks! Took out those changes.

On 3/19/24 02:01, jian he wrote:
> + * types matching the PERIOD element. periodprocoid is a GiST support
> function to
> + * aggregate multiple PERIOD element values into a single value
> + * (whose return type need not match its inputs,
> + * e.g. many ranges can be aggregated into a multirange).
> * And aggedperiodoperoid is also a ContainedBy operator,
> - * but one whose rhs is anymultirange.
> + * but one whose rhs matches the type returned by aggedperiodoperoid.
> * That way foreign keys can compare fkattr <@ range_agg(pkattr).
> */
> void
> -FindFKPeriodOpers(Oid opclass,
> - Oid *periodoperoid,
> - Oid *aggedperiodoperoid)
> +FindFKPeriodOpersAndProcs(Oid opclass,
> + Oid *periodoperoid,
> + Oid *aggedperiodoperoid,
> + Oid *periodprocoid)
>
> I think, aggedperiodoperoid is more descriptive than periodprocoid, in
> 0005, you don't need to rename it.
> aslo do we need to squash v29 0001 to 0005 together?

I changed the operator names to {,agged}containedbyoperoid. The proc names are not included now
because we only need them for supporting more than ranges + multiranges.

> --- a/doc/src/sgml/ref/create_table.sgml
> +++ b/doc/src/sgml/ref/create_table.sgml
> @@ -1167,7 +1167,8 @@ WITH ( MODULUS <replaceable
> class="parameter">numeric_literal</replaceable>, REM
> column(s) of some row of the referenced table. If the <replaceable
> class="parameter">refcolumn</replaceable> list is omitted, the
> primary key of the <replaceable class="parameter">reftable</replaceable>
> - is used. Otherwise, the <replaceable
> class="parameter">refcolumn</replaceable>
> + is used (omitting any part declared with <literal>WITHOUT
> OVERLAPS</literal>).
> + Otherwise, the <replaceable class="parameter">refcolumn</replaceable>
> list must refer to the columns of a non-deferrable unique or primary key
> constraint or be the columns of a non-partial unique index.
> </para>
> I think this does not express that
> foreign key is PERIOD, then the last column of refcolumn must specify PERIOD?

Okay, added a sentence about that (and adjusted some other things re allowing implicit REFERENCES
and only supporting ranges + multiranges).

> + <para>
> + If the last column is marked with <literal>PERIOD</literal>,
> + it is treated in a special way.
> + While the non-<literal>PERIOD</literal> columns are compared for equality
> + (and there must be at least one of them),
> + the <literal>PERIOD</literal> column is not.
> + Instead the constraint is considered satisfied
> + if the referenced table has matching records
> + (based on the non-<literal>PERIOD</literal> parts of the key)
> + whose combined <literal>PERIOD</literal> values completely cover
> + the referencing record's.
> + In other words, the reference must have a referent for its
> entire duration.
> + This column must be a column with a range type.
> + In addition the referenced table must have a primary key
> + or unique constraint declared with <literal>WITHOUT PORTION</literal>.
> + </para>
> you forgot to change <literal>WITHOUT PORTION</literal> to
> <literal>WITHOUT OVERLAPS</literal>

Oh! Thanks, I guess I was just blind.

> Oid pf_eq_oprs[RI_MAX_NUMKEYS]; /* equality operators (PK = FK) */
> Oid pp_eq_oprs[RI_MAX_NUMKEYS]; /* equality operators (PK = PK) */
> Oid ff_eq_oprs[RI_MAX_NUMKEYS]; /* equality operators (FK = FK) */
> in struct RI_ConstraintInfo, these comments need to be updated?

In earlier feedback Peter advised not changing the "equals" language (e.g. in KeysEqual). But I
added a comment at the top of the struct to clarify.

Rebased to 605721f819.

Yours,

--
Paul ~{:-)
pj(at)illuminatedcomputing(dot)com