Skip to content

Commit 37a795a

Browse files
tglsfdctglsfdc
committed
Support domains over composite types.
This is the last major omission in our domains feature: you can now make a domain over anything that's not a pseudotype. The major complication from an implementation standpoint is that places that might be creating tuples of a domain type now need to be prepared to apply domain_check(). It seems better that unprepared code fail with an error like "<type> is not composite" than that it silently fail to apply domain constraints. Therefore, relevant infrastructure like get_func_result_type() and lookup_rowtype_tupdesc() has been adjusted to treat domain-over-composite as a distinct case that unprepared code won't recognize, rather than just transparently treating it the same as plain composite. This isn't a 100% solution to the possibility of overlooked domain checks, but it catches most places. In passing, improve typcache.c's support for domains (it can now cache the identity of a domain's base type), and rewrite the argument handling logic in jsonfuncs.c's populate_record[set]_worker to reduce duplicative per-call lookups. I believe this is code-complete so far as the core and contrib code go. The PLs need varying amounts of work, which will be tackled in followup patches. Discussion: https://postgr.es/m/4206.1499798337@sss.pgh.pa.us
1 parent 08f1e1f commit 37a795a

37 files changed

+1083
-291
lines changed

‎contrib/hstore/hstore_io.c

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -752,6 +752,8 @@ typedef struct RecordIOData
752752
{
753753
Oid record_type;
754754
int32 record_typmod;
755+
/* this field is used only if target type is domain over composite: */
756+
void *domain_info; /* opaque cache for domain checks */
755757
int ncolumns;
756758
ColumnIOData columns[FLEXIBLE_ARRAY_MEMBER];
757759
} RecordIOData;
@@ -780,9 +782,11 @@ hstore_from_record(PG_FUNCTION_ARGS)
780782
Oid argtype = get_fn_expr_argtype(fcinfo->flinfo, 0);
781783

782784
/*
783-
* have no tuple to look at, so the only source of type info is the
784-
* argtype. The lookup_rowtype_tupdesc call below will error out if we
785-
* don't have a known composite type oid here.
785+
* We have no tuple to look at, so the only source of type info is the
786+
* argtype --- which might be domain over composite, but we don't care
787+
* here, since we have no need to be concerned about domain
788+
* constraints. The lookup_rowtype_tupdesc_domain call below will
789+
* error out if we don't have a known composite type oid here.
786790
*/
787791
tupType = argtype;
788792
tupTypmod = -1;
@@ -793,12 +797,15 @@ hstore_from_record(PG_FUNCTION_ARGS)
793797
{
794798
rec = PG_GETARG_HEAPTUPLEHEADER(0);
795799

796-
/* Extract type info from the tuple itself */
800+
/*
801+
* Extract type info from the tuple itself -- this will work even for
802+
* anonymous record types.
803+
*/
797804
tupType = HeapTupleHeaderGetTypeId(rec);
798805
tupTypmod = HeapTupleHeaderGetTypMod(rec);
799806
}
800807

801-
tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
808+
tupdesc = lookup_rowtype_tupdesc_domain(tupType, tupTypmod, false);
802809
ncolumns = tupdesc->natts;
803810

804811
/*
@@ -943,9 +950,9 @@ hstore_populate_record(PG_FUNCTION_ARGS)
943950
rec = NULL;
944951

945952
/*
946-
* have no tuple to look at, so the only source of type info is the
947-
* argtype. The lookup_rowtype_tupdesc call below will error out if we
948-
* don't have a known composite type oid here.
953+
* We have no tuple to look at, so the only source of type info is the
954+
* argtype. The lookup_rowtype_tupdesc_domain call below will error
955+
* out if we don't have a known composite type oid here.
949956
*/
950957
tupType = argtype;
951958
tupTypmod = -1;
@@ -957,7 +964,10 @@ hstore_populate_record(PG_FUNCTION_ARGS)
957964
if (PG_ARGISNULL(1))
958965
PG_RETURN_POINTER(rec);
959966

960-
/* Extract type info from the tuple itself */
967+
/*
968+
* Extract type info from the tuple itself -- this will work even for
969+
* anonymous record types.
970+
*/
961971
tupType = HeapTupleHeaderGetTypeId(rec);
962972
tupTypmod = HeapTupleHeaderGetTypMod(rec);
963973
}
@@ -975,7 +985,11 @@ hstore_populate_record(PG_FUNCTION_ARGS)
975985
if (HS_COUNT(hs) == 0 && rec)
976986
PG_RETURN_POINTER(rec);
977987

