Re: SQL:2011 application time

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].

[1] https://stackoverflow.com/a/7316500/15603477