On Tue, May 24, 2011 at 02:00:54PM -0400, Noah Misch wrote:
> On Tue, May 24, 2011 at 01:28:38PM -0400, Tom Lane wrote:
> > Noah Misch <n...@leadboat.com> writes:
> > > On Tue, May 24, 2011 at 12:12:55PM -0400, Tom Lane wrote:
> > >> This is a consequence of the changes I made to fix bug #5717,
> > >> particularly the issues around ANYARRAY matching discussed here:
> > >> http://archives.postgresql.org/pgsql-hackers/2010-10/msg01545.php
> >
> > > We discussed this a few weeks ago:
> > > http://archives.postgresql.org/message-id/20110511093217.gb26...@tornado.gateway.2wire.net
> >
> > > What's to recommend #1 over what I proposed then? Seems like a discard of
> > > functionality for little benefit.
> >
> > I am unwilling to commit to making #2 work, especially not under time
> > constraints; and you apparently aren't either, since you haven't
> > produced the patch you alluded to at the end of that thread.
>
> I took your lack of any response as non-acceptance of the plan I outlined.
> Alas, the wrong conclusion. I'll send a patch this week.
See attached arrdom1v1-polymorphism.patch. This currently adds one syscache
lookup per array_append, array_prepend or array_cat call when the anyarray
type is not a domain. When the type is a domain, it adds a few more. We
could add caching without too much trouble. I suppose someone out there uses
these functions in bulk operations, though I've yet to see it. Is it worth
optimizing this straightway?
For a function like
CREATE FUNCTION f(anyarray, VARIADIC anyarray) RETURNS int LANGUAGE sql
AS 'SELECT array_length($1, 1) + array_length($2, 1)'
we must coerce the variadic argument array to a domain type when the other
anyarray argument(s) compel it. Having implemented that, it was nearly free
to re-support a VARIADIC parameter specifically declared with a domain over an
array. Consequently, I've done that as well.
See here for previously-disclosed rationale:
http://archives.postgresql.org/message-id/20110511191249.ga29...@tornado.gateway.2wire.net
I audited remaining get_element_type() callers. CheckAttributeType() needs to
recurse into domains over array types just like any other array type. Fixed
trivially in arrdom2v1-checkattr.patch; see its test case for an example hole.
nm
diff --git a/src/backend/catalog/pg_aggregate.c
b/src/backend/catalog/pg_aggregate.c
index 86e8c6b..8194519 100644
*** a/src/backend/catalog/pg_aggregate.c
--- b/src/backend/catalog/pg_aggregate.c
***************
*** 305,310 **** lookup_agg_function(List *fnName,
--- 305,311 ----
Oid *rettype)
{
Oid fnOid;
+ Oid vartype;
bool retset;
int nvargs;
Oid *true_oid_array;
***************
*** 321,327 **** lookup_agg_function(List *fnName,
*/
fdresult = func_get_detail(fnName, NIL, NIL,
nargs, input_types,
false, false,
! &fnOid, rettype,
&retset, &nvargs,
&true_oid_array,
NULL);
/* only valid case is a normal function not returning a set */
--- 322,328 ----
*/
fdresult = func_get_detail(fnName, NIL, NIL,
nargs, input_types,
false, false,
! &fnOid, rettype,
&vartype, &retset, &nvargs,
&true_oid_array,
NULL);
/* only valid case is a normal function not returning a set */
diff --git a/src/backend/catalog/pg_proc.index 6250b07..8fdd88e 100644
*** a/src/backend/catalog/pg_proc.c
--- b/src/backend/catalog/pg_proc.c
***************
*** 272,278 **** ProcedureCreate(const char *procedureName,
variadicType =
ANYELEMENTOID;
break;
default:
! variadicType =
get_element_type(allParams[i]);
if
(!OidIsValid(variadicType))
elog(ERROR,
"variadic parameter is not an array");
break;
--- 272,278 ----
variadicType =
ANYELEMENTOID;
break;
default:
! variadicType =
get_base_element_type(allParams[i]);
if
(!OidIsValid(variadicType))
elog(ERROR,
"variadic parameter is not an array");
break;
diff --git a/src/backend/commands/fuindex 03da168..717c632 100644
*** a/src/backend/commands/functioncmds.c
--- b/src/backend/commands/functioncmds.c
***************
*** 273,279 **** examine_parameter_list(List *parameters, Oid languageOid,
/* okay */
break;
default:
! if (!OidIsValid(get_element_type(toid)))
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("VARIADIC
parameter must be an array")));
--- 273,279 ----
/* okay */
break;
default:
! if
(!OidIsValid(get_base_element_type(toid)))
ereport(ERROR,
(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
errmsg("VARIADIC
parameter must be an array")));
diff --git a/src/backend/parser/parse_coerindex 0418972..990ee29 100644
*** a/src/backend/parser/parse_coerce.c
--- b/src/backend/parser/parse_coerce.c
***************
*** 1326,1332 **** check_generic_type_consistency(Oid *actual_arg_types,
return true;
}
! array_typelem = get_element_type(array_typeid);
if (!OidIsValid(array_typelem))
return false; /* should be an array, but
isn't */
--- 1326,1332 ----
return true;
}
! array_typelem = get_base_element_type(array_typeid);
if (!OidIsValid(array_typelem))
return false; /* should be an array, but
isn't */
***************
*** 1513,1519 **** enforce_generic_type_consistency(Oid *actual_arg_types,
}
else
{
! array_typelem = get_element_type(array_typeid);
if (!OidIsValid(array_typelem))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
--- 1513,1519 ----
}
else
{
! array_typelem = get_base_element_type(array_typeid);
if (!OidIsValid(array_typelem))
ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
***************
*** 1656,1662 **** resolve_generic_type(Oid declared_type,
if (context_declared_type == ANYARRAYOID)
{
/* Use actual type, but it must be an array */
! Oid array_typelem =
get_element_type(context_actual_type);
if (!OidIsValid(array_typelem))
ereport(ERROR,
--- 1656,1662 ----
if (context_declared_type == ANYARRAYOID)
{
/* Use actual type, but it must be an array */
! Oid array_typelem =
get_base_element_type(context_actual_type);
if (!OidIsValid(array_typelem))
ereport(ERROR,
***************
*** 1687,1693 **** resolve_generic_type(Oid declared_type,
if (context_declared_type == ANYARRAYOID)
{
/* Use the element type corresponding to actual type */
! Oid array_typelem =
get_element_type(context_actual_type);
if (!OidIsValid(array_typelem))
ereport(ERROR,
--- 1687,1693 ----
if (context_declared_type == ANYARRAYOID)
{
/* Use the element type corresponding to actual type */
! Oid array_typelem =
get_base_element_type(context_actual_type);
if (!OidIsValid(array_typelem))
ereport(ERROR,
diff --git a/src/backend/parser/parse_fuindex 75f1e20..acf9317 100644
*** a/src/backend/parser/parse_func.c
--- b/src/backend/parser/parse_func.c
***************
*** 64,69 **** ParseFuncOrColumn(ParseState *pstate, List *funcname, List
*fargs,
--- 64,70 ----
WindowDef *over, bool is_column, int location)
{
Oid rettype;
+ Oid vartype;
Oid funcid;
ListCell *l;
ListCell *nextl;
***************
*** 212,218 **** ParseFuncOrColumn(ParseState *pstate, List *funcname, List
*fargs,
fdresult = func_get_detail(funcname, fargs, argnames, nargs,
actual_arg_types,
!func_variadic, true,
! &funcid, &rettype,
&retset, &nvargs,
&declared_arg_types,
&argdefaults);
if (fdresult == FUNCDETAIL_COERCION)
{
--- 213,219 ----
fdresult = func_get_detail(funcname, fargs, argnames, nargs,
actual_arg_types,
!func_variadic, true,
! &funcid, &rettype,
&vartype, &retset, &nvargs,
&declared_arg_types,
&argdefaults);
if (fdresult == FUNCDETAIL_COERCION)
{
***************
*** 371,377 **** ParseFuncOrColumn(ParseState *pstate, List *funcname, List
*fargs,
newa->multidims = false;
newa->location = exprLocation((Node *) vargs);
! fargs = lappend(fargs, newa);
}
/* build the appropriate output structure */
--- 372,403 ----
newa->multidims = false;
newa->location = exprLocation((Node *) vargs);
! /*
! * If the VARIADIC parameter is anyarray, look for other
anyarray
! * arguments that constrain its true type. If we don't find
one, the
! * standard array type of the variadic element type fills in.
! */
! if (vartype == ANYARRAYOID)
! {
! int i;
!
! vartype = newa->array_typeid;
! for (i = 0; i < nargs; i++)
! if (declared_arg_types[i] == ANYARRAYOID)
! {
! vartype = actual_arg_types[i];
! break;
! }
! }
!
! /* VARIADIC parameter type may be a domain - coerce it */
! fargs = lappend(fargs, coerce_type(pstate,
!
(Node *) newa,
!
newa->array_typeid,
!
vartype, -1,
!
COERCION_IMPLICIT,
!
COERCE_IMPLICIT_CAST,
!
-1));
}
/* build the appropriate output structure */
***************
*** 929,934 **** func_get_detail(List *funcname,
--- 955,961 ----
bool expand_defaults,
Oid *funcid, /* return value */
Oid *rettype, /* return value */
+ Oid *vartype, /* return value */
bool *retset, /* return value */
int *nvargs, /* return value */
Oid **true_typeids, /* return value
*/
***************
*** 940,945 **** func_get_detail(List *funcname,
--- 967,973 ----
/* initialize output arguments to silence compiler warnings */
*funcid = InvalidOid;
*rettype = InvalidOid;
+ *vartype = InvalidOid;
*retset = false;
*nvargs = 0;
*true_typeids = NULL;
***************
*** 1050,1055 **** func_get_detail(List *funcname,
--- 1078,1084 ----
/* Treat it as a type coercion */
*funcid = InvalidOid;
*rettype = targetType;
+ *vartype = InvalidOid;
*retset = false;
*nvargs = 0;
*true_typeids = argtypes;
***************
*** 1148,1153 **** func_get_detail(List *funcname,
--- 1177,1186 ----
best_candidate->oid);
pform = (Form_pg_proc) GETSTRUCT(ftup);
*rettype = pform->prorettype;
+ if (*nvargs > 0)
+ *vartype = pform->proargtypes.values[pform->pronargs -
1];
+ else
+ *vartype = InvalidOid;
*retset = pform->proretset;
/* fetch default args if caller wants 'em */
if (argdefaults && best_candidate->ndargs > 0)
diff --git a/src/backend/utils/adt/arrindex 274e867..6f9824d 100644
*** a/src/backend/utils/adt/array_userfuncs.c
--- b/src/backend/utils/adt/array_userfuncs.c
***************
*** 32,37 **** array_push(PG_FUNCTION_ARGS)
--- 32,38 ----
*lb;
ArrayType *result;
int indx;
+ Oid array_type;
Oid element_type;
int16 typlen;
bool typbyval;
***************
*** 47,57 **** array_push(PG_FUNCTION_ARGS)
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("could not determine input data
types")));
! arg0_elemid = get_element_type(arg0_typeid);
! arg1_elemid = get_element_type(arg1_typeid);
if (arg0_elemid != InvalidOid)
{
if (PG_ARGISNULL(0))
v = construct_empty_array(arg0_elemid);
else
--- 48,59 ----
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("could not determine input data
types")));
! arg0_elemid = get_base_element_type(arg0_typeid);
! arg1_elemid = get_base_element_type(arg1_typeid);
if (arg0_elemid != InvalidOid)
{
+ array_type = arg0_typeid;
if (PG_ARGISNULL(0))
v = construct_empty_array(arg0_elemid);
else
***************
*** 64,69 **** array_push(PG_FUNCTION_ARGS)
--- 66,72 ----
}
else if (arg1_elemid != InvalidOid)
{
+ array_type = arg1_typeid;
if (PG_ARGISNULL(1))
v = construct_empty_array(arg1_elemid);
else
***************
*** 156,161 **** array_push(PG_FUNCTION_ARGS)
--- 159,171 ----
if (ARR_NDIM(v) == 1)
ARR_LBOUND(result)[0] = ARR_LBOUND(v)[0];
+ /*
+ * If the input array type is a domain, our returned value must also
+ * conform to the constraints of that domain.
+ */
+ if (getBaseType(array_type) != array_type)
+ domain_check(PointerGetDatum(result), false, array_type, NULL,
NULL);
+
PG_RETURN_ARRAYTYPE_P(result);
}
***************
*** 195,200 **** array_cat(PG_FUNCTION_ARGS)
--- 205,211 ----
Oid element_type;
Oid element_type1;
Oid element_type2;
+ Oid array_type;
int32 dataoffset;
/* Concatenating a null array is a no-op, just return the other input */
***************
*** 396,401 **** array_cat(PG_FUNCTION_ARGS)
--- 407,421 ----
nitems2);
}
+ /*
+ * If the input array type is a domain, our returned value must also
+ * conform to the constraints of that domain. Earlier exits did not
need
+ * this check, because they merely returned input data unchanged.
+ */
+ array_type = get_fn_expr_argtype(fcinfo->flinfo, 0);
+ if (getBaseType(array_type) != array_type)
+ domain_check(PointerGetDatum(result), false, array_type, NULL,
NULL);
+
PG_RETURN_ARRAYTYPE_P(result);
}
diff --git a/src/backend/utils/adt/ruleutils.cindex 3ab90cb..b7bdd0b 100644
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
***************
*** 7001,7006 **** generate_function_name(Oid funcid, int nargs, List *argnames,
--- 7001,7007 ----
FuncDetailCode p_result;
Oid p_funcid;
Oid p_rettype;
+ Oid p_vartype;
bool p_retset;
int p_nvargs;
Oid *p_true_typeids;
***************
*** 7020,7026 **** generate_function_name(Oid funcid, int nargs, List *argnames,
p_result = func_get_detail(list_make1(makeString(proname)),
NIL, argnames,
nargs, argtypes,
!OidIsValid(procform->provariadic), true,
! &p_funcid,
&p_rettype,
&p_retset,
&p_nvargs, &p_true_typeids, NULL);
if ((p_result == FUNCDETAIL_NORMAL ||
p_result == FUNCDETAIL_AGGREGATE ||
--- 7021,7027 ----
p_result = func_get_detail(list_make1(makeString(proname)),
NIL, argnames,
nargs, argtypes,
!OidIsValid(procform->provariadic), true,
! &p_funcid,
&p_rettype, &p_vartype,
&p_retset,
&p_nvargs, &p_true_typeids, NULL);
if ((p_result == FUNCDETAIL_NORMAL ||
p_result == FUNCDETAIL_AGGREGATE ||
diff --git a/src/include/parser/parse_fuindex 2fe6f90..01f4954 100644
*** a/src/include/parser/parse_func.h
--- b/src/include/parser/parse_func.h
***************
*** 52,58 **** extern FuncDetailCode func_get_detail(List *funcname,
List *fargs, List *fargnames,
int nargs, Oid *argtypes,
bool expand_variadic, bool expand_defaults,
! Oid *funcid, Oid *rettype,
bool *retset, int *nvargs, Oid **true_typeids,
List **argdefaults);
--- 52,58 ----
List *fargs, List *fargnames,
int nargs, Oid *argtypes,
bool expand_variadic, bool expand_defaults,
! Oid *funcid, Oid *rettype, Oid *vartype,
bool *retset, int *nvargs, Oid **true_typeids,
List **argdefaults);
diff --git a/src/test/regress/expectedindex 7d72791..6c1d2ee 100644
*** a/src/test/regress/expected/domain.out
--- b/src/test/regress/expected/domain.out
***************
*** 595,597 **** select array_elem_check(-1);
--- 595,637 ----
ERROR: value for domain orderedpair violates check constraint
"orderedpair_check"
CONTEXT: PL/pgSQL function "array_elem_check" line 5 at assignment
drop function array_elem_check(int);
+ select array_append('{1}'::orderedpair, 2);
+ array_append
+ --------------
+ {1,2}
+ (1 row)
+
+ select array_prepend(2, '{1}'::orderedpair); -- fail
+ ERROR: value for domain orderedpair violates check constraint
"orderedpair_check"
+ select array_cat('{2}'::orderedpair, '{1}'); -- fail
+ ERROR: value for domain orderedpair violates check constraint
"orderedpair_check"
+ create or replace function domain_variadic(variadic orderedpair)
+ returns int
+ as 'select $1[2] - $1[1]' language sql;
+ select domain_variadic(1, 2);
+ domain_variadic
+ -----------------
+ 1
+ (1 row)
+
+ select domain_variadic(2, 1); -- fail
+ ERROR: value for domain orderedpair violates check constraint
"orderedpair_check"
+ select domain_variadic(variadic array[1,2]);
+ domain_variadic
+ -----------------
+ 1
+ (1 row)
+
+ select domain_variadic(variadic array[2,1]); -- fail
+ ERROR: value for domain orderedpair violates check constraint
"orderedpair_check"
+ create or replace function domain_poly_variadic(anyarray, variadic anyarray)
+ returns int
+ as 'select array_length($1, 1) * array_length($2, 1)' language sql;
+ select domain_poly_variadic('{2}'::orderedpair, 3, 4);
+ domain_poly_variadic
+ ----------------------
+ 2
+ (1 row)
+
+ select domain_poly_variadic('{2}'::orderedpair, 3, 1); -- fail
+ ERROR: value for domain orderedpair violates check constraint
"orderedpair_check"
diff --git a/src/test/regress/sql/domain.sqindex 545af62..180fdcf 100644
*** a/src/test/regress/sql/domain.sql
--- b/src/test/regress/sql/domain.sql
***************
*** 466,468 **** select array_elem_check(3);
--- 466,486 ----
select array_elem_check(-1);
drop function array_elem_check(int);
+
+ select array_append('{1}'::orderedpair, 2);
+ select array_prepend(2, '{1}'::orderedpair); -- fail
+ select array_cat('{2}'::orderedpair, '{1}'); -- fail
+
+ create or replace function domain_variadic(variadic orderedpair)
+ returns int
+ as 'select $1[2] - $1[1]' language sql;
+ select domain_variadic(1, 2);
+ select domain_variadic(2, 1); -- fail
+ select domain_variadic(variadic array[1,2]);
+ select domain_variadic(variadic array[2,1]); -- fail
+
+ create or replace function domain_poly_variadic(anyarray, variadic anyarray)
+ returns int
+ as 'select array_length($1, 1) * array_length($2, 1)' language sql;
+ select domain_poly_variadic('{2}'::orderedpair, 3, 4);
+ select domain_poly_variadic('{2}'::orderedpair, 3, 1); -- fail
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 71c9931..41fd13e 100644
*** a/src/backend/catalog/heap.c
--- b/src/backend/catalog/heap.c
***************
*** 530,536 **** CheckAttributeType(const char *attname,
containing_rowtypes = list_delete_first(containing_rowtypes);
}
! else if (OidIsValid((att_typelem = get_element_type(atttypid))))
{
/*
* Must recurse into array types, too, in case they are
composite.
--- 530,536 ----
containing_rowtypes = list_delete_first(containing_rowtypes);
}
! else if (OidIsValid((att_typelem = get_base_element_type(atttypid))))
{
/*
* Must recurse into array types, too, in case they are
composite.
diff --git a/src/test/regress/expindex 26e7bfd..f84da45 100644
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
***************
*** 1516,1521 **** alter table recur1 add column f2 recur1; -- fails
--- 1516,1524 ----
ERROR: composite type recur1 cannot be made a member of itself
alter table recur1 add column f2 recur1[]; -- fails
ERROR: composite type recur1 cannot be made a member of itself
+ create domain array_of_recur1 as recur1[];
+ alter table recur1 add column f2 array_of_recur1; -- fails
+ ERROR: composite type recur1 cannot be made a member of itself
create temp table recur2 (f1 int, f2 recur1);
alter table recur1 add column f2 recur2; -- fails
ERROR: composite type recur1 cannot be made a member of itself
diff --git a/src/test/regress/sql/alter_table.sqindex 0ed16fb..b5d76ea 100644
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
***************
*** 1128,1133 **** alter table tab1 alter column b type varchar; -- fails
--- 1128,1135 ----
create temp table recur1 (f1 int);
alter table recur1 add column f2 recur1; -- fails
alter table recur1 add column f2 recur1[]; -- fails
+ create domain array_of_recur1 as recur1[];
+ alter table recur1 add column f2 array_of_recur1; -- fails
create temp table recur2 (f1 int, f2 recur1);
alter table recur1 add column f2 recur2; -- fails
alter table recur1 add column f2 int;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