978-
tupdesc = lookup_rowtype_tupdesc(tupType, tupTypmod);
988+
/*
989+
* Lookup the input record's tupdesc. For the moment, we don't worry
990+
* about whether it is a domain over composite.
991+
*/
992+
tupdesc = lookup_rowtype_tupdesc_domain(tupType, tupTypmod, false);
979993
ncolumns = tupdesc->natts;
980994

981995
if (rec)
@@ -1002,6 +1016,7 @@ hstore_populate_record(PG_FUNCTION_ARGS)
10021016
my_extra = (RecordIOData *) fcinfo->flinfo->fn_extra;
10031017
my_extra->record_type = InvalidOid;
10041018
my_extra->record_typmod = 0;
1019+
my_extra->domain_info = NULL;
10051020
}
10061021

10071022
if (my_extra->record_type != tupType ||
@@ -1103,6 +1118,17 @@ hstore_populate_record(PG_FUNCTION_ARGS)
11031118

11041119
rettuple = heap_form_tuple(tupdesc, values, nulls);
11051120

1121+
/*
1122+
* If the target type is domain over composite, all we know at this point
1123+
* is that we've made a valid value of the base composite type. Must
1124+
* check domain constraints before deciding we're done.
1125+
*/
1126+
if (argtype != tupdesc->tdtypeid)
1127+
domain_check(HeapTupleGetDatum(rettuple), false,
1128+
argtype,
1129+
&my_extra->domain_info,
1130+
fcinfo->flinfo->fn_mcxt);
1131+
11061132
ReleaseTupleDesc(tupdesc);
11071133

11081134
PG_RETURN_DATUM(HeapTupleGetDatum(rettuple));

‎doc/src/sgml/datatype.sgml

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4379,8 +4379,7 @@ SET xmloption TO { DOCUMENT | CONTENT };
43794379
underlying type &mdash; for example, any operator or function that
43804380
can be applied to the underlying type will work on the domain type.
43814381
The underlying type can be any built-in or user-defined base type,
4382-
enum type, array or range type, or another domain. (Currently, domains
4383-
over composite types are not implemented.)
4382+
enum type, array type, composite type, range type, or another domain.
43844383
</para>
43854384

43864385
<para>

‎doc/src/sgml/rowtypes.sgml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,9 @@ CREATE TABLE inventory_item (
8484
restriction of the current implementation: since no constraints are
8585
associated with a composite type, the constraints shown in the table
8686
definition <emphasis>do not apply</emphasis> to values of the composite type
87-
outside the table. (A partial workaround is to use domain
88-
types as members of composite types.)
87+
outside the table. (To work around this, create a domain over the composite
88+
type, and apply the desired constraints as <literal>CHECK</literal>
89+
constraints of the domain.)
8990
</para>
9091
</sect2>
9192

‎doc/src/sgml/xfunc.sgml

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -351,6 +351,31 @@ CREATE FUNCTION tf1 (accountno integer, debit numeric) RETURNS numeric AS $$
351351
WHERE accountno = tf1.accountno
352352
RETURNING balance;
353353
$$ LANGUAGE SQL;
354+
</programlisting>
355+
</para>
356+
357+
<para>
358+
A <acronym>SQL</acronym> function must return exactly its declared
359+
result type. This may require inserting an explicit cast.
360+
For example, suppose we wanted the
361+
previous <function>add_em</function> function to return
362+
type <type>float8</type> instead. This won't work:
363+
364+
<programlisting>
365+
CREATE FUNCTION add_em(integer, integer) RETURNS float8 AS $$
366+
SELECT $1 + $2;
367+
$$ LANGUAGE SQL;
368+
</programlisting>
369+
370+
even though in other contexts <productname>PostgreSQL</productname>
371+
would be willing to insert an implicit cast to
372+
convert <type>integer</type> to <type>float8</type>.
373+
We need to write it as
374+
375+
<programlisting>
376+
CREATE FUNCTION add_em(integer, integer) RETURNS float8 AS $$
377+
SELECT ($1 + $2)::float8;
378+
$$ LANGUAGE SQL;
354379
</programlisting>
355380
</para>
356381
</sect2>
@@ -452,13 +477,16 @@ $$ LANGUAGE SQL;
452477
</listitem>
453478
<listitem>
454479
<para>
455-
You must typecast the expressions to match the
456-
definition of the composite type, or you will get errors like this:
480+
We must ensure each expression's type matches the corresponding
481+
column of the composite type, inserting a cast if necessary.
482+
Otherwise we'll get errors like this:
457483
<screen>
458484
<computeroutput>
459485
ERROR: function declared to return emp returns varchar instead of text at column 1
460486
</computeroutput>
461487
</screen>
488+
As with the base-type case, the function will not insert any casts
489+
automatically.
462490
</para>
463491
</listitem>
464492
</itemizedlist>
@@ -478,6 +506,11 @@ $$ LANGUAGE SQL;
478506
in this situation, but it is a handy alternative in some cases
479507
&mdash; for example, if we need to compute the result by calling
480508
another function that returns the desired composite value.
509+
Another example is that if we are trying to write a function that
510+
returns a domain over composite, rather than a plain composite type,
511+
it is always necessary to write it as returning a single column,
512+
since there is no other way to produce a value that is exactly of
513+
the domain type.
481514
</para>
482515

483516
<para>

‎src/backend/catalog/pg_inherits.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,11 @@ has_superclass(Oid relationId)
301301
/*
302302
* Given two type OIDs, determine whether the first is a complex type
303303
* (class type) that inherits from the second.
304+
*
305+
* This essentially asks whether the first type is guaranteed to be coercible
306+
* to the second. Therefore, we allow the first type to be a domain over a
307+
* complex type that inherits from the second; that creates no difficulties.
308+
* But the second type cannot be a domain.
304309
*/
305310
bool
306311
typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId)
@@ -314,9 +319,9 @@ typeInheritsFrom(Oid subclassTypeId, Oid superclassTypeId)
314319
ListCell *queue_item;
315320

316321
/* We need to work with the associated relation OIDs */
317-
subclassRelid = typeidTypeRelid(subclassTypeId);
322+
subclassRelid = typeOrDomainTypeRelid(subclassTypeId);
318323
if (subclassRelid == InvalidOid)
319-
return false; /* not a complex type */
324+
return false; /* not a complex type or domain over one */
320325
superclassRelid = typeidTypeRelid(superclassTypeId);
321326
if (superclassRelid == InvalidOid)
322327
return false; /* not a complex type */

‎src/backend/catalog/pg_proc.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ ProcedureCreate(const char *procedureName,
262262
*/
263263
if (parameterCount == 1 &&
264264
OidIsValid(parameterTypes->values[0]) &&
265-
(relid = typeidTypeRelid(parameterTypes->values[0])) != InvalidOid &&
265+
(relid = typeOrDomainTypeRelid(parameterTypes->values[0])) != InvalidOid &&
266266
get_attnum(relid, procedureName) != InvalidAttrNumber)
267267
ereport(ERROR,
268268
(errcode(ERRCODE_DUPLICATE_COLUMN),

‎src/backend/commands/tablecmds.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5091,6 +5091,8 @@ find_typed_table_dependencies(Oid typeOid, const char *typeName, DropBehavior be
50915091
* isn't suitable, throw an error. Currently, we require that the type
50925092
* originated with CREATE TYPE AS. We could support any row type, but doing so
50935093
* would require handling a number of extra corner cases in the DDL commands.
5094+
* (Also, allowing domain-over-composite would open up a can of worms about
5095+
* whether and how the domain's constraints should apply to derived tables.)
50945096
*/
50955097
void
50965098
check_of_type(HeapTuple typetuple)
@@ -6190,8 +6192,8 @@ ATPrepSetStatistics(Relation rel, const char *colName, int16 colNum, Node *newVa
61906192
RelationGetRelationName(rel))));
61916193

61926194
/*
6193-
* We allow referencing columns by numbers only for indexes, since
6194-
* table column numbers could contain gaps if columns are later dropped.
6195+
* We allow referencing columns by numbers only for indexes, since table
6196+
* column numbers could contain gaps if columns are later dropped.
61956197
*/
61966198
if (rel->rd_rel->relkind != RELKIND_INDEX && !colName)
61976199
ereport(ERROR,

‎src/backend/commands/typecmds.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -798,13 +798,16 @@ DefineDomain(CreateDomainStmt *stmt)
798798
basetypeoid = HeapTupleGetOid(typeTup);
799799

800800
/*
801-
* Base type must be a plain base type, another domain, an enum or a range
802-
* type. Domains over pseudotypes would create a security hole. Domains
803-
* over composite types might be made to work in the future, but not
804-
* today.
801+
* Base type must be a plain base type, a composite type, another domain,
802+
* an enum or a range type. Domains over pseudotypes would create a
803+
* security hole. (It would be shorter to code this to just check for
804+
* pseudotypes; but it seems safer to call out the specific typtypes that
805+
* are supported, rather than assume that all future typtypes would be
806+
* automatically supported.)
805807
*/
806808
typtype = baseType->typtype;
807809
if (typtype != TYPTYPE_BASE &&
810+
typtype != TYPTYPE_COMPOSITE &&
808811
typtype != TYPTYPE_DOMAIN &&
809812
typtype != TYPTYPE_ENUM &&
810813
typtype != TYPTYPE_RANGE)

‎src/backend/executor/execExprInterp.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3469,8 +3469,12 @@ ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext)
34693469
* generates an INT4 NULL regardless of the dropped column type).
34703470
* If we find a dropped column and cannot verify that case (1)
34713471
* holds, we have to use the slow path to check (2) for each row.
3472+
*
3473+
* If vartype is a domain over composite, just look through that
3474+
* to the base composite type.
34723475
*/
3473-
var_tupdesc = lookup_rowtype_tupdesc(variable->vartype, -1);
3476+
var_tupdesc = lookup_rowtype_tupdesc_domain(variable->vartype,
3477+
-1, false);
34743478

34753479
slot_tupdesc = slot->tts_tupleDescriptor;
34763480

‎src/backend/executor/execSRF.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ ExecMakeFunctionResultSet(SetExprState *fcache,
502502
{
503503
TupleTableSlot *slot = fcache->funcResultSlot;
504504
MemoryContext oldContext;
505-
bool foundTup;
505+
bool foundTup;
506506

507507
/*
508508
* Have to make sure tuple in slot lives long enough, otherwise
@@ -734,7 +734,8 @@ init_sexpr(Oid foid, Oid input_collation, Expr *node,
734734
/* Must save tupdesc in sexpr's context */
735735
oldcontext = MemoryContextSwitchTo(sexprCxt);
736736

737-
if (functypclass == TYPEFUNC_COMPOSITE)
737+
if (functypclass == TYPEFUNC_COMPOSITE ||
738+
functypclass == TYPEFUNC_COMPOSITE_DOMAIN)
738739
{
739740
/* Composite data type, e.g. a table's row type */
740741
Assert(tupdesc);

‎src/backend/executor/functions.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1665,7 +1665,15 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
16651665
}
16661666
else if (fn_typtype == TYPTYPE_COMPOSITE || rettype == RECORDOID)
16671667
{
1668-
/* Returns a rowtype */
1668+
/*
1669+
* Returns a rowtype.
1670+
*
1671+
* Note that we will not consider a domain over composite to be a
1672+
* "rowtype" return type; it goes through the scalar case above. This
1673+
* is because SQL functions don't provide any implicit casting to the
1674+
* result type, so there is no way to produce a domain-over-composite
1675+
* result except by computing it as an explicit single-column result.
1676+
*/
16691677
TupleDesc tupdesc;
16701678
int tupnatts; /* physical number of columns in tuple */
16711679
int tuplogcols; /* # of nondeleted columns in tuple */
@@ -1711,7 +1719,10 @@ check_sql_fn_retval(Oid func_id, Oid rettype, List *queryTreeList,
17111719
}
17121720
}
17131721

1714-
/* Is the rowtype fixed, or determined only at runtime? */
1722+
/*
1723+
* Is the rowtype fixed, or determined only at runtime? (Note we
1724+
* cannot see TYPEFUNC_COMPOSITE_DOMAIN here.)
1725+
*/
17151726
if (get_func_result_type(func_id, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
17161727
{
17171728
/*

‎src/backend/executor/nodeFunctionscan.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,8 @@ ExecInitFunctionScan(FunctionScan *node, EState *estate, int eflags)
383383
&funcrettype,
384384
&tupdesc);
385385

386-
if (functypclass == TYPEFUNC_COMPOSITE)
386+
if (functypclass == TYPEFUNC_COMPOSITE ||
387+
functypclass == TYPEFUNC_COMPOSITE_DOMAIN)
387388
{
388389
/* Composite data type, e.g. a table's row type */
389390
Assert(tupdesc);

‎src/backend/nodes/makefuncs.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,10 @@ makeVarFromTargetEntry(Index varno,
120120
* table entry, and varattno == 0 to signal that it references the whole
121121
* tuple. (Use of zero here is unclean, since it could easily be confused
122122
* with error cases, but it's not worth changing now.) The vartype indicates
123-
* a rowtype; either a named composite type, or RECORD. This function
124-
* encapsulates the logic for determining the correct rowtype OID to use.
123+
* a rowtype; either a named composite type, or a domain over a named
124+
* composite type (only possible if the RTE is a function returning that),
125+
* or RECORD. This function encapsulates the logic for determining the
126+
* correct rowtype OID to use.
125127
*
126128
* If allowScalar is true, then for the case where the RTE is a single function
127129
* returning a non-composite result type, we produce a normal Var referencing

‎src/backend/optimizer/util/clauses.c

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2356,6 +2356,10 @@ CommuteRowCompareExpr(RowCompareExpr *clause)
23562356
* is still what it was when the expression was parsed. This is needed to
23572357
* guard against improper simplification after ALTER COLUMN TYPE. (XXX we
23582358
* may well need to make similar checks elsewhere?)
2359+
*
2360+
* rowtypeid may come from a whole-row Var, and therefore it can be a domain
2361+
* over composite, but for this purpose we only care about checking the type
2362+
* of a contained field.
23592363
*/
23602364
static bool
23612365
rowtype_field_matches(Oid rowtypeid, int fieldnum,
@@ -2368,7 +2372,7 @@ rowtype_field_matches(Oid rowtypeid, int fieldnum,
23682372
/* No issue for RECORD, since there is no way to ALTER such a type */
23692373
if (rowtypeid == RECORDOID)
23702374
return true;
2371-
tupdesc = lookup_rowtype_tupdesc(rowtypeid, -1);
2375+
tupdesc = lookup_rowtype_tupdesc_domain(rowtypeid, -1, false);
23722376
if (fieldnum <= 0 || fieldnum > tupdesc->natts)
23732377
{
23742378
ReleaseTupleDesc(tupdesc);
@@ -5005,7 +5009,9 @@ inline_set_returning_function(PlannerInfo *root, RangeTblEntry *rte)
50055009
*
50065010
* If the function returns a composite type, don't inline unless the check
50075011
* shows it's returning a whole tuple result; otherwise what it's
5008-
* returning is a single composite column which is not what we need.
5012+
* returning is a single composite column which is not what we need. (Like
5013+
* check_sql_fn_retval, we deliberately exclude domains over composite
5014+
* here.)
50095015
*/
50105016
if (!check_sql_fn_retval(func_oid, fexpr->funcresulttype,
50115017
querytree_list,

0 commit comments

Comments
 (0)