From: | jian he <jian(dot)universality(at)gmail(dot)com> |
---|---|
To: | Paul A Jungwirth <pj(at)illuminatedcomputing(dot)com> |
Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: SQL:2011 application time |
Date: | 2024-03-17 23:30:00 |
Message-ID: | CACJufxF3voTh5yT46w2C4Noy=YDxKNi_LW7r0C55MuPvk=r1Pw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi, minor issues from 00001 to 0005.
+ <row>
+ <entry><function>referencedagg</function></entry>
+ <entry>aggregates referenced rows' <literal>WITHOUT OVERLAPS</literal>
+ part</entry>
+ <entry>13</entry>
+ </row>
comparing with surrounding items, maybe need to add `(optional)`?
I think the explanation is not good as explained in referencedagg entry below:
<para>
An aggregate function. Given values of this opclass,
it returns a value combining them all. The return value
need not be the same type as the input, but it must be a
type that can appear on the right hand side of the "contained by"
operator. For example the built-in <literal>range_ops</literal>
opclass uses <literal>range_agg</literal> here, so that foreign
keys can check <literal>fkperiod @> range_agg(pkperiod)</literal>.
</para>
+ 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>
seems you missed replacing this one.
in v28-0002, the function name is FindFKPeriodOpers,
then in v28-0005 rename it to FindFKPeriodOpersAndProcs?
renaming the function name in a set of patches seems not a good idea?
+ <para>
+ This is used for temporal foreign key constraints.
+ If you omit this support function, your type cannot be used
+ as the <literal>PERIOD</literal> part of a foreign key.
+ </para>
in v28-0004, I think here "your type" should change to "your opclass"?
+bool
+check_amproc_is_aggregate(Oid funcid)
+{
+ bool result;
+ HeapTuple tp;
+ Form_pg_proc procform;
+
+ tp = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid));
+ if (!HeapTupleIsValid(tp))
+ elog(ERROR, "cache lookup failed for function %u", funcid);
+ procform = (Form_pg_proc) GETSTRUCT(tp);
+ result = procform->prokind == 'a';
+ ReleaseSysCache(tp);
+ return result;
+}
maybe
`
change procform->prokind == 'a';
`
to
`
procform->prokind == PROKIND_AGGREGATE;
`
or we can put the whole function to cache/lsyscache.c
name it just as proc_is_aggregate.
- Added pg_dump support.
- Show the correct syntax in psql \d output for foreign keys.
in 28-0002, seems there is no work to correspond to these 2 items in
the commit message?
@@ -12335,7 +12448,8 @@ validateForeignKeyConstraint(char *conname,
Relation rel,
Relation pkrel,
Oid pkindOid,
- Oid constraintOid)
+ Oid constraintOid,
+ bool temporal)
do you need to change the last argument of this function to "is_period"?
+ sprintf(paramname, "$%d", riinfo->nkeys);
+ sprintf(paramname, "$%d", riinfo->nkeys);
do you think it worth the trouble to change to snprintf, I found
related post on [1].
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Gustafsson | 2024-03-17 23:49:24 | Re: Support json_errdetail in FRONTEND builds |
Previous Message | Thomas Munro | 2024-03-17 23:20:26 | Re: Regression tests fail with musl libc because libpq.so can't be loaded |