ALTER TYPE 3: add facility to identify further no-work cases
Here I add the notion of an "exemptor function", a property of a cast that
determines when calls to the cast would be superfluous. Such calls can be
removed, reduced to RelabelType expressions, or annotated (via a new field in
FuncExpr) with the applicable exemptions. I modify various parse_coerce.c
functions to retrieve, call, and act on these exemptor functions; this includes
GetCoerceExemptions() from the last patch. I did opt to make
find_typmod_coercion_function return COERCION_PATH_RELABELTYPE when no work is
needed, rather than COERCION_PATH_NONE; this makes it consistent with
find_coercion_pathway's use of that enumeration.
To demonstrate the functionality, I add exemptor functions for varchar and xml.
Originally I was only going to start with varchar, but xml tests the other major
code path, and the exemptor function for xml is dead simple.
This helps on conversions like varchar(4)->varchar(8) and text->xml.
Attachments:
at3-exemptor.patchtext/plain; charset=us-asciiDownload
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***************
*** 1419,1424 ****
--- 1419,1434 ----
</row>
<row>
+ <entry><structfield>castexemptor</structfield></entry>
+ <entry><type>oid</type></entry>
+ <entry><literal><link linkend="catalog-pg-proc"><structname>pg_proc</structname></link>.oid</literal></entry>
+ <entry>
+ The OID of the exemptor function used to identify when this cast is
+ unnecessary. Zero is stored if the cast has no exemptor function.
+ </entry>
+ </row>
+
+ <row>
<entry><structfield>castcontext</structfield></entry>
<entry><type>char</type></entry>
<entry></entry>
*** a/doc/src/sgml/ref/create_cast.sgml
--- b/doc/src/sgml/ref/create_cast.sgml
***************
*** 20,25 ****
--- 20,26 ----
<synopsis>
CREATE CAST (<replaceable>source_type</replaceable> AS <replaceable>target_type</replaceable>)
WITH FUNCTION <replaceable>function_name</replaceable> (<replaceable>argument_type</replaceable> [, ...])
+ [ EXEMPTOR <replaceable>function_name</replaceable> (<replaceable>argument_type</replaceable> [, ...]) ]
[ AS ASSIGNMENT | AS IMPLICIT ]
CREATE CAST (<replaceable>source_type</replaceable> AS <replaceable>target_type</replaceable>)
***************
*** 204,209 **** SELECT CAST ( 2 AS numeric ) + 4.0;
--- 205,221 ----
</varlistentry>
<varlistentry>
+ <term><literal>EXEMPTOR</literal> <replaceable>function_name</replaceable>(<replaceable>argument_type</replaceable> [, ...])</term>
+
+ <listitem>
+ <para>
+ The <firstterm>exemptor function</> used to identify when this cast is
+ unnecessary.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>WITHOUT FUNCTION</literal></term>
<listitem>
***************
*** 269,290 **** SELECT CAST ( 2 AS numeric ) + 4.0;
</para>
<para>
! Ordinarily a cast must have different source and target data types.
! However, it is allowed to declare a cast with identical source and
! target types if it has a cast implementation function with more than one
! argument. This is used to represent type-specific length coercion
! functions in the system catalogs. The named function is used to
! coerce a value of the type to the type modifier value given by its
! second argument.
</para>
<para>
! When a cast has different source and
! target types and a function that takes more than one argument, it
! supports converting from one type to another and applying a length
! coercion in a single step. When no such entry is available, coercion
! to a type that uses a type modifier involves two cast steps, one to
! convert between data types and a second to apply the modifier.
</para>
</refsect1>
--- 281,338 ----
</para>
<para>
! Ordinarily a cast must have different source and target data types. However,
! it is allowed to declare a cast with identical source and target types if it
! has a cast implementation function with more than one argument. This is used
! to represent type-specific length coercion functions in the system catalogs.
! The named function is used to coerce a value of the type to the type modifier
! value given by its second argument.
! </para>
!
! <para>
! When a cast has different source and target types and a function that takes
! more than one argument, it supports converting from one type to another and
! applying a length coercion in a single step. When no such entry is
! available, coercion to a type that uses a type modifier involves two cast
! steps, one to convert between data types and a second to apply the modifier.
! These casts may also define an exemptor function, but it is currently unused.
</para>
<para>
! To minimize unnecessary calls its a cast function, the cast may specify an
! exemptor function. It must take three arguments: an <type>integer</> source
! type modifier, an <type>integer</> destination type modifier, and
! a <type>boolean</> indicating whether the cast is explicit. Its result shall
! be authoritative for all calls to the cast function with the given
! destination type modifier, the given explicitness, and a value most-recently
! coerced to the given source type modifier. It returns an <type>integer</>,
! which may contain the following bits:
!
! <variablelist>
! <varlistentry>
! <term><literal>COERCE_EXEMPT_NOCHANGE</> (<literal>1</>)</term>
! <listitem>
! <para>
! If the cast function completes without error, its result will be
! binary-equivalent to its input.
! </para>
! </listitem>
! </varlistentry>
! <varlistentry>
! <term><literal>COERCE_EXEMPT_NOERROR</> (<literal>2</>)</term>
! <listitem>
! <para>
! The cast function will not throw an error.
! </para>
! </listitem>
! </varlistentry>
! </variablelist>
!
! Consider a length coercion cast implementing a simple notion of length and
! throwing an error when the input value is too long for the destination. Its
! exemptor function might return <literal>COERCE_EXEMPT_NOCHANGE |
! COERCE_EXEMPT_NOERROR</> when the destination type modifier is at least as
! large as the source type modifier.
</para>
</refsect1>
*** a/src/backend/commands/functioncmds.c
--- b/src/backend/commands/functioncmds.c
***************
*** 1487,1492 **** CreateCast(CreateCastStmt *stmt)
--- 1487,1493 ----
char sourcetyptype;
char targettyptype;
Oid funcid;
+ Oid exemptorid;
int nargs;
char castcontext;
char castmethod;
***************
*** 1599,1604 **** CreateCast(CreateCastStmt *stmt)
--- 1600,1669 ----
nargs = 0;
}
+ /* Validate the exemptor function. */
+ if (stmt->exemptor != NULL)
+ {
+ Form_pg_proc procstruct;
+
+ if (nargs == 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("cast function must take two or three arguments to use an exemptor function")));
+
+ exemptorid = LookupFuncNameTypeNames(stmt->exemptor->funcname,
+ stmt->exemptor->funcargs,
+ false);
+
+ tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(exemptorid));
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "cache lookup failed for function %u", exemptorid);
+
+ procstruct = (Form_pg_proc) GETSTRUCT(tuple);
+ if (procstruct->pronargs != 3)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("exemptor function must take three arguments")));
+ if (procstruct->proargtypes.values[0] != INT4OID)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("first argument of exemptor function must be type integer")));
+ if (procstruct->proargtypes.values[1] != INT4OID)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("second argument of exemptor function must be type integer")));
+ if (procstruct->proargtypes.values[2] != BOOLOID)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("third argument of exemptor function must be type boolean")));
+ if (procstruct->prorettype != INT4OID)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("return data type of exemptor function must be type integer")));
+ if (procstruct->provolatile == PROVOLATILE_VOLATILE)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("exemptor function must not be volatile")));
+
+ if (procstruct->proisagg)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("exemptor function must not be an aggregate function")));
+ if (procstruct->proiswindow)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("exemptor function must not be a window function")));
+ if (procstruct->proretset)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+ errmsg("exemptor function must not return a set")));
+
+ ReleaseSysCache(tuple);
+ }
+ else
+ {
+ exemptorid = InvalidOid;
+ }
+
if (castmethod == COERCION_METHOD_BINARY)
{
int16 typ1len;
***************
*** 1725,1730 **** CreateCast(CreateCastStmt *stmt)
--- 1790,1796 ----
values[Anum_pg_cast_castsource - 1] = ObjectIdGetDatum(sourcetypeid);
values[Anum_pg_cast_casttarget - 1] = ObjectIdGetDatum(targettypeid);
values[Anum_pg_cast_castfunc - 1] = ObjectIdGetDatum(funcid);
+ values[Anum_pg_cast_castexemptor - 1] = ObjectIdGetDatum(exemptorid);
values[Anum_pg_cast_castcontext - 1] = CharGetDatum(castcontext);
values[Anum_pg_cast_castmethod - 1] = CharGetDatum(castmethod);
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 1133,1138 **** _copyFuncExpr(FuncExpr *from)
--- 1133,1139 ----
COPY_SCALAR_FIELD(funcresulttype);
COPY_SCALAR_FIELD(funcretset);
COPY_SCALAR_FIELD(funcformat);
+ COPY_SCALAR_FIELD(funcexempt);
COPY_NODE_FIELD(args);
COPY_LOCATION_FIELD(location);
***************
*** 3503,3508 **** _copyCreateCastStmt(CreateCastStmt *from)
--- 3504,3510 ----
COPY_NODE_FIELD(sourcetype);
COPY_NODE_FIELD(targettype);
COPY_NODE_FIELD(func);
+ COPY_NODE_FIELD(exemptor);
COPY_SCALAR_FIELD(context);
COPY_SCALAR_FIELD(inout);
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
***************
*** 236,241 **** _equalFuncExpr(FuncExpr *a, FuncExpr *b)
--- 236,242 ----
b->funcformat != COERCE_DONTCARE)
return false;
+ COMPARE_SCALAR_FIELD(funcexempt);
COMPARE_NODE_FIELD(args);
COMPARE_LOCATION_FIELD(location);
***************
*** 1883,1888 **** _equalCreateCastStmt(CreateCastStmt *a, CreateCastStmt *b)
--- 1884,1890 ----
COMPARE_NODE_FIELD(sourcetype);
COMPARE_NODE_FIELD(targettype);
COMPARE_NODE_FIELD(func);
+ COMPARE_NODE_FIELD(exemptor);
COMPARE_SCALAR_FIELD(context);
COMPARE_SCALAR_FIELD(inout);
*** a/src/backend/nodes/makefuncs.c
--- b/src/backend/nodes/makefuncs.c
***************
*** 447,452 **** makeFuncExpr(Oid funcid, Oid rettype, List *args, CoercionForm fformat)
--- 447,453 ----
funcexpr->funcresulttype = rettype;
funcexpr->funcretset = false; /* only allowed case here */
funcexpr->funcformat = fformat;
+ funcexpr->funcexempt = 0;
funcexpr->args = args;
funcexpr->location = -1;
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
***************
*** 956,961 **** _outFuncExpr(StringInfo str, FuncExpr *node)
--- 956,962 ----
WRITE_OID_FIELD(funcresulttype);
WRITE_BOOL_FIELD(funcretset);
WRITE_ENUM_FIELD(funcformat, CoercionForm);
+ WRITE_INT_FIELD(funcexempt);
WRITE_NODE_FIELD(args);
WRITE_LOCATION_FIELD(location);
}
*** a/src/backend/nodes/readfuncs.c
--- b/src/backend/nodes/readfuncs.c
***************
*** 525,530 **** _readFuncExpr(void)
--- 525,531 ----
READ_OID_FIELD(funcresulttype);
READ_BOOL_FIELD(funcretset);
READ_ENUM_FIELD(funcformat, CoercionForm);
+ READ_INT_FIELD(funcexempt);
READ_NODE_FIELD(args);
READ_LOCATION_FIELD(location);
*** a/src/backend/optimizer/util/clauses.c
--- b/src/backend/optimizer/util/clauses.c
***************
*** 2189,2194 **** eval_const_expressions_mutator(Node *node,
--- 2189,2195 ----
newexpr->funcresulttype = expr->funcresulttype;
newexpr->funcretset = expr->funcretset;
newexpr->funcformat = expr->funcformat;
+ newexpr->funcexempt = 0;
newexpr->args = args;
newexpr->location = expr->location;
return (Node *) newexpr;
***************
*** 3663,3668 **** evaluate_function(Oid funcid, Oid result_type, int32 result_typmod, List *args,
--- 3664,3670 ----
newexpr->funcresulttype = result_type;
newexpr->funcretset = false;
newexpr->funcformat = COERCE_DONTCARE; /* doesn't matter */
+ newexpr->funcexempt = 0;
newexpr->args = args;
newexpr->location = -1;
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 489,495 **** static RangeVar *makeRangeVarFromAnyName(List *names, int position, core_yyscan_
DICTIONARY DISABLE_P DISCARD DISTINCT DO DOCUMENT_P DOMAIN_P DOUBLE_P DROP
EACH ELSE ENABLE_P ENCODING ENCRYPTED END_P ENUM_P ESCAPE EXCEPT
! EXCLUDE EXCLUDING EXCLUSIVE EXECUTE EXISTS EXPLAIN EXTERNAL EXTRACT
FALSE_P FAMILY FETCH FIRST_P FLOAT_P FOLLOWING FOR FORCE FOREIGN FORWARD
FREEZE FROM FULL FUNCTION FUNCTIONS
--- 489,495 ----
DICTIONARY DISABLE_P DISCARD DISTINCT DO DOCUMENT_P DOMAIN_P DOUBLE_P DROP
EACH ELSE ENABLE_P ENCODING ENCRYPTED END_P ENUM_P ESCAPE EXCEPT
! EXCLUDE EXCLUDING EXCLUSIVE EXECUTE EXEMPTOR EXISTS EXPLAIN EXTERNAL EXTRACT
FALSE_P FAMILY FETCH FIRST_P FLOAT_P FOLLOWING FOR FORCE FOREIGN FORWARD
FREEZE FROM FULL FUNCTION FUNCTIONS
***************
*** 5811,5827 **** CreateCastStmt: CREATE CAST '(' Typename AS Typename ')'
--- 5811,5842 ----
n->sourcetype = $4;
n->targettype = $6;
n->func = $10;
+ n->exemptor = NULL;
n->context = (CoercionContext) $11;
n->inout = false;
$$ = (Node *)n;
}
| CREATE CAST '(' Typename AS Typename ')'
+ WITH FUNCTION function_with_argtypes
+ EXEMPTOR function_with_argtypes cast_context
+ {
+ CreateCastStmt *n = makeNode(CreateCastStmt);
+ n->sourcetype = $4;
+ n->targettype = $6;
+ n->func = $10;
+ n->exemptor = $12;
+ n->context = (CoercionContext) $13;
+ n->inout = false;
+ $$ = (Node *)n;
+ }
+ | CREATE CAST '(' Typename AS Typename ')'
WITHOUT FUNCTION cast_context
{
CreateCastStmt *n = makeNode(CreateCastStmt);
n->sourcetype = $4;
n->targettype = $6;
n->func = NULL;
+ n->exemptor = NULL;
n->context = (CoercionContext) $10;
n->inout = false;
$$ = (Node *)n;
***************
*** 5833,5838 **** CreateCastStmt: CREATE CAST '(' Typename AS Typename ')'
--- 5848,5854 ----
n->sourcetype = $4;
n->targettype = $6;
n->func = NULL;
+ n->exemptor = NULL;
n->context = (CoercionContext) $10;
n->inout = true;
$$ = (Node *)n;
***************
*** 11371,11376 **** unreserved_keyword:
--- 11387,11393 ----
| EXCLUDING
| EXCLUSIVE
| EXECUTE
+ | EXEMPTOR
| EXPLAIN
| EXTERNAL
| FAMILY
*** a/src/backend/parser/parse_coerce.c
--- b/src/backend/parser/parse_coerce.c
***************
*** 43,54 **** static Node *build_coercion_expression(Node *node,
Oid funcId,
Oid targetTypeId, int32 targetTypMod,
CoercionForm cformat, int location,
! bool isExplicit);
static Node *coerce_record_to_complex(ParseState *pstate, Node *node,
Oid targetTypeId,
CoercionContext ccontext,
CoercionForm cformat,
int location);
static bool is_complex_array(Oid typid);
static bool typeIsOfTypedTable(Oid reltypeId, Oid reloftypeId);
--- 43,57 ----
Oid funcId,
Oid targetTypeId, int32 targetTypMod,
CoercionForm cformat, int location,
! bool isExplicit, CoerceExemptions exempt);
static Node *coerce_record_to_complex(ParseState *pstate, Node *node,
Oid targetTypeId,
CoercionContext ccontext,
CoercionForm cformat,
int location);
+ static CoerceExemptions apply_exemptor_function(Oid exemptId,
+ int32 sourceTypMod, int32 targetTypMod,
+ bool isExplicit, CoercionPathType *pathtype);
static bool is_complex_array(Oid typid);
static bool typeIsOfTypedTable(Oid reltypeId, Oid reloftypeId);
***************
*** 91,97 **** coerce_to_target_type(ParseState *pstate, Node *expr, Oid exprtype,
ccontext, cformat, location);
/*
! * If the target is a fixed-length type, it may need a length coercion as
* well as a type coercion. If we find ourselves adding both, force the
* inner coercion node to implicit display form.
*/
--- 94,100 ----
ccontext, cformat, location);
/*
! * If the target is a variable-length type, it may need a length coercion as
* well as a type coercion. If we find ourselves adding both, force the
* inner coercion node to implicit display form.
*/
***************
*** 129,137 **** coerce_type(ParseState *pstate, Node *node,
--- 132,144 ----
Oid inputTypeId, Oid targetTypeId, int32 targetTypeMod,
CoercionContext ccontext, CoercionForm cformat, int location)
{
+ Oid baseTypeId;
+ int32 baseTypeMod;
Node *result;
CoercionPathType pathtype;
Oid funcId;
+ Oid exemptId;
+ CoerceExemptions exempt;
if (targetTypeId == inputTypeId ||
node == NULL)
***************
*** 182,189 **** coerce_type(ParseState *pstate, Node *node,
*/
Const *con = (Const *) node;
Const *newcon = makeNode(Const);
- Oid baseTypeId;
- int32 baseTypeMod;
int32 inputTypeMod;
Type targetType;
ParseCallbackState pcbstate;
--- 189,194 ----
***************
*** 278,306 **** coerce_type(ParseState *pstate, Node *node,
if (result)
return result;
}
pathtype = find_coercion_pathway(targetTypeId, inputTypeId, ccontext,
! &funcId);
if (pathtype != COERCION_PATH_NONE)
{
if (pathtype != COERCION_PATH_RELABELTYPE)
{
/*
* Generate an expression tree representing run-time application
! * of the conversion function. If we are dealing with a domain
! * target type, the conversion function will yield the base type,
! * and we need to extract the correct typmod to use from the
! * domain's typtypmod.
*/
- Oid baseTypeId;
- int32 baseTypeMod;
-
- baseTypeMod = targetTypeMod;
- baseTypeId = getBaseTypeAndTypmod(targetTypeId, &baseTypeMod);
-
result = build_coercion_expression(node, pathtype, funcId,
baseTypeId, baseTypeMod,
cformat, location,
! (cformat != COERCE_IMPLICIT_CAST));
/*
* If domain, coerce to the domain type and relabel with domain
--- 283,322 ----
if (result)
return result;
}
+
+ /* Conventional case. */
pathtype = find_coercion_pathway(targetTypeId, inputTypeId, ccontext,
! &funcId, &exemptId);
! if (pathtype != COERCION_PATH_NONE && pathtype != COERCION_PATH_RELABELTYPE)
! {
! /*
! * If we are dealing with a domain target type, the conversion function
! * will yield the base type, and we need to extract the correct typmod
! * to use from the domain's typtypmod.
! */
! baseTypeMod = targetTypeMod;
! baseTypeId = getBaseTypeAndTypmod(targetTypeId, &baseTypeMod);
!
! /* Look for available optimizations. This may change the pathtype. */
! exempt = apply_exemptor_function(exemptId,
! exprTypmod(node), baseTypeMod,
! cformat != COERCE_IMPLICIT_CAST,
! &pathtype);
! }
!
if (pathtype != COERCION_PATH_NONE)
{
if (pathtype != COERCION_PATH_RELABELTYPE)
{
/*
* Generate an expression tree representing run-time application
! * of the conversion function.
*/
result = build_coercion_expression(node, pathtype, funcId,
baseTypeId, baseTypeMod,
cformat, location,
! (cformat != COERCE_IMPLICIT_CAST),
! exempt);
/*
* If domain, coerce to the domain type and relabel with domain
***************
*** 418,423 **** can_coerce_type(int nargs, Oid *input_typeids, Oid *target_typeids,
--- 434,440 ----
Oid targetTypeId = target_typeids[i];
CoercionPathType pathtype;
Oid funcId;
+ Oid exemptId;
/* no problem if same type */
if (inputTypeId == targetTypeId)
***************
*** 446,452 **** can_coerce_type(int nargs, Oid *input_typeids, Oid *target_typeids,
* both binary-compatible and coercion-function cases.
*/
pathtype = find_coercion_pathway(targetTypeId, inputTypeId, ccontext,
! &funcId);
if (pathtype != COERCION_PATH_NONE)
continue;
--- 463,469 ----
* both binary-compatible and coercion-function cases.
*/
pathtype = find_coercion_pathway(targetTypeId, inputTypeId, ccontext,
! &funcId, &exemptId);
if (pathtype != COERCION_PATH_NONE)
continue;
***************
*** 611,616 **** coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod,
--- 628,635 ----
{
CoercionPathType pathtype;
Oid funcId;
+ Oid exemptId;
+ CoerceExemptions exempt;
/*
* A negative typmod is assumed to mean that no coercion is wanted. Also,
***************
*** 619,627 **** coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod,
if (targetTypMod < 0 || targetTypMod == exprTypmod(node))
return node;
! pathtype = find_typmod_coercion_function(targetTypeId, &funcId);
! if (pathtype != COERCION_PATH_NONE)
{
/* Suppress display of nested coercion steps */
if (hideInputCoercion)
--- 638,648 ----
if (targetTypMod < 0 || targetTypMod == exprTypmod(node))
return node;
! pathtype = find_typmod_coercion_function(targetTypeId, &funcId, &exemptId);
! exempt = apply_exemptor_function(exemptId, exprTypmod(node), targetTypMod,
! isExplicit, &pathtype);
! if (pathtype != COERCION_PATH_RELABELTYPE)
{
/* Suppress display of nested coercion steps */
if (hideInputCoercion)
***************
*** 630,636 **** coerce_type_typmod(Node *node, Oid targetTypeId, int32 targetTypMod,
node = build_coercion_expression(node, pathtype, funcId,
targetTypeId, targetTypMod,
cformat, location,
! isExplicit);
}
return node;
--- 651,657 ----
node = build_coercion_expression(node, pathtype, funcId,
targetTypeId, targetTypMod,
cformat, location,
! isExplicit, exempt);
}
return node;
***************
*** 680,686 **** build_coercion_expression(Node *node,
Oid funcId,
Oid targetTypeId, int32 targetTypMod,
CoercionForm cformat, int location,
! bool isExplicit)
{
int nargs = 0;
--- 701,707 ----
Oid funcId,
Oid targetTypeId, int32 targetTypMod,
CoercionForm cformat, int location,
! bool isExplicit, CoerceExemptions exempt)
{
int nargs = 0;
***************
*** 751,756 **** build_coercion_expression(Node *node,
--- 772,778 ----
}
fexpr = makeFuncExpr(funcId, targetTypeId, args, cformat);
+ fexpr->funcexempt = exempt;
fexpr->location = location;
return (Node *) fexpr;
}
***************
*** 1821,1827 **** IsBinaryCoercible(Oid srctype, Oid targettype)
* 3. A CoerceToDomain node inherits any always-noop invariant from its sole
* argument. When GetDomainConstraints() == NIL, it also inherits
* never-error. Otherwise, never-error becomes false.
! * 4. All other nodes have neither invariant.
*
* Returns a bit string that may contain the following bits:
* COERCE_EXEMPT_NOCHANGE: expression result will always have the same binary
--- 1843,1853 ----
* 3. A CoerceToDomain node inherits any always-noop invariant from its sole
* argument. When GetDomainConstraints() == NIL, it also inherits
* never-error. Otherwise, never-error becomes false.
! * 4. A FuncExpr has the intersection of the invariants noted in its funcexempt
! * field and the invariants from its first argument. These nodes will
! * represent calls to length coercion casts, so the first argument holds
! * the datum to coerce.
! * 5. All other nodes have neither invariant.
*
* Returns a bit string that may contain the following bits:
* COERCE_EXEMPT_NOCHANGE: expression result will always have the same binary
***************
*** 1857,1862 **** GetCoerceExemptions(Node *expr,
--- 1883,1897 ----
ret &= ~COERCE_EXEMPT_NOERROR;
expr = (Node *) d->arg;
}
+ else if (IsA(expr, FuncExpr))
+ {
+ FuncExpr *f = (FuncExpr *) expr;
+
+ ret &= f->funcexempt;
+ expr = linitial(f->args);
+ if (expr == NULL)
+ return 0;
+ }
else
{
return 0;
***************
*** 1893,1906 **** GetCoerceExemptions(Node *expr,
* conversion then use IsBinaryCoercible().
*/
CoercionPathType
! find_coercion_pathway(Oid targetTypeId, Oid sourceTypeId,
CoercionContext ccontext,
! Oid *funcid)
{
CoercionPathType result = COERCION_PATH_NONE;
HeapTuple tuple;
*funcid = InvalidOid;
/* Perhaps the types are domains; if so, look at their base types */
if (OidIsValid(sourceTypeId))
--- 1928,1943 ----
* conversion then use IsBinaryCoercible().
*/
CoercionPathType
! find_coercion_pathway(Oid targetTypeId,
! Oid sourceTypeId,
CoercionContext ccontext,
! Oid *funcid, Oid *exemptid)
{
CoercionPathType result = COERCION_PATH_NONE;
HeapTuple tuple;
*funcid = InvalidOid;
+ *exemptid = InvalidOid;
/* Perhaps the types are domains; if so, look at their base types */
if (OidIsValid(sourceTypeId))
***************
*** 1949,1954 **** find_coercion_pathway(Oid targetTypeId, Oid sourceTypeId,
--- 1986,1992 ----
case COERCION_METHOD_FUNCTION:
result = COERCION_PATH_FUNC;
*funcid = castForm->castfunc;
+ *exemptid = castForm->castexemptor;
break;
case COERCION_METHOD_INOUT:
result = COERCION_PATH_COERCEVIAIO;
***************
*** 1992,2006 **** find_coercion_pathway(Oid targetTypeId, Oid sourceTypeId,
{
CoercionPathType elempathtype;
Oid elemfuncid;
elempathtype = find_coercion_pathway(targetElem,
sourceElem,
ccontext,
! &elemfuncid);
if (elempathtype != COERCION_PATH_NONE &&
elempathtype != COERCION_PATH_ARRAYCOERCE)
{
*funcid = elemfuncid;
if (elempathtype == COERCION_PATH_COERCEVIAIO)
result = COERCION_PATH_COERCEVIAIO;
else
--- 2030,2047 ----
{
CoercionPathType elempathtype;
Oid elemfuncid;
+ Oid elemexemptorid;
elempathtype = find_coercion_pathway(targetElem,
sourceElem,
ccontext,
! &elemfuncid,
! &elemexemptorid);
if (elempathtype != COERCION_PATH_NONE &&
elempathtype != COERCION_PATH_ARRAYCOERCE)
{
*funcid = elemfuncid;
+ *exemptid = elemexemptorid;
if (elempathtype == COERCION_PATH_COERCEVIAIO)
result = COERCION_PATH_COERCEVIAIO;
else
***************
*** 2038,2044 **** find_coercion_pathway(Oid targetTypeId, Oid sourceTypeId,
* find_typmod_coercion_function -- does the given type need length coercion?
*
* If the target type possesses a pg_cast function from itself to itself,
! * it must need length coercion.
*
* "bpchar" (ie, char(N)) and "numeric" are examples of such types.
*
--- 2079,2085 ----
* find_typmod_coercion_function -- does the given type need length coercion?
*
* If the target type possesses a pg_cast function from itself to itself,
! * it needs length coercion at least some of the time.
*
* "bpchar" (ie, char(N)) and "numeric" are examples of such types.
*
***************
*** 2050,2061 **** find_coercion_pathway(Oid targetTypeId, Oid sourceTypeId,
* We use the same result enum as find_coercion_pathway, but the only possible
* result codes are:
* COERCION_PATH_NONE: no length coercion needed
! * COERCION_PATH_FUNC: apply the function returned in *funcid
* COERCION_PATH_ARRAYCOERCE: apply the function using ArrayCoerceExpr
*/
CoercionPathType
find_typmod_coercion_function(Oid typeId,
! Oid *funcid)
{
CoercionPathType result;
Type targetType;
--- 2091,2103 ----
* We use the same result enum as find_coercion_pathway, but the only possible
* result codes are:
* COERCION_PATH_NONE: no length coercion needed
! * COERCION_PATH_FUNC: apply the function returned in *funcid, subject to
! * exemptions in *exempt
* COERCION_PATH_ARRAYCOERCE: apply the function using ArrayCoerceExpr
*/
CoercionPathType
find_typmod_coercion_function(Oid typeId,
! Oid *funcid, Oid *exemptid)
{
CoercionPathType result;
Type targetType;
***************
*** 2063,2068 **** find_typmod_coercion_function(Oid typeId,
--- 2105,2111 ----
HeapTuple tuple;
*funcid = InvalidOid;
+ *exemptid = InvalidOid;
result = COERCION_PATH_FUNC;
targetType = typeidType(typeId);
***************
*** 2087,2101 **** find_typmod_coercion_function(Oid typeId,
Form_pg_cast castForm = (Form_pg_cast) GETSTRUCT(tuple);
*funcid = castForm->castfunc;
ReleaseSysCache(tuple);
}
if (!OidIsValid(*funcid))
! result = COERCION_PATH_NONE;
return result;
}
/*
* is_complex_array
* Is this type an array of composite?
--- 2130,2171 ----
Form_pg_cast castForm = (Form_pg_cast) GETSTRUCT(tuple);
*funcid = castForm->castfunc;
+ *exemptid = castForm->castexemptor;
ReleaseSysCache(tuple);
}
if (!OidIsValid(*funcid))
! result = COERCION_PATH_RELABELTYPE;
return result;
}
+
+ static CoerceExemptions
+ apply_exemptor_function(Oid exemptId,
+ int32 sourceTypMod, int32 targetTypMod,
+ bool isExplicit, CoercionPathType *pathtype)
+ {
+ CoerceExemptions ret = 0;
+
+ /*
+ * Call the exemptor function, if any; its verdict may let us skip calls to
+ * the length coercion function entirely.
+ */
+ if (OidIsValid(exemptId))
+ {
+ ret = DatumGetInt32(OidFunctionCall3(exemptId,
+ Int32GetDatum(sourceTypMod),
+ Int32GetDatum(targetTypMod),
+ BoolGetDatum(isExplicit)));
+ if (ret == (COERCE_EXEMPT_NOCHANGE | COERCE_EXEMPT_NOERROR))
+ *pathtype = COERCION_PATH_RELABELTYPE;
+ }
+
+ return ret;
+ }
+
+
/*
* is_complex_array
* Is this type an array of composite?
*** a/src/backend/parser/parse_func.c
--- b/src/backend/parser/parse_func.c
***************
*** 382,387 **** ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs,
--- 382,388 ----
funcexpr->funcresulttype = rettype;
funcexpr->funcretset = retset;
funcexpr->funcformat = COERCE_EXPLICIT_CALL;
+ funcexpr->funcexempt = 0;
funcexpr->args = fargs;
funcexpr->location = location;
***************
*** 1018,1027 **** func_get_detail(List *funcname,
{
CoercionPathType cpathtype;
Oid cfuncid;
cpathtype = find_coercion_pathway(targetType, sourceType,
COERCION_EXPLICIT,
! &cfuncid);
switch (cpathtype)
{
case COERCION_PATH_RELABELTYPE:
--- 1019,1029 ----
{
CoercionPathType cpathtype;
Oid cfuncid;
+ Oid cexemptid;
cpathtype = find_coercion_pathway(targetType, sourceType,
COERCION_EXPLICIT,
! &cfuncid, &cexemptid);
switch (cpathtype)
{
case COERCION_PATH_RELABELTYPE:
*** a/src/backend/utils/adt/ri_triggers.c
--- b/src/backend/utils/adt/ri_triggers.c
***************
*** 3950,3956 **** ri_HashCompareOp(Oid eq_opr, Oid typeid)
{
Oid lefttype,
righttype,
! castfunc;
CoercionPathType pathtype;
/* We always need to know how to call the equality operator */
--- 3950,3957 ----
{
Oid lefttype,
righttype,
! castfunc,
! castexempt;
CoercionPathType pathtype;
/* We always need to know how to call the equality operator */
***************
*** 3977,3983 **** ri_HashCompareOp(Oid eq_opr, Oid typeid)
{
pathtype = find_coercion_pathway(lefttype, typeid,
COERCION_IMPLICIT,
! &castfunc);
if (pathtype != COERCION_PATH_FUNC &&
pathtype != COERCION_PATH_RELABELTYPE)
{
--- 3978,3984 ----
{
pathtype = find_coercion_pathway(lefttype, typeid,
COERCION_IMPLICIT,
! &castfunc, &castexempt);
if (pathtype != COERCION_PATH_FUNC &&
pathtype != COERCION_PATH_RELABELTYPE)
{
*** a/src/backend/utils/adt/varchar.c
--- b/src/backend/utils/adt/varchar.c
***************
*** 549,554 **** varcharsend(PG_FUNCTION_ARGS)
--- 549,568 ----
/*
+ * Identify superfluous calls to our length coercion function.
+ */
+ Datum
+ varchar_exemptor(PG_FUNCTION_ARGS)
+ {
+ int32 old_max = PG_GETARG_INT32(0) - VARHDRSZ;
+ int32 new_max = PG_GETARG_INT32(1) - VARHDRSZ;
+
+ if (new_max < 0 || (old_max >= 0 && old_max <= new_max))
+ PG_RETURN_INT32(COERCE_EXEMPT_NOCHANGE | COERCE_EXEMPT_NOERROR);
+ PG_RETURN_INT32(0);
+ }
+
+ /*
* Converts a VARCHAR type to the specified size.
*
* maxlen is the typmod, ie, declared length plus VARHDRSZ bytes.
*** a/src/backend/utils/adt/xml.c
--- b/src/backend/utils/adt/xml.c
***************
*** 508,513 **** xmlconcat2(PG_FUNCTION_ARGS)
--- 508,520 ----
}
+ /* texttoxml can only throw an error, never change the data. */
+ Datum
+ xml_exemptor(PG_FUNCTION_ARGS)
+ {
+ PG_RETURN_INT32(COERCE_EXEMPT_NOCHANGE);
+ }
+
Datum
texttoxml(PG_FUNCTION_ARGS)
{
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
***************
*** 5031,5054 **** getCasts(int *numCasts)
int i_castsource;
int i_casttarget;
int i_castfunc;
int i_castcontext;
int i_castmethod;
/* Make sure we are in proper schema */
selectSourceSchema("pg_catalog");
! if (g_fout->remoteVersion >= 80400)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, "
"castsource, casttarget, castfunc, castcontext, "
! "castmethod "
"FROM pg_cast ORDER BY 3,4");
}
else if (g_fout->remoteVersion >= 70300)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, "
"castsource, casttarget, castfunc, castcontext, "
! "CASE WHEN castfunc = 0 THEN 'b' ELSE 'f' END AS castmethod "
"FROM pg_cast ORDER BY 3,4");
}
else
--- 5031,5063 ----
int i_castsource;
int i_casttarget;
int i_castfunc;
+ int i_castexemptor;
int i_castcontext;
int i_castmethod;
/* Make sure we are in proper schema */
selectSourceSchema("pg_catalog");
! if (g_fout->remoteVersion >= 90100)
! {
! appendPQExpBuffer(query, "SELECT tableoid, oid, "
! "castsource, casttarget, castfunc, castcontext, "
! "castmethod, castexemptor "
! "FROM pg_cast ORDER BY 3,4");
! }
! else if (g_fout->remoteVersion >= 80400)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, "
"castsource, casttarget, castfunc, castcontext, "
! "castmethod, 0 AS castexemptor "
"FROM pg_cast ORDER BY 3,4");
}
else if (g_fout->remoteVersion >= 70300)
{
appendPQExpBuffer(query, "SELECT tableoid, oid, "
"castsource, casttarget, castfunc, castcontext, "
! "CASE WHEN castfunc = 0 THEN 'b' ELSE 'f' END AS castmethod, "
! "0 AS castexemptor "
"FROM pg_cast ORDER BY 3,4");
}
else
***************
*** 5056,5062 **** getCasts(int *numCasts)
appendPQExpBuffer(query, "SELECT 0 AS tableoid, p.oid, "
"t1.oid AS castsource, t2.oid AS casttarget, "
"p.oid AS castfunc, 'e' AS castcontext, "
! "'f' AS castmethod "
"FROM pg_type t1, pg_type t2, pg_proc p "
"WHERE p.pronargs = 1 AND "
"p.proargtypes[0] = t1.oid AND "
--- 5065,5072 ----
appendPQExpBuffer(query, "SELECT 0 AS tableoid, p.oid, "
"t1.oid AS castsource, t2.oid AS casttarget, "
"p.oid AS castfunc, 'e' AS castcontext, "
! "'f' AS castmethod, "
! "0 AS castexemptor "
"FROM pg_type t1, pg_type t2, pg_proc p "
"WHERE p.pronargs = 1 AND "
"p.proargtypes[0] = t1.oid AND "
***************
*** 5078,5083 **** getCasts(int *numCasts)
--- 5088,5094 ----
i_castsource = PQfnumber(res, "castsource");
i_casttarget = PQfnumber(res, "casttarget");
i_castfunc = PQfnumber(res, "castfunc");
+ i_castexemptor = PQfnumber(res, "castexemptor");
i_castcontext = PQfnumber(res, "castcontext");
i_castmethod = PQfnumber(res, "castmethod");
***************
*** 5094,5099 **** getCasts(int *numCasts)
--- 5105,5111 ----
castinfo[i].castsource = atooid(PQgetvalue(res, i, i_castsource));
castinfo[i].casttarget = atooid(PQgetvalue(res, i, i_casttarget));
castinfo[i].castfunc = atooid(PQgetvalue(res, i, i_castfunc));
+ castinfo[i].castexemptor = atooid(PQgetvalue(res, i, i_castexemptor));
castinfo[i].castcontext = *(PQgetvalue(res, i, i_castcontext));
castinfo[i].castmethod = *(PQgetvalue(res, i, i_castmethod));
***************
*** 5124,5129 **** getCasts(int *numCasts)
--- 5136,5142 ----
addObjectDependency(&castinfo[i].dobj,
funcInfo->dobj.dumpId);
}
+ /* Exemptor functions postdate the dependency mechanism. */
}
PQclear(res);
***************
*** 8330,8335 **** dumpCast(Archive *fout, CastInfo *cast)
--- 8343,8349 ----
PQExpBuffer delqry;
PQExpBuffer castsig;
FuncInfo *funcInfo = NULL;
+ FuncInfo *exemptInfo = NULL;
TypeInfo *sourceInfo;
TypeInfo *targetInfo;
***************
*** 8340,8345 **** dumpCast(Archive *fout, CastInfo *cast)
--- 8354,8367 ----
{
funcInfo = findFuncByOid(cast->castfunc);
if (funcInfo == NULL)
+ /* XXX: is silently bailing correct? */
+ return;
+ }
+ if (OidIsValid(cast->castexemptor))
+ {
+ exemptInfo = findFuncByOid(cast->castexemptor);
+ if (exemptInfo == NULL)
+ /* XXX: is silently bailing correct? */
return;
}
***************
*** 8360,8375 **** dumpCast(Archive *fout, CastInfo *cast)
*/
if ((funcInfo == NULL ||
strncmp(funcInfo->dobj.namespace->dobj.name, "pg_", 3) == 0) &&
strncmp(sourceInfo->dobj.namespace->dobj.name, "pg_", 3) == 0 &&
strncmp(targetInfo->dobj.namespace->dobj.name, "pg_", 3) == 0)
return;
/*
! * Skip cast if function isn't from pg_ and is not to be dumped.
*/
! if (funcInfo &&
! strncmp(funcInfo->dobj.namespace->dobj.name, "pg_", 3) != 0 &&
! !funcInfo->dobj.dump)
return;
/*
--- 8382,8402 ----
*/
if ((funcInfo == NULL ||
strncmp(funcInfo->dobj.namespace->dobj.name, "pg_", 3) == 0) &&
+ (exemptInfo == NULL ||
+ strncmp(exemptInfo->dobj.namespace->dobj.name, "pg_", 3) == 0) &&
strncmp(sourceInfo->dobj.namespace->dobj.name, "pg_", 3) == 0 &&
strncmp(targetInfo->dobj.namespace->dobj.name, "pg_", 3) == 0)
return;
/*
! * Skip cast if a function isn't from pg_ and is not to be dumped.
*/
! if ((funcInfo &&
! strncmp(funcInfo->dobj.namespace->dobj.name, "pg_", 3) != 0 &&
! !funcInfo->dobj.dump) ||
! (exemptInfo &&
! strncmp(exemptInfo->dobj.namespace->dobj.name, "pg_", 3) != 0 &&
! !exemptInfo->dobj.dump))
return;
/*
***************
*** 8419,8424 **** dumpCast(Archive *fout, CastInfo *cast)
--- 8446,8459 ----
fmtId(funcInfo->dobj.namespace->dobj.name));
appendPQExpBuffer(defqry, "%s",
format_function_signature(funcInfo, true));
+
+ if (exemptInfo != NULL)
+ {
+ appendPQExpBuffer(defqry, " EXEMPTOR %s.",
+ fmtId(exemptInfo->dobj.namespace->dobj.name));
+ appendPQExpBuffer(defqry, "%s",
+ format_function_signature(exemptInfo, true));
+ }
break;
default:
write_msg(NULL, "WARNING: bogus value in pg_cast.castmethod field\n");
*** a/src/bin/pg_dump/pg_dump.h
--- b/src/bin/pg_dump/pg_dump.h
***************
*** 373,378 **** typedef struct _castInfo
--- 373,379 ----
Oid castsource;
Oid casttarget;
Oid castfunc;
+ Oid castexemptor;
char castcontext;
char castmethod;
} CastInfo;
*** a/src/include/catalog/catversion.h
--- b/src/include/catalog/catversion.h
***************
*** 53,58 ****
*/
/* yyyymmddN */
! #define CATALOG_VERSION_NO 201101081
#endif
--- 53,58 ----
*/
/* yyyymmddN */
! #define CATALOG_VERSION_NO 201101101
#endif
*** a/src/include/catalog/pg_cast.h
--- b/src/include/catalog/pg_cast.h
***************
*** 35,40 **** CATALOG(pg_cast,2605)
--- 35,41 ----
Oid castsource; /* source datatype for cast */
Oid casttarget; /* destination datatype for cast */
Oid castfunc; /* cast function; 0 = binary coercible */
+ Oid castexemptor; /* exemptor function; 0 = none */
char castcontext; /* contexts in which cast can be used */
char castmethod; /* cast method */
} FormData_pg_cast;
***************
*** 74,85 **** typedef enum CoercionMethod
* compiler constants for pg_cast
* ----------------
*/
! #define Natts_pg_cast 5
#define Anum_pg_cast_castsource 1
#define Anum_pg_cast_casttarget 2
#define Anum_pg_cast_castfunc 3
! #define Anum_pg_cast_castcontext 4
! #define Anum_pg_cast_castmethod 5
/* ----------------
* initial contents of pg_cast
--- 75,87 ----
* compiler constants for pg_cast
* ----------------
*/
! #define Natts_pg_cast 6
#define Anum_pg_cast_castsource 1
#define Anum_pg_cast_casttarget 2
#define Anum_pg_cast_castfunc 3
! #define Anum_pg_cast_castexemptor 4
! #define Anum_pg_cast_castcontext 5
! #define Anum_pg_cast_castmethod 6
/* ----------------
* initial contents of pg_cast
***************
*** 94,135 **** typedef enum CoercionMethod
* int2->int4->int8->numeric->float4->float8, while casts in the
* reverse direction are assignment-only.
*/
! DATA(insert ( 20 21 714 a f ));
! DATA(insert ( 20 23 480 a f ));
! DATA(insert ( 20 700 652 i f ));
! DATA(insert ( 20 701 482 i f ));
! DATA(insert ( 20 1700 1781 i f ));
! DATA(insert ( 21 20 754 i f ));
! DATA(insert ( 21 23 313 i f ));
! DATA(insert ( 21 700 236 i f ));
! DATA(insert ( 21 701 235 i f ));
! DATA(insert ( 21 1700 1782 i f ));
! DATA(insert ( 23 20 481 i f ));
! DATA(insert ( 23 21 314 a f ));
! DATA(insert ( 23 700 318 i f ));
! DATA(insert ( 23 701 316 i f ));
! DATA(insert ( 23 1700 1740 i f ));
! DATA(insert ( 700 20 653 a f ));
! DATA(insert ( 700 21 238 a f ));
! DATA(insert ( 700 23 319 a f ));
! DATA(insert ( 700 701 311 i f ));
! DATA(insert ( 700 1700 1742 a f ));
! DATA(insert ( 701 20 483 a f ));
! DATA(insert ( 701 21 237 a f ));
! DATA(insert ( 701 23 317 a f ));
! DATA(insert ( 701 700 312 a f ));
! DATA(insert ( 701 1700 1743 a f ));
! DATA(insert ( 1700 20 1779 a f ));
! DATA(insert ( 1700 21 1783 a f ));
! DATA(insert ( 1700 23 1744 a f ));
! DATA(insert ( 1700 700 1745 i f ));
! DATA(insert ( 1700 701 1746 i f ));
! DATA(insert ( 790 1700 3823 a f ));
! DATA(insert ( 1700 790 3824 a f ));
/* Allow explicit coercions between int4 and bool */
! DATA(insert ( 23 16 2557 e f ));
! DATA(insert ( 16 23 2558 e f ));
/*
* OID category: allow implicit conversion from any integral type (including
--- 96,137 ----
* int2->int4->int8->numeric->float4->float8, while casts in the
* reverse direction are assignment-only.
*/
! DATA(insert ( 20 21 714 0 a f ));
! DATA(insert ( 20 23 480 0 a f ));
! DATA(insert ( 20 700 652 0 i f ));
! DATA(insert ( 20 701 482 0 i f ));
! DATA(insert ( 20 1700 1781 0 i f ));
! DATA(insert ( 21 20 754 0 i f ));
! DATA(insert ( 21 23 313 0 i f ));
! DATA(insert ( 21 700 236 0 i f ));
! DATA(insert ( 21 701 235 0 i f ));
! DATA(insert ( 21 1700 1782 0 i f ));
! DATA(insert ( 23 20 481 0 i f ));
! DATA(insert ( 23 21 314 0 a f ));
! DATA(insert ( 23 700 318 0 i f ));
! DATA(insert ( 23 701 316 0 i f ));
! DATA(insert ( 23 1700 1740 0 i f ));
! DATA(insert ( 700 20 653 0 a f ));
! DATA(insert ( 700 21 238 0 a f ));
! DATA(insert ( 700 23 319 0 a f ));
! DATA(insert ( 700 701 311 0 i f ));
! DATA(insert ( 700 1700 1742 0 a f ));
! DATA(insert ( 701 20 483 0 a f ));
! DATA(insert ( 701 21 237 0 a f ));
! DATA(insert ( 701 23 317 0 a f ));
! DATA(insert ( 701 700 312 0 a f ));
! DATA(insert ( 701 1700 1743 0 a f ));
! DATA(insert ( 1700 20 1779 0 a f ));
! DATA(insert ( 1700 21 1783 0 a f ));
! DATA(insert ( 1700 23 1744 0 a f ));
! DATA(insert ( 1700 700 1745 0 i f ));
! DATA(insert ( 1700 701 1746 0 i f ));
! DATA(insert ( 790 1700 3823 0 a f ));
! DATA(insert ( 1700 790 3824 0 a f ));
/* Allow explicit coercions between int4 and bool */
! DATA(insert ( 23 16 2557 0 e f ));
! DATA(insert ( 16 23 2558 0 e f ));
/*
* OID category: allow implicit conversion from any integral type (including
***************
*** 141,307 **** DATA(insert ( 16 23 2558 e f ));
* casts from text and varchar to regclass, which exist mainly to support
* legacy forms of nextval() and related functions.
*/
! DATA(insert ( 20 26 1287 i f ));
! DATA(insert ( 21 26 313 i f ));
! DATA(insert ( 23 26 0 i b ));
! DATA(insert ( 26 20 1288 a f ));
! DATA(insert ( 26 23 0 a b ));
! DATA(insert ( 26 24 0 i b ));
! DATA(insert ( 24 26 0 i b ));
! DATA(insert ( 20 24 1287 i f ));
! DATA(insert ( 21 24 313 i f ));
! DATA(insert ( 23 24 0 i b ));
! DATA(insert ( 24 20 1288 a f ));
! DATA(insert ( 24 23 0 a b ));
! DATA(insert ( 24 2202 0 i b ));
! DATA(insert ( 2202 24 0 i b ));
! DATA(insert ( 26 2202 0 i b ));
! DATA(insert ( 2202 26 0 i b ));
! DATA(insert ( 20 2202 1287 i f ));
! DATA(insert ( 21 2202 313 i f ));
! DATA(insert ( 23 2202 0 i b ));
! DATA(insert ( 2202 20 1288 a f ));
! DATA(insert ( 2202 23 0 a b ));
! DATA(insert ( 26 2203 0 i b ));
! DATA(insert ( 2203 26 0 i b ));
! DATA(insert ( 20 2203 1287 i f ));
! DATA(insert ( 21 2203 313 i f ));
! DATA(insert ( 23 2203 0 i b ));
! DATA(insert ( 2203 20 1288 a f ));
! DATA(insert ( 2203 23 0 a b ));
! DATA(insert ( 2203 2204 0 i b ));
! DATA(insert ( 2204 2203 0 i b ));
! DATA(insert ( 26 2204 0 i b ));
! DATA(insert ( 2204 26 0 i b ));
! DATA(insert ( 20 2204 1287 i f ));
! DATA(insert ( 21 2204 313 i f ));
! DATA(insert ( 23 2204 0 i b ));
! DATA(insert ( 2204 20 1288 a f ));
! DATA(insert ( 2204 23 0 a b ));
! DATA(insert ( 26 2205 0 i b ));
! DATA(insert ( 2205 26 0 i b ));
! DATA(insert ( 20 2205 1287 i f ));
! DATA(insert ( 21 2205 313 i f ));
! DATA(insert ( 23 2205 0 i b ));
! DATA(insert ( 2205 20 1288 a f ));
! DATA(insert ( 2205 23 0 a b ));
! DATA(insert ( 26 2206 0 i b ));
! DATA(insert ( 2206 26 0 i b ));
! DATA(insert ( 20 2206 1287 i f ));
! DATA(insert ( 21 2206 313 i f ));
! DATA(insert ( 23 2206 0 i b ));
! DATA(insert ( 2206 20 1288 a f ));
! DATA(insert ( 2206 23 0 a b ));
! DATA(insert ( 26 3734 0 i b ));
! DATA(insert ( 3734 26 0 i b ));
! DATA(insert ( 20 3734 1287 i f ));
! DATA(insert ( 21 3734 313 i f ));
! DATA(insert ( 23 3734 0 i b ));
! DATA(insert ( 3734 20 1288 a f ));
! DATA(insert ( 3734 23 0 a b ));
! DATA(insert ( 26 3769 0 i b ));
! DATA(insert ( 3769 26 0 i b ));
! DATA(insert ( 20 3769 1287 i f ));
! DATA(insert ( 21 3769 313 i f ));
! DATA(insert ( 23 3769 0 i b ));
! DATA(insert ( 3769 20 1288 a f ));
! DATA(insert ( 3769 23 0 a b ));
! DATA(insert ( 25 2205 1079 i f ));
! DATA(insert ( 1043 2205 1079 i f ));
/*
* String category
*/
! DATA(insert ( 25 1042 0 i b ));
! DATA(insert ( 25 1043 0 i b ));
! DATA(insert ( 1042 25 401 i f ));
! DATA(insert ( 1042 1043 401 i f ));
! DATA(insert ( 1043 25 0 i b ));
! DATA(insert ( 1043 1042 0 i b ));
! DATA(insert ( 18 25 946 i f ));
! DATA(insert ( 18 1042 860 a f ));
! DATA(insert ( 18 1043 946 a f ));
! DATA(insert ( 19 25 406 i f ));
! DATA(insert ( 19 1042 408 a f ));
! DATA(insert ( 19 1043 1401 a f ));
! DATA(insert ( 25 18 944 a f ));
! DATA(insert ( 1042 18 944 a f ));
! DATA(insert ( 1043 18 944 a f ));
! DATA(insert ( 25 19 407 i f ));
! DATA(insert ( 1042 19 409 i f ));
! DATA(insert ( 1043 19 1400 i f ));
/* Allow explicit coercions between int4 and "char" */
! DATA(insert ( 18 23 77 e f ));
! DATA(insert ( 23 18 78 e f ));
/* pg_node_tree can be coerced to, but not from, text */
! DATA(insert ( 194 25 0 i b ));
/*
* Datetime category
*/
! DATA(insert ( 702 1082 1179 a f ));
! DATA(insert ( 702 1083 1364 a f ));
! DATA(insert ( 702 1114 2023 i f ));
! DATA(insert ( 702 1184 1173 i f ));
! DATA(insert ( 703 1186 1177 i f ));
! DATA(insert ( 1082 1114 2024 i f ));
! DATA(insert ( 1082 1184 1174 i f ));
! DATA(insert ( 1083 1186 1370 i f ));
! DATA(insert ( 1083 1266 2047 i f ));
! DATA(insert ( 1114 702 2030 a f ));
! DATA(insert ( 1114 1082 2029 a f ));
! DATA(insert ( 1114 1083 1316 a f ));
! DATA(insert ( 1114 1184 2028 i f ));
! DATA(insert ( 1184 702 1180 a f ));
! DATA(insert ( 1184 1082 1178 a f ));
! DATA(insert ( 1184 1083 2019 a f ));
! DATA(insert ( 1184 1114 2027 a f ));
! DATA(insert ( 1184 1266 1388 a f ));
! DATA(insert ( 1186 703 1194 a f ));
! DATA(insert ( 1186 1083 1419 a f ));
! DATA(insert ( 1266 1083 2046 a f ));
/* Cross-category casts between int4 and abstime, reltime */
! DATA(insert ( 23 702 0 e b ));
! DATA(insert ( 702 23 0 e b ));
! DATA(insert ( 23 703 0 e b ));
! DATA(insert ( 703 23 0 e b ));
/*
* Geometric category
*/
! DATA(insert ( 601 600 1532 e f ));
! DATA(insert ( 602 600 1533 e f ));
! DATA(insert ( 602 604 1449 a f ));
! DATA(insert ( 603 600 1534 e f ));
! DATA(insert ( 603 601 1541 e f ));
! DATA(insert ( 603 604 1448 a f ));
! DATA(insert ( 603 718 1479 e f ));
! DATA(insert ( 604 600 1540 e f ));
! DATA(insert ( 604 602 1447 a f ));
! DATA(insert ( 604 603 1446 e f ));
! DATA(insert ( 604 718 1474 e f ));
! DATA(insert ( 718 600 1416 e f ));
! DATA(insert ( 718 603 1480 e f ));
! DATA(insert ( 718 604 1544 e f ));
/*
* INET category
*/
! DATA(insert ( 650 869 0 i b ));
! DATA(insert ( 869 650 1715 a f ));
/*
* BitString category
*/
! DATA(insert ( 1560 1562 0 i b ));
! DATA(insert ( 1562 1560 0 i b ));
/* Cross-category casts between bit and int4, int8 */
! DATA(insert ( 20 1560 2075 e f ));
! DATA(insert ( 23 1560 1683 e f ));
! DATA(insert ( 1560 20 2076 e f ));
! DATA(insert ( 1560 23 1684 e f ));
/*
* Cross-category casts to and from TEXT
--- 143,309 ----
* casts from text and varchar to regclass, which exist mainly to support
* legacy forms of nextval() and related functions.
*/
! DATA(insert ( 20 26 1287 0 i f ));
! DATA(insert ( 21 26 313 0 i f ));
! DATA(insert ( 23 26 0 0 i b ));
! DATA(insert ( 26 20 1288 0 a f ));
! DATA(insert ( 26 23 0 0 a b ));
! DATA(insert ( 26 24 0 0 i b ));
! DATA(insert ( 24 26 0 0 i b ));
! DATA(insert ( 20 24 1287 0 i f ));
! DATA(insert ( 21 24 313 0 i f ));
! DATA(insert ( 23 24 0 0 i b ));
! DATA(insert ( 24 20 1288 0 a f ));
! DATA(insert ( 24 23 0 0 a b ));
! DATA(insert ( 24 2202 0 0 i b ));
! DATA(insert ( 2202 24 0 0 i b ));
! DATA(insert ( 26 2202 0 0 i b ));
! DATA(insert ( 2202 26 0 0 i b ));
! DATA(insert ( 20 2202 1287 0 i f ));
! DATA(insert ( 21 2202 313 0 i f ));
! DATA(insert ( 23 2202 0 0 i b ));
! DATA(insert ( 2202 20 1288 0 a f ));
! DATA(insert ( 2202 23 0 0 a b ));
! DATA(insert ( 26 2203 0 0 i b ));
! DATA(insert ( 2203 26 0 0 i b ));
! DATA(insert ( 20 2203 1287 0 i f ));
! DATA(insert ( 21 2203 313 0 i f ));
! DATA(insert ( 23 2203 0 0 i b ));
! DATA(insert ( 2203 20 1288 0 a f ));
! DATA(insert ( 2203 23 0 0 a b ));
! DATA(insert ( 2203 2204 0 0 i b ));
! DATA(insert ( 2204 2203 0 0 i b ));
! DATA(insert ( 26 2204 0 0 i b ));
! DATA(insert ( 2204 26 0 0 i b ));
! DATA(insert ( 20 2204 1287 0 i f ));
! DATA(insert ( 21 2204 313 0 i f ));
! DATA(insert ( 23 2204 0 0 i b ));
! DATA(insert ( 2204 20 1288 0 a f ));
! DATA(insert ( 2204 23 0 0 a b ));
! DATA(insert ( 26 2205 0 0 i b ));
! DATA(insert ( 2205 26 0 0 i b ));
! DATA(insert ( 20 2205 1287 0 i f ));
! DATA(insert ( 21 2205 313 0 i f ));
! DATA(insert ( 23 2205 0 0 i b ));
! DATA(insert ( 2205 20 1288 0 a f ));
! DATA(insert ( 2205 23 0 0 a b ));
! DATA(insert ( 26 2206 0 0 i b ));
! DATA(insert ( 2206 26 0 0 i b ));
! DATA(insert ( 20 2206 1287 0 i f ));
! DATA(insert ( 21 2206 313 0 i f ));
! DATA(insert ( 23 2206 0 0 i b ));
! DATA(insert ( 2206 20 1288 0 a f ));
! DATA(insert ( 2206 23 0 0 a b ));
! DATA(insert ( 26 3734 0 0 i b ));
! DATA(insert ( 3734 26 0 0 i b ));
! DATA(insert ( 20 3734 1287 0 i f ));
! DATA(insert ( 21 3734 313 0 i f ));
! DATA(insert ( 23 3734 0 0 i b ));
! DATA(insert ( 3734 20 1288 0 a f ));
! DATA(insert ( 3734 23 0 0 a b ));
! DATA(insert ( 26 3769 0 0 i b ));
! DATA(insert ( 3769 26 0 0 i b ));
! DATA(insert ( 20 3769 1287 0 i f ));
! DATA(insert ( 21 3769 313 0 i f ));
! DATA(insert ( 23 3769 0 0 i b ));
! DATA(insert ( 3769 20 1288 0 a f ));
! DATA(insert ( 3769 23 0 0 a b ));
! DATA(insert ( 25 2205 1079 0 i f ));
! DATA(insert ( 1043 2205 1079 0 i f ));
/*
* String category
*/
! DATA(insert ( 25 1042 0 0 i b ));
! DATA(insert ( 25 1043 0 0 i b ));
! DATA(insert ( 1042 25 401 0 i f ));
! DATA(insert ( 1042 1043 401 0 i f ));
! DATA(insert ( 1043 25 0 0 i b ));
! DATA(insert ( 1043 1042 0 0 i b ));
! DATA(insert ( 18 25 946 0 i f ));
! DATA(insert ( 18 1042 860 0 a f ));
! DATA(insert ( 18 1043 946 0 a f ));
! DATA(insert ( 19 25 406 0 i f ));
! DATA(insert ( 19 1042 408 0 a f ));
! DATA(insert ( 19 1043 1401 0 a f ));
! DATA(insert ( 25 18 944 0 a f ));
! DATA(insert ( 1042 18 944 0 a f ));
! DATA(insert ( 1043 18 944 0 a f ));
! DATA(insert ( 25 19 407 0 i f ));
! DATA(insert ( 1042 19 409 0 i f ));
! DATA(insert ( 1043 19 1400 0 i f ));
/* Allow explicit coercions between int4 and "char" */
! DATA(insert ( 18 23 77 0 e f ));
! DATA(insert ( 23 18 78 0 e f ));
/* pg_node_tree can be coerced to, but not from, text */
! DATA(insert ( 194 25 0 0 i b ));
/*
* Datetime category
*/
! DATA(insert ( 702 1082 1179 0 a f ));
! DATA(insert ( 702 1083 1364 0 a f ));
! DATA(insert ( 702 1114 2023 0 i f ));
! DATA(insert ( 702 1184 1173 0 i f ));
! DATA(insert ( 703 1186 1177 0 i f ));
! DATA(insert ( 1082 1114 2024 0 i f ));
! DATA(insert ( 1082 1184 1174 0 i f ));
! DATA(insert ( 1083 1186 1370 0 i f ));
! DATA(insert ( 1083 1266 2047 0 i f ));
! DATA(insert ( 1114 702 2030 0 a f ));
! DATA(insert ( 1114 1082 2029 0 a f ));
! DATA(insert ( 1114 1083 1316 0 a f ));
! DATA(insert ( 1114 1184 2028 0 i f ));
! DATA(insert ( 1184 702 1180 0 a f ));
! DATA(insert ( 1184 1082 1178 0 a f ));
! DATA(insert ( 1184 1083 2019 0 a f ));
! DATA(insert ( 1184 1114 2027 0 a f ));
! DATA(insert ( 1184 1266 1388 0 a f ));
! DATA(insert ( 1186 703 1194 0 a f ));
! DATA(insert ( 1186 1083 1419 0 a f ));
! DATA(insert ( 1266 1083 2046 0 a f ));
/* Cross-category casts between int4 and abstime, reltime */
! DATA(insert ( 23 702 0 0 e b ));
! DATA(insert ( 702 23 0 0 e b ));
! DATA(insert ( 23 703 0 0 e b ));
! DATA(insert ( 703 23 0 0 e b ));
/*
* Geometric category
*/
! DATA(insert ( 601 600 1532 0 e f ));
! DATA(insert ( 602 600 1533 0 e f ));
! DATA(insert ( 602 604 1449 0 a f ));
! DATA(insert ( 603 600 1534 0 e f ));
! DATA(insert ( 603 601 1541 0 e f ));
! DATA(insert ( 603 604 1448 0 a f ));
! DATA(insert ( 603 718 1479 0 e f ));
! DATA(insert ( 604 600 1540 0 e f ));
! DATA(insert ( 604 602 1447 0 a f ));
! DATA(insert ( 604 603 1446 0 e f ));
! DATA(insert ( 604 718 1474 0 e f ));
! DATA(insert ( 718 600 1416 0 e f ));
! DATA(insert ( 718 603 1480 0 e f ));
! DATA(insert ( 718 604 1544 0 e f ));
/*
* INET category
*/
! DATA(insert ( 650 869 0 0 i b ));
! DATA(insert ( 869 650 1715 0 a f ));
/*
* BitString category
*/
! DATA(insert ( 1560 1562 0 0 i b ));
! DATA(insert ( 1562 1560 0 0 i b ));
/* Cross-category casts between bit and int4, int8 */
! DATA(insert ( 20 1560 2075 0 e f ));
! DATA(insert ( 23 1560 1683 0 e f ));
! DATA(insert ( 1560 20 2076 0 e f ));
! DATA(insert ( 1560 23 1684 0 e f ));
/*
* Cross-category casts to and from TEXT
***************
*** 315,360 **** DATA(insert ( 1560 23 1684 e f ));
* behavior will ensue when the automatic cast is applied instead of the
* pg_cast entry!
*/
! DATA(insert ( 650 25 730 a f ));
! DATA(insert ( 869 25 730 a f ));
! DATA(insert ( 16 25 2971 a f ));
! DATA(insert ( 142 25 0 a b ));
! DATA(insert ( 25 142 2896 e f ));
/*
* Cross-category casts to and from VARCHAR
*
* We support all the same casts as for TEXT.
*/
! DATA(insert ( 650 1043 730 a f ));
! DATA(insert ( 869 1043 730 a f ));
! DATA(insert ( 16 1043 2971 a f ));
! DATA(insert ( 142 1043 0 a b ));
! DATA(insert ( 1043 142 2896 e f ));
/*
* Cross-category casts to and from BPCHAR
*
* We support all the same casts as for TEXT.
*/
! DATA(insert ( 650 1042 730 a f ));
! DATA(insert ( 869 1042 730 a f ));
! DATA(insert ( 16 1042 2971 a f ));
! DATA(insert ( 142 1042 0 a b ));
! DATA(insert ( 1042 142 2896 e f ));
/*
* Length-coercion functions
*/
! DATA(insert ( 1042 1042 668 i f ));
! DATA(insert ( 1043 1043 669 i f ));
! DATA(insert ( 1083 1083 1968 i f ));
! DATA(insert ( 1114 1114 1961 i f ));
! DATA(insert ( 1184 1184 1967 i f ));
! DATA(insert ( 1186 1186 1200 i f ));
! DATA(insert ( 1266 1266 1969 i f ));
! DATA(insert ( 1560 1560 1685 i f ));
! DATA(insert ( 1562 1562 1687 i f ));
! DATA(insert ( 1700 1700 1703 i f ));
#endif /* PG_CAST_H */
--- 317,362 ----
* behavior will ensue when the automatic cast is applied instead of the
* pg_cast entry!
*/
! DATA(insert ( 650 25 730 0 a f ));
! DATA(insert ( 869 25 730 0 a f ));
! DATA(insert ( 16 25 2971 0 a f ));
! DATA(insert ( 142 25 0 0 a b ));
! DATA(insert ( 25 142 2896 3831 e f ));
/*
* Cross-category casts to and from VARCHAR
*
* We support all the same casts as for TEXT.
*/
! DATA(insert ( 650 1043 730 0 a f ));
! DATA(insert ( 869 1043 730 0 a f ));
! DATA(insert ( 16 1043 2971 0 a f ));
! DATA(insert ( 142 1043 0 0 a b ));
! DATA(insert ( 1043 142 2896 3831 e f ));
/*
* Cross-category casts to and from BPCHAR
*
* We support all the same casts as for TEXT.
*/
! DATA(insert ( 650 1042 730 0 a f ));
! DATA(insert ( 869 1042 730 0 a f ));
! DATA(insert ( 16 1042 2971 0 a f ));
! DATA(insert ( 142 1042 0 0 a b ));
! DATA(insert ( 1042 142 2896 3831 e f ));
/*
* Length-coercion functions
*/
! DATA(insert ( 1042 1042 668 0 i f ));
! DATA(insert ( 1043 1043 669 3829 i f ));
! DATA(insert ( 1083 1083 1968 0 i f ));
! DATA(insert ( 1114 1114 1961 0 i f ));
! DATA(insert ( 1184 1184 1967 0 i f ));
! DATA(insert ( 1186 1186 1200 0 i f ));
! DATA(insert ( 1266 1266 1969 0 i f ));
! DATA(insert ( 1560 1560 1685 0 i f ));
! DATA(insert ( 1562 1562 1687 0 i f ));
! DATA(insert ( 1700 1700 1703 0 i f ));
#endif /* PG_CAST_H */
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
***************
*** 937,942 **** DESCR("not equal");
--- 937,944 ----
DATA(insert OID = 668 ( bpchar PGNSP PGUID 12 1 0 0 f f f t f i 3 0 1042 "1042 23 16" _null_ _null_ _null_ _null_ bpchar _null_ _null_ _null_ ));
DESCR("adjust char() to typmod length");
+ DATA(insert OID = 3829 ( varchar_exemptor PGNSP PGUID 12 1 0 0 f f f t f i 3 0 23 "23 23 16" _null_ _null_ _null_ _null_ varchar_exemptor _null_ _null_ _null_ ));
+ DESCR("varchar cast exemptor");
DATA(insert OID = 669 ( varchar PGNSP PGUID 12 1 0 0 f f f t f i 3 0 1043 "1043 23 16" _null_ _null_ _null_ _null_ varchar _null_ _null_ _null_ ));
DESCR("adjust varchar() to typmod length");
***************
*** 4437,4442 **** DATA(insert OID = 2894 ( xml_out PGNSP PGUID 12 1 0 0 f f f t f i 1 0 2275
--- 4439,4446 ----
DESCR("I/O");
DATA(insert OID = 2895 ( xmlcomment PGNSP PGUID 12 1 0 0 f f f t f i 1 0 142 "25" _null_ _null_ _null_ _null_ xmlcomment _null_ _null_ _null_ ));
DESCR("generate an XML comment");
+ DATA(insert OID = 3831 ( xml_exemptor PGNSP PGUID 12 1 0 0 f f f t f i 3 0 23 "23 23 16" _null_ _null_ _null_ _null_ xml_exemptor _null_ _null_ _null_ ));
+ DESCR("texttoxml cast exemptor");
DATA(insert OID = 2896 ( xml PGNSP PGUID 12 1 0 0 f f f t f s 1 0 142 "25" _null_ _null_ _null_ _null_ texttoxml _null_ _null_ _null_ ));
DESCR("perform a non-validating parse of a character string to produce an XML value");
DATA(insert OID = 2897 ( xmlvalidate PGNSP PGUID 12 1 0 0 f f f t f i 2 0 16 "142 25" _null_ _null_ _null_ _null_ xmlvalidate _null_ _null_ _null_ ));
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
***************
*** 2439,2444 **** typedef struct CreateCastStmt
--- 2439,2445 ----
TypeName *sourcetype;
TypeName *targettype;
FuncWithArgs *func;
+ FuncWithArgs *exemptor;
CoercionContext context;
bool inout;
} CreateCastStmt;
*** a/src/include/nodes/primnodes.h
--- b/src/include/nodes/primnodes.h
***************
*** 313,318 **** typedef enum CoercionForm
--- 313,323 ----
COERCE_DONTCARE /* special case for planner */
} CoercionForm;
+ /* Bits in FuncExpr.funcexempt */
+ typedef int CoerceExemptions;
+ #define COERCE_EXEMPT_NOCHANGE 0x1 /* expression never changes storage */
+ #define COERCE_EXEMPT_NOERROR 0x2 /* expression never throws an error */
+
/*
* FuncExpr - expression node for a function call
*/
***************
*** 323,328 **** typedef struct FuncExpr
--- 328,334 ----
Oid funcresulttype; /* PG_TYPE OID of result value */
bool funcretset; /* true if function returns set */
CoercionForm funcformat; /* how to display this function call */
+ CoerceExemptions funcexempt; /* potential optimizations */
List *args; /* arguments to the function */
int location; /* token location, or -1 if unknown */
} FuncExpr;
*** a/src/include/parser/kwlist.h
--- b/src/include/parser/kwlist.h
***************
*** 148,153 **** PG_KEYWORD("exclude", EXCLUDE, UNRESERVED_KEYWORD)
--- 148,154 ----
PG_KEYWORD("excluding", EXCLUDING, UNRESERVED_KEYWORD)
PG_KEYWORD("exclusive", EXCLUSIVE, UNRESERVED_KEYWORD)
PG_KEYWORD("execute", EXECUTE, UNRESERVED_KEYWORD)
+ PG_KEYWORD("exemptor", EXEMPTOR, UNRESERVED_KEYWORD)
PG_KEYWORD("exists", EXISTS, COL_NAME_KEYWORD)
PG_KEYWORD("explain", EXPLAIN, UNRESERVED_KEYWORD)
PG_KEYWORD("external", EXTERNAL, UNRESERVED_KEYWORD)
*** a/src/include/parser/parse_coerce.h
--- b/src/include/parser/parse_coerce.h
***************
*** 30,40 **** typedef enum CoercionPathType
COERCION_PATH_COERCEVIAIO /* need a CoerceViaIO node */
} CoercionPathType;
- /* Bits in the return value of GetCoerceExemptions. */
- typedef int CoerceExemptions;
-
- #define COERCE_EXEMPT_NOCHANGE 0x1 /* expression never changes storage */
- #define COERCE_EXEMPT_NOERROR 0x2 /* expression never throws an error */
extern bool IsBinaryCoercible(Oid srctype, Oid targettype);
extern CoerceExemptions GetCoerceExemptions(Node *expr,
--- 30,35 ----
***************
*** 90,97 **** extern Oid resolve_generic_type(Oid declared_type,
extern CoercionPathType find_coercion_pathway(Oid targetTypeId,
Oid sourceTypeId,
CoercionContext ccontext,
! Oid *funcid);
extern CoercionPathType find_typmod_coercion_function(Oid typeId,
! Oid *funcid);
#endif /* PARSE_COERCE_H */
--- 85,92 ----
extern CoercionPathType find_coercion_pathway(Oid targetTypeId,
Oid sourceTypeId,
CoercionContext ccontext,
! Oid *funcid, Oid *exemptid);
extern CoercionPathType find_typmod_coercion_function(Oid typeId,
! Oid *funcid, Oid *exemptid);
#endif /* PARSE_COERCE_H */
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
***************
*** 676,681 **** extern Datum varcharrecv(PG_FUNCTION_ARGS);
--- 676,682 ----
extern Datum varcharsend(PG_FUNCTION_ARGS);
extern Datum varchartypmodin(PG_FUNCTION_ARGS);
extern Datum varchartypmodout(PG_FUNCTION_ARGS);
+ extern Datum varchar_exemptor(PG_FUNCTION_ARGS);
extern Datum varchar(PG_FUNCTION_ARGS);
/* varlena.c */
*** a/src/include/utils/xml.h
--- b/src/include/utils/xml.h
***************
*** 33,38 **** extern Datum xml_recv(PG_FUNCTION_ARGS);
--- 33,39 ----
extern Datum xml_send(PG_FUNCTION_ARGS);
extern Datum xmlcomment(PG_FUNCTION_ARGS);
extern Datum xmlconcat2(PG_FUNCTION_ARGS);
+ extern Datum xml_exemptor(PG_FUNCTION_ARGS);
extern Datum texttoxml(PG_FUNCTION_ARGS);
extern Datum xmltotext(PG_FUNCTION_ARGS);
extern Datum xmlvalidate(PG_FUNCTION_ARGS);
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
***************
*** 2046,2084 **** DEBUG: Rewriting table "t"
ERROR: numeric field overflow
DETAIL: A field with precision 4, scale 3 must round to an absolute value less than 10^1.
ALTER TABLE t ALTER string TYPE varchar(4); -- noop
- DEBUG: Rewriting table "t"
- DEBUG: Rebuilding index "t_daytimetz_key"
- DEBUG: Rebuilding index "t_daytime_key"
- DEBUG: Rebuilding index "t_stamptz_key"
- DEBUG: Rebuilding index "t_stamp_key"
- DEBUG: Rebuilding index "t_timegap_key"
- DEBUG: Rebuilding index "t_bits_key"
- DEBUG: Rebuilding index "t_network_key"
- DEBUG: Rebuilding index "t_strarr_idx"
- DEBUG: Rebuilding index "t_square_idx"
- DEBUG: Rebuilding index "t_touchy_f_idx"
- DEBUG: Rebuilding index "t_expr_idx"
- DEBUG: Rebuilding index "t_constraint4_key"
- DEBUG: Rebuilding index "t_integral_key"
- DEBUG: Rebuilding index "t_rational_key"
DEBUG: Rebuilding index "t_string_idx1"
DEBUG: Rebuilding index "t_string_idx"
ALTER TABLE t ALTER string TYPE lendom; -- noop
- DEBUG: Rewriting table "t"
- DEBUG: Rebuilding index "t_daytimetz_key"
- DEBUG: Rebuilding index "t_daytime_key"
- DEBUG: Rebuilding index "t_stamptz_key"
- DEBUG: Rebuilding index "t_stamp_key"
- DEBUG: Rebuilding index "t_timegap_key"
- DEBUG: Rebuilding index "t_bits_key"
- DEBUG: Rebuilding index "t_network_key"
- DEBUG: Rebuilding index "t_strarr_idx"
- DEBUG: Rebuilding index "t_square_idx"
- DEBUG: Rebuilding index "t_touchy_f_idx"
- DEBUG: Rebuilding index "t_expr_idx"
- DEBUG: Rebuilding index "t_constraint4_key"
- DEBUG: Rebuilding index "t_integral_key"
- DEBUG: Rebuilding index "t_rational_key"
DEBUG: Rebuilding index "t_string_idx"
DEBUG: Rebuilding index "t_string_idx1"
ALTER TABLE t ALTER string TYPE shortdom; -- rewrite-e
--- 2046,2054 ----
***************
*** 2732,2754 **** DEBUG: Rebuilding index "t_bits_key"
DEBUG: Rebuilding index "t_network_key"
ALTER TABLE t ALTER document TYPE text; -- noop
ALTER TABLE t ALTER document TYPE xml USING document::xml; -- verify
! DEBUG: Rewriting table "t"
! DEBUG: Rebuilding index "t_strarr_idx"
! DEBUG: Rebuilding index "t_square_idx"
! DEBUG: Rebuilding index "t_touchy_f_idx"
! DEBUG: Rebuilding index "t_expr_idx"
! DEBUG: Rebuilding index "t_constraint4_key"
! DEBUG: Rebuilding index "t_integral_key"
! DEBUG: Rebuilding index "t_rational_key"
! DEBUG: Rebuilding index "t_string_idx1"
! DEBUG: Rebuilding index "t_string_idx"
! DEBUG: Rebuilding index "t_daytimetz_key"
! DEBUG: Rebuilding index "t_daytime_key"
! DEBUG: Rebuilding index "t_stamptz_key"
! DEBUG: Rebuilding index "t_stamp_key"
! DEBUG: Rebuilding index "t_timegap_key"
! DEBUG: Rebuilding index "t_bits_key"
! DEBUG: Rebuilding index "t_network_key"
ALTER TABLE t ALTER document TYPE char(20); -- rewrite
DEBUG: Rewriting table "t"
DEBUG: Rebuilding index "t_strarr_idx"
--- 2702,2708 ----
DEBUG: Rebuilding index "t_network_key"
ALTER TABLE t ALTER document TYPE text; -- noop
ALTER TABLE t ALTER document TYPE xml USING document::xml; -- verify
! DEBUG: Verifying table "t"
ALTER TABLE t ALTER document TYPE char(20); -- rewrite
DEBUG: Rewriting table "t"
DEBUG: Rebuilding index "t_strarr_idx"
***************
*** 2768,2790 **** DEBUG: Rebuilding index "t_timegap_key"
DEBUG: Rebuilding index "t_bits_key"
DEBUG: Rebuilding index "t_network_key"
ALTER TABLE t ALTER document TYPE xml USING document::xml; -- verify
! DEBUG: Rewriting table "t"
! DEBUG: Rebuilding index "t_strarr_idx"
! DEBUG: Rebuilding index "t_square_idx"
! DEBUG: Rebuilding index "t_touchy_f_idx"
! DEBUG: Rebuilding index "t_expr_idx"
! DEBUG: Rebuilding index "t_constraint4_key"
! DEBUG: Rebuilding index "t_integral_key"
! DEBUG: Rebuilding index "t_rational_key"
! DEBUG: Rebuilding index "t_string_idx1"
! DEBUG: Rebuilding index "t_string_idx"
! DEBUG: Rebuilding index "t_daytimetz_key"
! DEBUG: Rebuilding index "t_daytime_key"
! DEBUG: Rebuilding index "t_stamptz_key"
! DEBUG: Rebuilding index "t_stamp_key"
! DEBUG: Rebuilding index "t_timegap_key"
! DEBUG: Rebuilding index "t_bits_key"
! DEBUG: Rebuilding index "t_network_key"
ALTER TABLE t ALTER document TYPE varchar(30); -- rewrite-v
DEBUG: Rewriting table "t"
DEBUG: Rebuilding index "t_strarr_idx"
--- 2722,2728 ----
DEBUG: Rebuilding index "t_bits_key"
DEBUG: Rebuilding index "t_network_key"
ALTER TABLE t ALTER document TYPE xml USING document::xml; -- verify
! DEBUG: Verifying table "t"
ALTER TABLE t ALTER document TYPE varchar(30); -- rewrite-v
DEBUG: Rewriting table "t"
DEBUG: Rebuilding index "t_strarr_idx"
***************
*** 2804,2843 **** DEBUG: Rebuilding index "t_timegap_key"
DEBUG: Rebuilding index "t_bits_key"
DEBUG: Rebuilding index "t_network_key"
ALTER TABLE t ALTER document TYPE xml USING document::xml; -- verify
! DEBUG: Rewriting table "t"
! DEBUG: Rebuilding index "t_strarr_idx"
! DEBUG: Rebuilding index "t_square_idx"
! DEBUG: Rebuilding index "t_touchy_f_idx"
! DEBUG: Rebuilding index "t_expr_idx"
! DEBUG: Rebuilding index "t_constraint4_key"
! DEBUG: Rebuilding index "t_integral_key"
! DEBUG: Rebuilding index "t_rational_key"
! DEBUG: Rebuilding index "t_string_idx1"
! DEBUG: Rebuilding index "t_string_idx"
! DEBUG: Rebuilding index "t_daytimetz_key"
! DEBUG: Rebuilding index "t_daytime_key"
! DEBUG: Rebuilding index "t_stamptz_key"
! DEBUG: Rebuilding index "t_stamp_key"
! DEBUG: Rebuilding index "t_timegap_key"
! DEBUG: Rebuilding index "t_bits_key"
! DEBUG: Rebuilding index "t_network_key"
ALTER TABLE t ALTER strarr TYPE varchar(4)[]; -- noop
- DEBUG: Rewriting table "t"
- DEBUG: Rebuilding index "t_square_idx"
- DEBUG: Rebuilding index "t_touchy_f_idx"
- DEBUG: Rebuilding index "t_expr_idx"
- DEBUG: Rebuilding index "t_constraint4_key"
- DEBUG: Rebuilding index "t_integral_key"
- DEBUG: Rebuilding index "t_rational_key"
- DEBUG: Rebuilding index "t_string_idx1"
- DEBUG: Rebuilding index "t_string_idx"
- DEBUG: Rebuilding index "t_daytimetz_key"
- DEBUG: Rebuilding index "t_daytime_key"
- DEBUG: Rebuilding index "t_stamptz_key"
- DEBUG: Rebuilding index "t_stamp_key"
- DEBUG: Rebuilding index "t_timegap_key"
- DEBUG: Rebuilding index "t_bits_key"
- DEBUG: Rebuilding index "t_network_key"
DEBUG: Rebuilding index "t_strarr_idx"
ALTER TABLE t ALTER strarr TYPE varchar(3)[]; -- rewrite-v
DEBUG: Rewriting table "t"
--- 2742,2749 ----
DEBUG: Rebuilding index "t_bits_key"
DEBUG: Rebuilding index "t_network_key"
ALTER TABLE t ALTER document TYPE xml USING document::xml; -- verify
! DEBUG: Verifying table "t"
ALTER TABLE t ALTER strarr TYPE varchar(4)[]; -- noop
DEBUG: Rebuilding index "t_strarr_idx"
ALTER TABLE t ALTER strarr TYPE varchar(3)[]; -- rewrite-v
DEBUG: Rewriting table "t"
*** a/src/test/regress/expected/create_cast.out
--- b/src/test/regress/expected/create_cast.out
***************
*** 17,22 **** CREATE TYPE casttesttype (
--- 17,24 ----
internallength = variable,
input = casttesttype_in,
output = casttesttype_out,
+ typmod_in = varchartypmodin,
+ typmod_out = varchartypmodout,
alignment = int4
);
-- a dummy function to test with
***************
*** 49,54 **** SELECT casttestfunc('foo'::text); -- Should work now
--- 51,57 ----
1
(1 row)
+ DROP CAST (text AS casttesttype); -- cleanup
-- Try I/O conversion cast.
SELECT 1234::int4::casttesttype; -- No cast yet, should fail
ERROR: cannot cast type integer to casttesttype
***************
*** 72,74 **** SELECT 1234::int4::casttesttype; -- Should work now
--- 75,110 ----
foo1234
(1 row)
+ -- Try an EXEMPTOR to skip work during ALTER TABLE. Make casttesttype behave
+ -- like varchar, but without the special trailing blank treatment during an
+ -- implicit cast.
+ SET client_min_messages = debug1;
+ CREATE CAST (text AS casttesttype) WITHOUT FUNCTION;
+ CREATE FUNCTION casttest_lencoerce(casttesttype, int, boolean)
+ RETURNS casttesttype LANGUAGE SQL IMMUTABLE STRICT AS $$
+ SELECT CASE
+ WHEN length($1::text) <= $2 THEN $1
+ WHEN $3 THEN substring($1::text from 1 for $2)::casttesttype
+ ELSE ('f' || $1)::numeric::text::casttesttype -- throws an error
+ END
+ $$;
+ CREATE FUNCTION casttest_exemptor(oldtypmod int, newtypmod int, explicit bool)
+ RETURNS int LANGUAGE SQL IMMUTABLE STRICT AS $$
+ SELECT CASE
+ WHEN $1 <= $2 THEN 3 -- no change, no error
+ WHEN $3 THEN 0 -- explicit shrink: can truncate
+ ELSE 1 -- implciit shrink: no change, might error
+ END
+ $$;
+ CREATE CAST (casttesttype AS casttesttype)
+ WITH FUNCTION casttest_lencoerce(casttesttype, int, boolean)
+ EXEMPTOR casttest_exemptor(int, int, boolean);
+ CREATE TABLE casttbl (c casttesttype(10));
+ INSERT INTO casttbl VALUES ('foo'), ('bar');
+ ALTER TABLE casttbl ALTER c TYPE casttesttype(11); -- noop
+ ALTER TABLE casttbl ALTER c TYPE casttesttype(10); -- verify
+ DEBUG: Verifying table "casttbl"
+ ALTER TABLE casttbl ALTER c TYPE casttesttype(11) USING c::casttesttype(11); -- noop
+ ALTER TABLE casttbl ALTER c TYPE casttesttype(10) USING c::casttesttype(10); -- rewrite
+ DEBUG: Rewriting table "casttbl"
+ RESET client_min_messages;
*** a/src/test/regress/expected/opr_sanity.out
--- b/src/test/regress/expected/opr_sanity.out
***************
*** 331,338 **** SELECT *
FROM pg_cast c
WHERE castsource = 0 OR casttarget = 0 OR castcontext NOT IN ('e', 'a', 'i')
OR castmethod NOT IN ('f', 'b' ,'i');
! castsource | casttarget | castfunc | castcontext | castmethod
! ------------+------------+----------+-------------+------------
(0 rows)
-- Check that castfunc is nonzero only for cast methods that need a function,
--- 331,338 ----
FROM pg_cast c
WHERE castsource = 0 OR casttarget = 0 OR castcontext NOT IN ('e', 'a', 'i')
OR castmethod NOT IN ('f', 'b' ,'i');
! castsource | casttarget | castfunc | castexemptor | castcontext | castmethod
! ------------+------------+----------+--------------+-------------+------------
(0 rows)
-- Check that castfunc is nonzero only for cast methods that need a function,
***************
*** 341,348 **** SELECT *
FROM pg_cast c
WHERE (castmethod = 'f' AND castfunc = 0)
OR (castmethod IN ('b', 'i') AND castfunc <> 0);
! castsource | casttarget | castfunc | castcontext | castmethod
! ------------+------------+----------+-------------+------------
(0 rows)
-- Look for casts to/from the same type that aren't length coercion functions.
--- 341,348 ----
FROM pg_cast c
WHERE (castmethod = 'f' AND castfunc = 0)
OR (castmethod IN ('b', 'i') AND castfunc <> 0);
! castsource | casttarget | castfunc | castexemptor | castcontext | castmethod
! ------------+------------+----------+--------------+-------------+------------
(0 rows)
-- Look for casts to/from the same type that aren't length coercion functions.
***************
*** 351,365 **** WHERE (castmethod = 'f' AND castfunc = 0)
SELECT *
FROM pg_cast c
WHERE castsource = casttarget AND castfunc = 0;
! castsource | casttarget | castfunc | castcontext | castmethod
! ------------+------------+----------+-------------+------------
(0 rows)
SELECT c.*
FROM pg_cast c, pg_proc p
WHERE c.castfunc = p.oid AND p.pronargs < 2 AND castsource = casttarget;
! castsource | casttarget | castfunc | castcontext | castmethod
! ------------+------------+----------+-------------+------------
(0 rows)
-- Look for cast functions that don't have the right signature. The
--- 351,365 ----
SELECT *
FROM pg_cast c
WHERE castsource = casttarget AND castfunc = 0;
! castsource | casttarget | castfunc | castexemptor | castcontext | castmethod
! ------------+------------+----------+--------------+-------------+------------
(0 rows)
SELECT c.*
FROM pg_cast c, pg_proc p
WHERE c.castfunc = p.oid AND p.pronargs < 2 AND castsource = casttarget;
! castsource | casttarget | castfunc | castexemptor | castcontext | castmethod
! ------------+------------+----------+--------------+-------------+------------
(0 rows)
-- Look for cast functions that don't have the right signature. The
***************
*** 377,384 **** WHERE c.castfunc = p.oid AND
OR (c.castsource = 'character'::regtype AND
p.proargtypes[0] = 'text'::regtype))
OR NOT binary_coercible(p.prorettype, c.casttarget));
! castsource | casttarget | castfunc | castcontext | castmethod
! ------------+------------+----------+-------------+------------
(0 rows)
SELECT c.*
--- 377,384 ----
OR (c.castsource = 'character'::regtype AND
p.proargtypes[0] = 'text'::regtype))
OR NOT binary_coercible(p.prorettype, c.casttarget));
! castsource | casttarget | castfunc | castexemptor | castcontext | castmethod
! ------------+------------+----------+--------------+-------------+------------
(0 rows)
SELECT c.*
***************
*** 386,393 **** FROM pg_cast c, pg_proc p
WHERE c.castfunc = p.oid AND
((p.pronargs > 1 AND p.proargtypes[1] != 'int4'::regtype) OR
(p.pronargs > 2 AND p.proargtypes[2] != 'bool'::regtype));
! castsource | casttarget | castfunc | castcontext | castmethod
! ------------+------------+----------+-------------+------------
(0 rows)
-- Look for binary compatible casts that do not have the reverse
--- 386,393 ----
WHERE c.castfunc = p.oid AND
((p.pronargs > 1 AND p.proargtypes[1] != 'int4'::regtype) OR
(p.pronargs > 2 AND p.proargtypes[2] != 'bool'::regtype));
! castsource | casttarget | castfunc | castexemptor | castcontext | castmethod
! ------------+------------+----------+--------------+-------------+------------
(0 rows)
-- Look for binary compatible casts that do not have the reverse
*** a/src/test/regress/expected/sanity_check.out
--- b/src/test/regress/expected/sanity_check.out
***************
*** 27,32 **** SELECT relname, relhasindex
--- 27,33 ----
bt_txt_heap | t
c | f
c_star | f
+ casttbl | f
char_tbl | f
check2_tbl | f
check_tbl | f
***************
*** 155,161 **** SELECT relname, relhasindex
timetz_tbl | f
tinterval_tbl | f
varchar_tbl | f
! (144 rows)
--
-- another sanity check: every system catalog that has OIDs should have
--- 156,162 ----
timetz_tbl | f
tinterval_tbl | f
varchar_tbl | f
! (145 rows)
--
-- another sanity check: every system catalog that has OIDs should have
*** a/src/test/regress/output/misc.source
--- b/src/test/regress/output/misc.source
***************
*** 585,590 **** SELECT user_relns() AS user_relns
--- 585,591 ----
bt_txt_heap
c
c_star
+ casttbl
char_tbl
check2_tbl
check_seq
***************
*** 669,675 **** SELECT user_relns() AS user_relns
toyemp
varchar_tbl
xacttest
! (102 rows)
SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer')));
name
--- 670,676 ----
toyemp
varchar_tbl
xacttest
! (103 rows)
SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer')));
name
*** a/src/test/regress/sql/create_cast.sql
--- b/src/test/regress/sql/create_cast.sql
***************
*** 18,23 **** CREATE TYPE casttesttype (
--- 18,25 ----
internallength = variable,
input = casttesttype_in,
output = casttesttype_out,
+ typmod_in = varchartypmodin,
+ typmod_out = varchartypmodout,
alignment = int4
);
***************
*** 36,41 **** DROP CAST (text AS casttesttype); -- cleanup
--- 38,44 ----
-- Try IMPLICIT binary coercion cast
CREATE CAST (text AS casttesttype) WITHOUT FUNCTION AS IMPLICIT;
SELECT casttestfunc('foo'::text); -- Should work now
+ DROP CAST (text AS casttesttype); -- cleanup
-- Try I/O conversion cast.
SELECT 1234::int4::casttesttype; -- No cast yet, should fail
***************
*** 52,54 **** $$ SELECT ('foo'::text || $1::text)::casttesttype; $$;
--- 55,94 ----
CREATE CAST (int4 AS casttesttype) WITH FUNCTION int4_casttesttype(int4) AS IMPLICIT;
SELECT 1234::int4::casttesttype; -- Should work now
+
+ -- Try an EXEMPTOR to skip work during ALTER TABLE. Make casttesttype behave
+ -- like varchar, but without the special trailing blank treatment during an
+ -- implicit cast.
+ SET client_min_messages = debug1;
+ CREATE CAST (text AS casttesttype) WITHOUT FUNCTION;
+
+ CREATE FUNCTION casttest_lencoerce(casttesttype, int, boolean)
+ RETURNS casttesttype LANGUAGE SQL IMMUTABLE STRICT AS $$
+ SELECT CASE
+ WHEN length($1::text) <= $2 THEN $1
+ WHEN $3 THEN substring($1::text from 1 for $2)::casttesttype
+ ELSE ('f' || $1)::numeric::text::casttesttype -- throws an error
+ END
+ $$;
+
+ CREATE FUNCTION casttest_exemptor(oldtypmod int, newtypmod int, explicit bool)
+ RETURNS int LANGUAGE SQL IMMUTABLE STRICT AS $$
+ SELECT CASE
+ WHEN $1 <= $2 THEN 3 -- no change, no error
+ WHEN $3 THEN 0 -- explicit shrink: can truncate
+ ELSE 1 -- implciit shrink: no change, might error
+ END
+ $$;
+
+ CREATE CAST (casttesttype AS casttesttype)
+ WITH FUNCTION casttest_lencoerce(casttesttype, int, boolean)
+ EXEMPTOR casttest_exemptor(int, int, boolean);
+
+ CREATE TABLE casttbl (c casttesttype(10));
+ INSERT INTO casttbl VALUES ('foo'), ('bar');
+ ALTER TABLE casttbl ALTER c TYPE casttesttype(11); -- noop
+ ALTER TABLE casttbl ALTER c TYPE casttesttype(10); -- verify
+ ALTER TABLE casttbl ALTER c TYPE casttesttype(11) USING c::casttesttype(11); -- noop
+ ALTER TABLE casttbl ALTER c TYPE casttesttype(10) USING c::casttesttype(10); -- rewrite
+
+ RESET client_min_messages;
On Sun, Jan 9, 2011 at 5:03 PM, Noah Misch <noah@leadboat.com> wrote:
Here I add the notion of an "exemptor function", a property of a cast that
determines when calls to the cast would be superfluous. Such calls can be
removed, reduced to RelabelType expressions, or annotated (via a new field in
FuncExpr) with the applicable exemptions. I modify various parse_coerce.c
functions to retrieve, call, and act on these exemptor functions; this includes
GetCoerceExemptions() from the last patch. I did opt to make
find_typmod_coercion_function return COERCION_PATH_RELABELTYPE when no work is
needed, rather than COERCION_PATH_NONE; this makes it consistent with
find_coercion_pathway's use of that enumeration.To demonstrate the functionality, I add exemptor functions for varchar and xml.
Originally I was only going to start with varchar, but xml tests the other major
code path, and the exemptor function for xml is dead simple.This helps on conversions like varchar(4)->varchar(8) and text->xml.
I've read through this patch somewhat. As I believe Tom also
commented previously, exemptor is a pretty bad choice of name. More
generally, I think this patch is a case of coding something
complicated without first achieving consensus on what the behavior
should be. I think we need to address that problem first, and then
decide what code needs to be written, and then write the code.
It seems to me that patch two in this series has the right idea: skip
rewriting the table in cases where the types are binary coercible
(WITHOUT FUNCTION) or one is an unconstrained domain over the other.
IIUC, the problem this patch is trying to address is that our current
CAST infrastructure is insufficient to identify all cases where this
is true - in particular, it makes the decision solely based on the
type OIDs, without consideration of the typmods. In general, varchar
-> varchar is not binary coercible, but varchar(4) -> varchar(8) is
binary coercible. I think what this patch is trying to do is tag the
call to the cast function with the information that we might not need
to call it, but ISTM it would be better to just notice that the
function call is unnecessary and insert a RelabelType node instead.
*reads archives*
In fact, Tom already made pretty much exactly this suggestion, on a
thread entitled "Avoiding rewrite in ALTER TABLE ALTER TYPE". The
only possible objection I can see to that line of attack is that it's
not clear what the API should be, or whether there might be too much
runtime overhead for the amount of benefit that can be obtained.
This is, incidentally, another problem with this patch - if you want
to make wide-ranging changes to the system like this, you need to
provide a convincing argument that it's not going to compromise
correctness or performance. Just submitting the patch without making
an argument for why it's correct is no good, unless it's so obviously
correct as to be unquestionable, which is certainly not the case here.
You've got to write up some kind of submission notes so that the
reviewer isn't trying to guess why you think this is the right
approach, or spending a lot of time thinking through approaches you've
discarded for good reason. If there is a danger of performance
regression, you need to refute it, preferably with actual benchmarks.
A particular aspect I'm concerned about with this patch in particular:
it strikes me that the overhead of calling the exemptor functions in
the ALTER TABLE case is negligible, but that this might not be true in
other contexts where some of this logic may get invoked - it appears
to implicate the casting machinery much more generally.
I think it's a mistake to merge the handling of the rewrite-vs-noop
case with the check-vs-noop case. They're fundamentally dissimilar.
As noted above, being able to notice the noop case seems like it could
save run-time work during ordinary query execution. But detecting the
"check" case is useless work except in the specific case of a table
rewrite. If we're going to do that at all, it seems to me that it
needs to be done via a code-path that's only going to get invoked in
the case of ALTER TABLE, not something that's going to happen every
time we parse an expression tree. But it seems to me that it might be
better to take the suggestion I made before: forget about the
check-only case for now, and focus on the do-nothing case.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Mon, Jan 24, 2011 at 07:18:47PM -0500, Robert Haas wrote:
On Sun, Jan 9, 2011 at 5:03 PM, Noah Misch <noah@leadboat.com> wrote:
Here I add the notion of an "exemptor function", a property of a cast that
determines when calls to the cast would be superfluous. ?Such calls can be
removed, reduced to RelabelType expressions, or annotated (via a new field in
FuncExpr) with the applicable exemptions. ?I modify various parse_coerce.c
functions to retrieve, call, and act on these exemptor functions; this includes
GetCoerceExemptions() from the last patch. ?I did opt to make
find_typmod_coercion_function return COERCION_PATH_RELABELTYPE when no work is
needed, rather than COERCION_PATH_NONE; this makes it consistent with
find_coercion_pathway's use of that enumeration.To demonstrate the functionality, I add exemptor functions for varchar and xml.
Originally I was only going to start with varchar, but xml tests the other major
code path, and the exemptor function for xml is dead simple.This helps on conversions like varchar(4)->varchar(8) and text->xml.
I've read through this patch somewhat. As I believe Tom also
commented previously, exemptor is a pretty bad choice of name.
I spent awhile with a thesaurus before coining that. Since Tom's comment, I've
been hoping the next person to complain would at least suggest a better name.
Shooting in the dark on a nomenclature issue isn't fun, and I'm not picky.
More
generally, I think this patch is a case of coding something
complicated without first achieving consensus on what the behavior
should be. I think we need to address that problem first, and then
decide what code needs to be written, and then write the code.
Well, that's why I started the design thread "Avoiding rewrite in ALTER TABLE
ALTER TYPE" before writing any code. That thread petered out rather than reach
any consensus. What would you have done then?
It seems to me that patch two in this series has the right idea: skip
rewriting the table in cases where the types are binary coercible
(WITHOUT FUNCTION) or one is an unconstrained domain over the other.
IIUC, the problem this patch is trying to address is that our current
CAST infrastructure is insufficient to identify all cases where this
is true - in particular, it makes the decision solely based on the
type OIDs, without consideration of the typmods. In general, varchar
-> varchar is not binary coercible, but varchar(4) -> varchar(8) is
binary coercible. I think what this patch is trying to do is tag the
call to the cast function with the information that we might not need
to call it, but ISTM it would be better to just notice that the
function call is unnecessary and insert a RelabelType node instead.
This patch does exactly that for varchar(4) -> varchar(8) and similar cases.
*reads archives*
In fact, Tom already made pretty much exactly this suggestion, on a
thread entitled "Avoiding rewrite in ALTER TABLE ALTER TYPE". The
only possible objection I can see to that line of attack is that it's
not clear what the API should be, or whether there might be too much
runtime overhead for the amount of benefit that can be obtained.
I believe the patch does implement Tom's suggestion, at least in spirit. He
mentioned expression_planner() as the place to do it. In principle, We could do
it *right there* and avoid any changes to coerce_to_target_type(), but the
overhead would increase. Specifically, we would be walking an already-built
expression tree looking for casts to remove, probably doing a syscache lookup
for every cast to grab the exemptor function OID. Doing it there would prevent
us from directly helping or harming plain old queries. Since ExecRelCheck(),
FormIndexDatum() and other notable paths call expression_planner(), we wouldn't
want to casually add an expensive algorithm there. To avoid the extra syscache
lookups, we'd piggyback off those already done in coerce_to_target_type(). That
brings us to the current implementation. Better to make the entire decision in
coerce_to_target_type() and never add the superfluous node to the expression
tree in the first place.
coerce_to_target_type() and callees seemed like the natural, low cost place to
make the changes. By doing it that way, did I miss some important detail or
implication of Tom's suggestion?
This is, incidentally, another problem with this patch - if you want
to make wide-ranging changes to the system like this, you need to
provide a convincing argument that it's not going to compromise
correctness or performance. Just submitting the patch without making
an argument for why it's correct is no good, unless it's so obviously
correct as to be unquestionable, which is certainly not the case here.
You've got to write up some kind of submission notes so that the
reviewer isn't trying to guess why you think this is the right
approach, or spending a lot of time thinking through approaches you've
discarded for good reason. If there is a danger of performance
regression, you need to refute it, preferably with actual benchmarks.
A particular aspect I'm concerned about with this patch in particular:
it strikes me that the overhead of calling the exemptor functions in
the ALTER TABLE case is negligible, but that this might not be true in
other contexts where some of this logic may get invoked - it appears
to implicate the casting machinery much more generally.
I assumed that readers of this patch email had read the series introduction
email, which referred them to the design discussion that you found. Surely that
design answered more than zero of "why this is correct". What questions in
particular did you find unanswered?
As for performance, coerce_to_target_type() is certainly in wide use, but it's
not exactly a hot path. I prepared and ran the attached bench-coerce-worst.sql
and bench-coerce-best.sql. Both are quite artificial. The first one basically
asks "Can we measure the new overhead at all?" The second one asks "Would any
ordinary query benefit from removing a superfluous cast from an expression
tree?" The worst case had an 4% _speedup_, and the best case had a 27% speedup,
suggesting answers of "no" and "yes (but not dramatically)". The "worst-case"
speedup doesn't make sense, so it's probably an artifact of some recent change
on master creating a larger performance dip in this pathological test case. I
could rebase the patch and rerun the benchmark, but I doubt an exact figure
would be much more meaningful. Unfortunately, I can't think of any more-natural
tests (help welcome) that would still illustrate any performance difference.
I think it's a mistake to merge the handling of the rewrite-vs-noop
case with the check-vs-noop case. They're fundamentally dissimilar.
As noted above, being able to notice the noop case seems like it could
save run-time work during ordinary query execution. But detecting the
"check" case is useless work except in the specific case of a table
rewrite. If we're going to do that at all, it seems to me that it
needs to be done via a code-path that's only going to get invoked in
the case of ALTER TABLE, not something that's going to happen every
time we parse an expression tree.
I disagree that they're fundamentally dissimilar. They're _fundamentally_
similar, in that they are both fences, standing at different distances, around
the necessity of a cast function. Your argument only mentions performance, and
it boils down to a suggestion that we proceed to optimize things now by
separating these questions. Perhaps we should, but that's a different question.
Based on my benchmarks, I'm not seeing a need to micro-optimize.
But it seems to me that it might be
better to take the suggestion I made before: forget about the
check-only case for now, and focus on the do-nothing case.
Let's revisit this when we're on the same page about the above points.
Thanks,
nm
On Mon, Jan 24, 2011 at 11:13 PM, Noah Misch <noah@leadboat.com> wrote:
This helps on conversions like varchar(4)->varchar(8) and text->xml.
I've read through this patch somewhat. As I believe Tom also
commented previously, exemptor is a pretty bad choice of name.I spent awhile with a thesaurus before coining that. Since Tom's comment, I've
been hoping the next person to complain would at least suggest a better name.
Shooting in the dark on a nomenclature issue isn't fun, and I'm not picky.
I can't speak for Tom, but I guess my gripe about that name is that we
seem to view a cast as either WITH FUNCTION or WITHOUT FUNCTION. A
cast-without-function is trivial from an implementation point of view,
but it's still a cast, whereas the word "exempt" seems to imply that
you're skipping the cast altogether. A side issue is that I really
want to avoid adding a new parser keyword for this. It doesn't bother
me too much to add keywords for really important and user-visible
features, but when we're adding stuff that's only going to be used by
0.1% of our users it's really better if we can avoid it, because it
slows down the parser. Maybe we could do something like:
CREATE CAST (source_type AS target_type)
WITH FUNCTION function_name (argument_type, [, ...])
[ ANALYZE USING function_name ]
[ AS ASSIGNMENT | AS IMPLICIT ]
Presumably, we wouldn't need the ANALYZE USING clause for the WITHOUT
FUNCTION case, since the answer is a foregone conclusion in that case.
We could possibly support it also for WITH INOUT, but I'm not sure
whether the real-world utility is more than zero.
Internally, we could refer to a function of this type as a "cast analyzer".
Don't take the above as Gospel truth, it's just an idea from one
person on one day.
More
generally, I think this patch is a case of coding something
complicated without first achieving consensus on what the behavior
should be. I think we need to address that problem first, and then
decide what code needs to be written, and then write the code.Well, that's why I started the design thread "Avoiding rewrite in ALTER TABLE
ALTER TYPE" before writing any code. That thread petered out rather than reach
any consensus. What would you have done then?
Let's defer this question until we're more on the same page about the
rest of this. It's obvious I'm not completely understanding this
patch...
I think what this patch is trying to do is tag the
call to the cast function with the information that we might not need
to call it, but ISTM it would be better to just notice that the
function call is unnecessary and insert a RelabelType node instead.This patch does exactly that for varchar(4) -> varchar(8) and similar cases.
Oh, uh, err... OK, I obviously haven't understood it well enough.
I'll look at it some more, but if there's anything you can write up to
explain what you changed and why, that would be helpful. I think I'm
also having trouble with the fact that these patches don't apply
cleanly against the master branch, because they're stacked up on each
other. Maybe this will be easier once we get more of the ALTER TYPE 2
stuff in.
*reads archives*
In fact, Tom already made pretty much exactly this suggestion, on a
thread entitled "Avoiding rewrite in ALTER TABLE ALTER TYPE". The
only possible objection I can see to that line of attack is that it's
not clear what the API should be, or whether there might be too much
runtime overhead for the amount of benefit that can be obtained.I believe the patch does implement Tom's suggestion, at least in spirit. He
mentioned expression_planner() as the place to do it. In principle, We could do
it *right there* and avoid any changes to coerce_to_target_type(), but the
overhead would increase. Specifically, we would be walking an already-built
expression tree looking for casts to remove, probably doing a syscache lookup
for every cast to grab the exemptor function OID. Doing it there would prevent
us from directly helping or harming plain old queries. Since ExecRelCheck(),
FormIndexDatum() and other notable paths call expression_planner(), we wouldn't
want to casually add an expensive algorithm there. To avoid the extra syscache
lookups, we'd piggyback off those already done in coerce_to_target_type(). That
brings us to the current implementation. Better to make the entire decision in
coerce_to_target_type() and never add the superfluous node to the expression
tree in the first place.coerce_to_target_type() and callees seemed like the natural, low cost place to
make the changes. By doing it that way, did I miss some important detail or
implication of Tom's suggestion?
I'm not sure. Tom?
As for performance, coerce_to_target_type() is certainly in wide use, but it's
not exactly a hot path. I prepared and ran the attached bench-coerce-worst.sql
and bench-coerce-best.sql. Both are quite artificial. The first one basically
asks "Can we measure the new overhead at all?" The second one asks "Would any
ordinary query benefit from removing a superfluous cast from an expression
tree?" The worst case had an 4% _speedup_, and the best case had a 27% speedup,
suggesting answers of "no" and "yes (but not dramatically)". The "worst-case"
speedup doesn't make sense, so it's probably an artifact of some recent change
on master creating a larger performance dip in this pathological test case. I
could rebase the patch and rerun the benchmark, but I doubt an exact figure
would be much more meaningful. Unfortunately, I can't think of any more-natural
tests (help welcome) that would still illustrate any performance difference.
That certainly seems promising, but I don't understand how the new
code can be faster on your constructed worst case. Thoughts?
I think it's a mistake to merge the handling of the rewrite-vs-noop
case with the check-vs-noop case. They're fundamentally dissimilar.
As noted above, being able to notice the noop case seems like it could
save run-time work during ordinary query execution. But detecting the
"check" case is useless work except in the specific case of a table
rewrite. If we're going to do that at all, it seems to me that it
needs to be done via a code-path that's only going to get invoked in
the case of ALTER TABLE, not something that's going to happen every
time we parse an expression tree.I disagree that they're fundamentally dissimilar. They're _fundamentally_
similar, in that they are both fences, standing at different distances, around
the necessity of a cast function. Your argument only mentions performance, and
it boils down to a suggestion that we proceed to optimize things now by
separating these questions. Perhaps we should, but that's a different question.
Based on my benchmarks, I'm not seeing a need to micro-optimize.
I'm more concerned about modularity than performance. Sticking a
field into every FuncExpr so that if it happens to get passed to an
ALTER TABLE statement we can pull the flag out seems really ugly to
me.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
... A side issue is that I really
want to avoid adding a new parser keyword for this. It doesn't bother
me too much to add keywords for really important and user-visible
features, but when we're adding stuff that's only going to be used by
0.1% of our users it's really better if we can avoid it, because it
slows down the parser. Maybe we could do something like:
CREATE CAST (source_type AS target_type)
WITH FUNCTION function_name (argument_type, [, ...])
[ ANALYZE USING function_name ]
[ AS ASSIGNMENT | AS IMPLICIT ]
I'm not terribly thrilled with the suggestion of "ANALYZE" here, because
given the way we use that word elsewhere, people are likely to think it
means something to do with statistics collection; or at least that it
implies some deep and complicated analysis of the cast.
I suggest using a phrase based on the idea that this function tells you
whether you can skip the cast, or (if the sense is inverted) whether the
cast has to be executed. "SKIP IF function_name" would be nice but SKIP
isn't an extant keyword either. The best variant that I can come up
with after a minute's contemplation of kwlist.h is "NO WORK IF
function_name". If you didn't mind inverting the sense of the result
then we could use "EXECUTE IF function_name".
Internally, we could refer to a function of this type as a "cast analyzer".
Another possibility is to call it a "cast checker" and use CHECK USING.
But I'm not sure that's much better than ANALYZE in terms of conveying
the purpose to a newbie.
I believe the patch does implement Tom's suggestion, at least in spirit. �He
mentioned expression_planner() as the place to do it. �In principle, We could do
it *right there* and avoid any changes to coerce_to_target_type(), but the
overhead would increase. �Specifically, we would be walking an already-built
expression tree looking for casts to remove, probably doing a syscache lookup
for every cast to grab the exemptor function OID.
No no no no. The right place to do it is during expression
simplification in eval_const_expressions(). We are already
disassembling and reassembling the tree, and looking up functions,
in that pass. Detecting that a call is a no-op fits into that
very naturally. Think of it as an alternative to inline_function().
One point worth making here is that eval_const_expressions() does not
currently care very much whether a function call came from cast syntax
or explicitly. It might be worth thinking about whether we want to have
a generic this-function-call-is-a-no-op simplifier hook available for
*all* functions not just those that are casts. I'm not sure we want to
pay the overhead of another pg_proc column, but it's something to think
about.
regards, tom lane
On Wed, Jan 26, 2011 at 12:47 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
... A side issue is that I really
want to avoid adding a new parser keyword for this. It doesn't bother
me too much to add keywords for really important and user-visible
features, but when we're adding stuff that's only going to be used by
0.1% of our users it's really better if we can avoid it, because it
slows down the parser. Maybe we could do something like:CREATE CAST (source_type AS target_type)
WITH FUNCTION function_name (argument_type, [, ...])
[ ANALYZE USING function_name ]
[ AS ASSIGNMENT | AS IMPLICIT ]I'm not terribly thrilled with the suggestion of "ANALYZE" here, because
given the way we use that word elsewhere, people are likely to think it
means something to do with statistics collection; or at least that it
implies some deep and complicated analysis of the cast.I suggest using a phrase based on the idea that this function tells you
whether you can skip the cast, or (if the sense is inverted) whether the
cast has to be executed. "SKIP IF function_name" would be nice but SKIP
isn't an extant keyword either. The best variant that I can come up
with after a minute's contemplation of kwlist.h is "NO WORK IF
function_name". If you didn't mind inverting the sense of the result
then we could use "EXECUTE IF function_name".
What about borrowing from the trigger syntax?
WITH FUNCTION function_name (argument_type, [...]) WHEN
function_that_returns_true_when_the_call_is_needed
One point worth making here is that eval_const_expressions() does not
currently care very much whether a function call came from cast syntax
or explicitly. It might be worth thinking about whether we want to have
a generic this-function-call-is-a-no-op simplifier hook available for
*all* functions not just those that are casts. I'm not sure we want to
pay the overhead of another pg_proc column, but it's something to think
about.
It's not obvious to me that it has a use case outside of casts, but
it's certainly possible I'm missing something.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jan 26, 2011 at 12:47 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
If you didn't mind inverting the sense of the result
then we could use "EXECUTE IF function_name".
What about borrowing from the trigger syntax?
WITH FUNCTION function_name (argument_type, [...]) WHEN
function_that_returns_true_when_the_call_is_needed
That's a good thought. Or we could use WHEN NOT check_function if you
want to keep to the negative-result semantics.
One point worth making here is that eval_const_expressions() does not
currently care very much whether a function call came from cast syntax
or explicitly. �It might be worth thinking about whether we want to have
a generic this-function-call-is-a-no-op simplifier hook available for
*all* functions not just those that are casts. �I'm not sure we want to
pay the overhead of another pg_proc column, but it's something to think
about.
It's not obvious to me that it has a use case outside of casts, but
it's certainly possible I'm missing something.
A possible example is simplifying X + 0 to X, or X * 0 to 0. I've never
wanted to inject such datatype-specific knowledge directly into the
planner, but if we viewed it as function-specific knowledge supplied by
a per-function helper routine, maybe it would fly. Knowing that a cast
function does nothing useful for particular cases could be seen as a
special case of this type of simplification.
regards, tom lane
On Wed, Jan 26, 2011 at 3:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jan 26, 2011 at 12:47 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
If you didn't mind inverting the sense of the result
then we could use "EXECUTE IF function_name".What about borrowing from the trigger syntax?
WITH FUNCTION function_name (argument_type, [...]) WHEN
function_that_returns_true_when_the_call_is_neededThat's a good thought. Or we could use WHEN NOT check_function if you
want to keep to the negative-result semantics.
Seems a bit contorted; I don't see any particular reason to prefer
positive vs negative semantics in this case.
One point worth making here is that eval_const_expressions() does not
currently care very much whether a function call came from cast syntax
or explicitly. It might be worth thinking about whether we want to have
a generic this-function-call-is-a-no-op simplifier hook available for
*all* functions not just those that are casts. I'm not sure we want to
pay the overhead of another pg_proc column, but it's something to think
about.It's not obvious to me that it has a use case outside of casts, but
it's certainly possible I'm missing something.A possible example is simplifying X + 0 to X, or X * 0 to 0. I've never
wanted to inject such datatype-specific knowledge directly into the
planner, but if we viewed it as function-specific knowledge supplied by
a per-function helper routine, maybe it would fly. Knowing that a cast
function does nothing useful for particular cases could be seen as a
special case of this type of simplification.
Oh, I see. The times I've seen an issue with those kinds of
expressions have always been related to selectivity estimation. For
example, you'd like to get a selectivity estimate of 1-nullfrac for
(x+0)=x and 0 for (x+1)=x, and maybe (1-nullfrac)/2 for (x%2)=0. This
would only handle the first of those cases, so I'm inclined to think
it's too weak to have much general utility. The casting cases can, I
think, have a much larger impact - they occur more often in practice,
and if you can (e.g.) avoid an entire table rewrite, that's a pretty
big deal.
Another semi-related problem case I've run across is that CASE WHEN
... THEN 1 WHEN ... THEN 2 END ought to be known to only ever emit 1
and 2, and the selectivity of comparing that to some other value ought
to be computed on that basis. But now I'm really wandering off into
the weeds.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jan 26, 2011 at 3:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
It's not obvious to me that it has a use case outside of casts, but
it's certainly possible I'm missing something.
A possible example is simplifying X + 0 to X, or X * 0 to 0.
Oh, I see. The times I've seen an issue with those kinds of
expressions have always been related to selectivity estimation.
Yeah, helping the planner recognize equivalent cases is at least as
large a reason for wanting this as any direct savings of execution time.
I don't mind confining the feature to casts to start with, but it might
be a good idea to specify the check-function API in a way that would let
it be plugged into a more generally available call-simplification hook
later.
regards, tom lane
On Wed, Jan 26, 2011 at 4:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jan 26, 2011 at 3:08 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
It's not obvious to me that it has a use case outside of casts, but
it's certainly possible I'm missing something.A possible example is simplifying X + 0 to X, or X * 0 to 0.
Oh, I see. The times I've seen an issue with those kinds of
expressions have always been related to selectivity estimation.Yeah, helping the planner recognize equivalent cases is at least as
large a reason for wanting this as any direct savings of execution time.
Agreed.
I don't mind confining the feature to casts to start with, but it might
be a good idea to specify the check-function API in a way that would let
it be plugged into a more generally available call-simplification hook
later.
Got any suggestions? My thought was that it should just get (type,
typemod, type, typemod) and return a boolean. We could do something
more complicated, like Expr -> Expr, but I'm pretty unconvinced
there's any value there. I'd like to see some kind of appropriate
hook for interjecting selectivity estimates for cases that we
currently can't recognize, but my gut feeling is that that's
insufficiently related at the problem at hand to worry about it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jan 26, 2011 at 4:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I don't mind confining the feature to casts to start with, but it might
be a good idea to specify the check-function API in a way that would let
it be plugged into a more generally available call-simplification hook
later.
Got any suggestions? My thought was that it should just get (type,
typemod, type, typemod) and return a boolean. We could do something
more complicated, like Expr -> Expr, but I'm pretty unconvinced
there's any value there.
Well, (type, typemod, type, typemod) would be fine as long as the only
case you're interested in optimizing is typemod-lengthening coercions.
I think we're going to want the more general Expr-to-Expr capability
eventually.
I guess we could do the restricted case for now, with the idea that
there could be a standard Expr-to-Expr interface function created later
and installed into the generic hook facility for functions that are cast
functions. That could look into pg_cast and make the more restricted
call when appropriate. (The meat of this function would be the same
code you'd have to add to eval_const_expressions anyway for the
hard-wired version...)
I'd like to see some kind of appropriate
hook for interjecting selectivity estimates for cases that we
currently can't recognize, but my gut feeling is that that's
insufficiently related at the problem at hand to worry about it.
Agreed, that seems a bit off-topic. There are ancient comments in the
source code complaining about the lack of selectivity hooks for function
(as opposed to operator) calls, but that's not what Noah signed up to
fix. In any case I'm not sure that expression simplification is
closely connected to selectivity estimation --- to my mind it's an
independent process that is good to run first. As an example, the ALTER
TABLE case has a use for the former but not the latter.
regards, tom lane
On Wed, Jan 26, 2011 at 5:01 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Jan 26, 2011 at 4:02 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I don't mind confining the feature to casts to start with, but it might
be a good idea to specify the check-function API in a way that would let
it be plugged into a more generally available call-simplification hook
later.Got any suggestions? My thought was that it should just get (type,
typemod, type, typemod) and return a boolean. We could do something
more complicated, like Expr -> Expr, but I'm pretty unconvinced
there's any value there.Well, (type, typemod, type, typemod) would be fine as long as the only
case you're interested in optimizing is typemod-lengthening coercions.
I think we're going to want the more general Expr-to-Expr capability
eventually.I guess we could do the restricted case for now, with the idea that
there could be a standard Expr-to-Expr interface function created later
and installed into the generic hook facility for functions that are cast
functions. That could look into pg_cast and make the more restricted
call when appropriate. (The meat of this function would be the same
code you'd have to add to eval_const_expressions anyway for the
hard-wired version...)
Well, if you're positive we're eventually going to want this in
pg_proc, we may as well add it now. But I'm not too convinced it's
the right general API. The number of people writing exactly x + 0 or
x * 0 in a query has got to be vanishingly small; I'm not eager to add
additional parse analysis time to every SQL statement that has a
function in it just to detect those cases. Even slightly more
complicated problems seem intractable - e.g. (x + 1) = x can be
simplified to constant false, and NOT ((x + 1) = x) can be simplified
to x IS NOT NULL, but under the proposed API those would have to hang
off of =(int4,int4), which seems pretty darn ugly.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
Well, if you're positive we're eventually going to want this in
pg_proc, we may as well add it now. But I'm not too convinced it's
the right general API. The number of people writing exactly x + 0 or
x * 0 in a query has got to be vanishingly small; I'm not eager to add
additional parse analysis time to every SQL statement that has a
function in it just to detect those cases.
Actually, you've got that backwards: the facility I've got in mind would
cost next to nothing when not used. The place where we'd want to insert
this in eval_const_expressions has already got its hands on the relevant
pg_proc row, so checking for a nonzero hook-function reference would be
a matter of a couple of instructions. If we go with a pg_cast entry
then we're going to have to add a pg_cast lookup for every cast, whether
it turns out to be optimizable or not; which is going to cost quite a
lot more. The intermediate hook function I was sketching might be
worthwhile from a performance standpoint even if we don't expose the
more general feature to users, just because it would be possible to
avoid useless pg_cast lookups (by not installing the hook except on
pg_proc entries for which there's a relevant CAST WHEN function to call).
Even slightly more
complicated problems seem intractable - e.g. (x + 1) = x can be
simplified to constant false, and NOT ((x + 1) = x) can be simplified
to x IS NOT NULL, but under the proposed API those would have to hang
off of =(int4,int4), which seems pretty darn ugly.
True, but where else are you going to hang them off of? Not that I was
particularly thinking of doing either one of those.
regards, tom lane
On Wed, Jan 26, 2011 at 5:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Well, if you're positive we're eventually going to want this in
pg_proc, we may as well add it now. But I'm not too convinced it's
the right general API. The number of people writing exactly x + 0 or
x * 0 in a query has got to be vanishingly small; I'm not eager to add
additional parse analysis time to every SQL statement that has a
function in it just to detect those cases.Actually, you've got that backwards: the facility I've got in mind would
cost next to nothing when not used. The place where we'd want to insert
this in eval_const_expressions has already got its hands on the relevant
pg_proc row, so checking for a nonzero hook-function reference would be
a matter of a couple of instructions. If we go with a pg_cast entry
then we're going to have to add a pg_cast lookup for every cast, whether
it turns out to be optimizable or not; which is going to cost quite a
lot more. The intermediate hook function I was sketching might be
worthwhile from a performance standpoint even if we don't expose the
more general feature to users, just because it would be possible to
avoid useless pg_cast lookups (by not installing the hook except on
pg_proc entries for which there's a relevant CAST WHEN function to call).
Oh, really? I was thinking the logic should go into find_coercion_pathway().
Even slightly more
complicated problems seem intractable - e.g. (x + 1) = x can be
simplified to constant false, and NOT ((x + 1) = x) can be simplified
to x IS NOT NULL, but under the proposed API those would have to hang
off of =(int4,int4), which seems pretty darn ugly.True, but where else are you going to hang them off of? Not that I was
particularly thinking of doing either one of those.
Beats me, just thinking out loud.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
Oh, really? I was thinking the logic should go into find_coercion_pathway().
Well, I've been saying right along that it should be in
eval_const_expressions. Putting this sort of optimization in the parser
is invariably the wrong thing, because it fails to catch all the
possibilities. As an example, inlining a SQL function could expose
opportunities of this type. Another issue is that premature
optimization in the parser creates headaches if conditions change such
that a previous optimization is no longer valid --- you may have stored
rules wherein the optimization was already applied. (Not sure that
specific issue applies to casting, since we have no ALTER CAST commmand;
but in general you want expression optimizations applied downstream from
the rule rewriter not upstream.)
regards, tom lane
On Wed, Jan 26, 2011 at 5:41 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Oh, really? I was thinking the logic should go into find_coercion_pathway().
Well, I've been saying right along that it should be in
eval_const_expressions. Putting this sort of optimization in the parser
is invariably the wrong thing, because it fails to catch all the
possibilities. As an example, inlining a SQL function could expose
opportunities of this type. Another issue is that premature
optimization in the parser creates headaches if conditions change such
that a previous optimization is no longer valid --- you may have stored
rules wherein the optimization was already applied. (Not sure that
specific issue applies to casting, since we have no ALTER CAST commmand;
but in general you want expression optimizations applied downstream from
the rule rewriter not upstream.)
Well, I guess my thought was that we what we were doing is extending
the coercion system to be able to make decisions based on both type
OID and typemod. It seems very odd to make an initial decision based
on type OID in one place and then have a completely different system
for incorporating the typemod; and it also seems quite a bit more
expensive, since you'd have to consider it for every function, not
just casts.
A further problem is that I don't think it'd play well at all with the
richer-typemod concept we keep bandying about. If, for example, we
represented all arrays with the same OID (getting rid of the
double-entries in pg_type) and used some kind of augmented-typemod to
store the number of dimensions and the base type, this would
completely fall over.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
I wrote:
... Another issue is that premature
optimization in the parser creates headaches if conditions change such
that a previous optimization is no longer valid --- you may have stored
rules wherein the optimization was already applied. (Not sure that
specific issue applies to casting, since we have no ALTER CAST commmand;
but in general you want expression optimizations applied downstream from
the rule rewriter not upstream.)
Actually, I can construct a concrete example where applying this
optimization in the parser will do the wrong thing:
CREATE TABLE base (f1 varchar(4));
CREATE VIEW vv AS SELECT f1::varchar(8) FROM base;
ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16);
If the parser is taught to throw away "useless" length coercions, then
the stored form of vv will contain no cast, and the results will be
wrong after base's column is widened.
regards, tom lane
On Wed, Jan 26, 2011 at 05:55:37PM -0500, Tom Lane wrote:
I wrote:
... Another issue is that premature
optimization in the parser creates headaches if conditions change such
that a previous optimization is no longer valid --- you may have stored
rules wherein the optimization was already applied. (Not sure that
specific issue applies to casting, since we have no ALTER CAST commmand;
but in general you want expression optimizations applied downstream from
the rule rewriter not upstream.)Actually, I can construct a concrete example where applying this
optimization in the parser will do the wrong thing:CREATE TABLE base (f1 varchar(4));
CREATE VIEW vv AS SELECT f1::varchar(8) FROM base;
ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16);If the parser is taught to throw away "useless" length coercions, then
the stored form of vv will contain no cast, and the results will be
wrong after base's column is widened.
We reject that:
[local] test=# ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16);
ERROR: cannot alter type of a column used by a view or rule
DETAIL: rule _RETURN on view vv depends on column "f1"
Robert Haas <robertmhaas@gmail.com> writes:
Well, I guess my thought was that we what we were doing is extending
the coercion system to be able to make decisions based on both type
OID and typemod.
Well, that's an interesting thought, but the proposal at hand is far
more limited than that --- it's only an optimization that can be applied
in limited situations. If we want to do something like what you're
suggesting here, it's not going to get done for 9.1.
A further problem is that I don't think it'd play well at all with the
richer-typemod concept we keep bandying about. If, for example, we
represented all arrays with the same OID (getting rid of the
double-entries in pg_type) and used some kind of augmented-typemod to
store the number of dimensions and the base type, this would
completely fall over.
Your proposal would completely fall over, you mean. An Expr->Expr hook
API wouldn't be affected at all.
I'm not sure how important that concern is though, because it's hard to
see how any such change wouldn't break existing cast implementation
functions anyway. If the API for length-coercing cast functions
changes, breaking their helper functions too hardly seems like an issue.
Or are you saying you want to punt this whole proposal till after that
happens? I had muttered something of the sort way upthread, but I
didn't think anyone else thought that way.
regards, tom lane
Noah Misch <noah@leadboat.com> writes:
On Wed, Jan 26, 2011 at 05:55:37PM -0500, Tom Lane wrote:
Actually, I can construct a concrete example where applying this
optimization in the parser will do the wrong thing:CREATE TABLE base (f1 varchar(4));
CREATE VIEW vv AS SELECT f1::varchar(8) FROM base;
ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16);If the parser is taught to throw away "useless" length coercions, then
the stored form of vv will contain no cast, and the results will be
wrong after base's column is widened.
We reject that:
[local] test=# ALTER TABLE base ALTER COLUMN f1 TYPE varchar(16);
ERROR: cannot alter type of a column used by a view or rule
DETAIL: rule _RETURN on view vv depends on column "f1"
Nonetheless, the stored form of vv will contain no cast, which can
hardly be thought safe here. Relaxing the restriction you cite is (or
should be) on the TODO list, but we'll never be able to do it if the
parser throws away relevant information.
regards, tom lane
On Wed, Jan 26, 2011 at 05:32:00PM -0500, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Well, if you're positive we're eventually going to want this in
pg_proc, we may as well add it now. But I'm not too convinced it's
the right general API. The number of people writing exactly x + 0 or
x * 0 in a query has got to be vanishingly small; I'm not eager to add
additional parse analysis time to every SQL statement that has a
function in it just to detect those cases.Actually, you've got that backwards: the facility I've got in mind would
cost next to nothing when not used. The place where we'd want to insert
this in eval_const_expressions has already got its hands on the relevant
pg_proc row, so checking for a nonzero hook-function reference would be
a matter of a couple of instructions. If we go with a pg_cast entry
then we're going to have to add a pg_cast lookup for every cast, whether
it turns out to be optimizable or not; which is going to cost quite a
lot more. The intermediate hook function I was sketching might be
worthwhile from a performance standpoint even if we don't expose the
more general feature to users, just because it would be possible to
avoid useless pg_cast lookups (by not installing the hook except on
pg_proc entries for which there's a relevant CAST WHEN function to call).
If we hook this into eval_const_expressions, it definitely seems cleaner to
attach the auxiliary function to the pg_proc. Otherwise, we'd reconstruct which
cast led to each function call -- is there even enough information available to
do so unambiguously? Unlike something typmod-specific, these functions would
effectively need to be written in C. Seems like a perfectly acceptable
constraint, though.
For the syntax, then, would a new common_func_opt_item of "WHEN func" fit?
That covers fully-removable casts, but ALTER TABLE still needs to identify casts
that may throw errors but never change the value's binary representation. Where
does that fit? Another pg_proc column for a function called to answer that
question, called only from an ALTER TABLE-specific code path?
Thanks for the feedback/analysis.
nm
Noah Misch <noah@leadboat.com> writes:
If we hook this into eval_const_expressions, it definitely seems
cleaner to attach the auxiliary function to the pg_proc. Otherwise,
we'd reconstruct which cast led to each function call -- is there even
enough information available to do so unambiguously?
As far as that goes, yes there is --- otherwise ruleutils would be
unable to reverse-list cast constructs. IIRC the function call is
marked as being a cast, and you get the source and dest type IDs
from ordinary exprType() inspection.
For the syntax, then, would a new common_func_opt_item of "WHEN func" fit?
WHEN is appropriate for the restricted CAST case, but it doesn't seem
like le mot juste for a general function option, unless we restrict
the helper function to return boolean which is not what I had in mind.
That covers fully-removable casts, but ALTER TABLE still needs to identify casts
that may throw errors but never change the value's binary representation.
I remain completely unexcited about optimizing that case, especially if
it doesn't fit into a general framework. The bang for the buck ratio
is not good: too much work, too much uglification, too little return.
regards, tom lane
On Wed, Jan 26, 2011 at 06:29:57PM -0500, Tom Lane wrote:
Noah Misch <noah@leadboat.com> writes:
If we hook this into eval_const_expressions, it definitely seems
cleaner to attach the auxiliary function to the pg_proc. Otherwise,
we'd reconstruct which cast led to each function call -- is there even
enough information available to do so unambiguously?As far as that goes, yes there is --- otherwise ruleutils would be
unable to reverse-list cast constructs. IIRC the function call is
marked as being a cast, and you get the source and dest type IDs
from ordinary exprType() inspection.
Good point. So it is, FuncExpr.funcformat conveys that.
For the syntax, then, would a new common_func_opt_item of "WHEN func" fit?
WHEN is appropriate for the restricted CAST case, but it doesn't seem
like le mot juste for a general function option, unless we restrict
the helper function to return boolean which is not what I had in mind.
True. Uhh, "PARSER MAPPING func"? "PLANS CONVERSION func"?
That covers fully-removable casts, but ALTER TABLE still needs to identify casts
that may throw errors but never change the value's binary representation.I remain completely unexcited about optimizing that case, especially if
it doesn't fit into a general framework. The bang for the buck ratio
is not good: too much work, too much uglification, too little return.
The return looks attractive when you actually save six hours of downtime. If
I'm the only one that sees such a savings for one of his databases, though, I
suppose it's not worthwhile. We'd miss optimizing these cases:
numeric(8,2) -> numeric(7,2)
varbit(8) -> varbit(7)
text -> xml
The xml one is the most interesting to me. In the design thread, Robert also
expressed interest in that one. Jim Nasby mentioned numeric generally; Jim,
what kind of numeric conversions are important to you? If anyone else will miss
one of those greatly, please speak up.
I found that many interesting cases in this area, most notably varchar(8) ->
varchar(4), are safe a good deal of the time, but not provably so. So perhaps a
system for automatically detecting them would be overkill, but an addition to
the ALTER TABLE syntax[1]http://archives.postgresql.org/message-id/20110106042626.GA28230@tornado.leadboat.com to request them would be worthwhile.
nm
[1]: http://archives.postgresql.org/message-id/20110106042626.GA28230@tornado.leadboat.com
Noah Misch <noah@leadboat.com> writes:
On Wed, Jan 26, 2011 at 06:29:57PM -0500, Tom Lane wrote:
I remain completely unexcited about optimizing that case, especially if
it doesn't fit into a general framework. The bang for the buck ratio
is not good: too much work, too much uglification, too little return.
The return looks attractive when you actually save six hours of downtime. If
I'm the only one that sees such a savings for one of his databases, though, I
suppose it's not worthwhile. We'd miss optimizing these cases:
numeric(8,2) -> numeric(7,2)
varbit(8) -> varbit(7)
text -> xml
But how often do those really come up? And do you really save that
much? The table still has to be locked against other users, so you're
still down, and you're still doing all the reads and computation. I
don't deny that saving the writes is worth something; I just don't agree
that it's worth the development and maintenance effort that such a wart
is going to cost us. User-exposed features are *expensive*.
regards, tom lane
Noah Misch <noah@leadboat.com> writes:
text -> xml
BTW, that reminds me of something that I think was mentioned way back
when, but absolutely fails to fit into any of the frameworks discussed
here: the mere fact that a type is binary-compatible (with or without
checking) doesn't make it compatible enough to not have to recreate
indexes. Where and how are we going to have a wart to determine if
that's necessary? And if the answer is "rebuild indexes whenever the
data type changes", isn't that a further big dent in the argument that
it's worth avoiding a table rewrite? A text->xml replacement is going
to be far from cheap anyway.
regards, tom lane
On Wed, Jan 26, 2011 at 6:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Well, I guess my thought was that we what we were doing is extending
the coercion system to be able to make decisions based on both type
OID and typemod.Well, that's an interesting thought, but the proposal at hand is far
more limited than that --- it's only an optimization that can be applied
in limited situations. If we want to do something like what you're
suggesting here, it's not going to get done for 9.1.
Eh, why is this not entirely straightforward? *scratches head*
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Jan 26, 2011 at 7:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
But how often do those really come up? And do you really save that
much? The table still has to be locked against other users, so you're
still down, and you're still doing all the reads and computation. I
don't deny that saving the writes is worth something; I just don't agree
that it's worth the development and maintenance effort that such a wart
is going to cost us. User-exposed features are *expensive*.
I would think that text -> [something that's still a varlena but with
tighter validation] would be quite common.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Jan 26, 2011 at 07:44:43PM -0500, Tom Lane wrote:
numeric(8,2) -> numeric(7,2)
varbit(8) -> varbit(7)
text -> xmlBut how often do those really come up?
I'll speak from my own experience, having little idea of the larger community
experience on this one. I usually don't even contemplate changes like this on
nontrivial tables, because the pain of the downtime usually won't make up for
getting the schema just like I want it. Cases that I can't discard on those
grounds are fairly rare. As an order-of-magnitude estimate, I'd throw out one
instance per DBA-annum among the DBAs whose work I witness.
And do you really save that
much? The table still has to be locked against other users, so you're
still down, and you're still doing all the reads and computation. I
don't deny that saving the writes is worth something; I just don't agree
that it's worth the development and maintenance effort that such a wart
is going to cost us. User-exposed features are *expensive*.
If you have no indexes, you still save 50-75% of the cost by just reading and
computing, not rewriting. Each index you add, even if it doesn't involve the
column, pushes that advantage even further. With a typical handful of indexes,
a 95+% cost savings is common enough.
If we implemented ALTER TABLE ... SET DATA TYPE ... IMPLICIT, I'd agree that the
marginal value of automatically detecting the above three cases would not
justify the cost.
On Wed, Jan 26, 2011 at 07:52:10PM -0500, Tom Lane wrote:
Noah Misch <noah@leadboat.com> writes:
text -> xml
BTW, that reminds me of something that I think was mentioned way back
when, but absolutely fails to fit into any of the frameworks discussed
here: the mere fact that a type is binary-compatible (with or without
checking) doesn't make it compatible enough to not have to recreate
indexes. Where and how are we going to have a wart to determine if
that's necessary?
Design (section 3):
http://archives.postgresql.org/message-id/20101229125625.GA27643@tornado.gateway.2wire.net
Implementation:
http://archives.postgresql.org/message-id/20110113230124.GA18733@tornado.gateway.2wire.net
[disclaimer: I've yet to post an updated version fixing a localized bug in this patch]
I ended up making no attempt to optimize indexes that have predicates or
expression columns; we'll just rebuild them every time. Aside from that, I
failed to find an index on built-in types that requires a rebuild after a type
change optimized by this patch stack. So, the entire wart is really for the
benefit of third-party types that might need it.
And if the answer is "rebuild indexes whenever the
data type changes", isn't that a further big dent in the argument that
it's worth avoiding a table rewrite?
No. Rewriting the table means rebuilding *all* indexes, but the worst case for
a non-rewrite type change is to rebuild all indexes that depend on the changed
column. That's a large win by itself, but we'll usually do even better.
A text->xml replacement is going
to be far from cheap anyway.
It's tough to generalize. You can certainly construct a pathological case with
minimal win, but you can just as well construct the opposite. Consider a wide
table with a narrow XML column. Consider a usually-NULL XML column.
nm
On Wed, Jan 26, 2011 at 09:35:24AM -0500, Robert Haas wrote:
On Mon, Jan 24, 2011 at 11:13 PM, Noah Misch <noah@leadboat.com> wrote:
This helps on conversions like varchar(4)->varchar(8) and text->xml.
I've read through this patch somewhat. ?As I believe Tom also
commented previously, exemptor is a pretty bad choice of name.I spent awhile with a thesaurus before coining that. ?Since Tom's comment, I've
been hoping the next person to complain would at least suggest a better name.
Shooting in the dark on a nomenclature issue isn't fun, and I'm not picky.
CREATE CAST (source_type AS target_type)
WITH FUNCTION function_name (argument_type, [, ...])
[ ANALYZE USING function_name ]
[ AS ASSIGNMENT | AS IMPLICIT ]
Thanks for writing about it. Seems the rest of the thread converged pretty well
on a set of viable name candidates.
?I think what this patch is trying to do is tag the
call to the cast function with the information that we might not need
to call it, but ISTM it would be better to just notice that the
function call is unnecessary and insert a RelabelType node instead.This patch does exactly that for varchar(4) -> varchar(8) and similar cases.
Oh, uh, err... OK, I obviously haven't understood it well enough.
I'll look at it some more, but if there's anything you can write up to
explain what you changed and why, that would be helpful.
Based on downthread discussion, I figure this will all change a good deal. I'll
still briefly explain the patch as written. Most of the patch is plumbing to
support the new syntax, catalog entries, and FuncExpr field. The important
changes are in parse_coerce.c. I modified find_coercion_pathway() and
find_typmod_coercion_function() to retrieve pg_cast.castexemptor alongside
pg_cast.castfunc. Their callers (coerce_type() and coerce_type_typmod(),
respectively) then call the new apply_exemptor_function(), which calls the
exemptor function, if any, returns the value to place in FuncExpr.funcexempt,
and possibly updates the CoercionPathType the caller is about to use.
build_coercion_expression(), unchanged except to populate FuncExpr.funcexempt,
remains responsible for creating the appropriate node (RelabelType, FuncExpr).
Finally, I change GetCoerceExemptions to use FuncExpr.funcexempt.
I think I'm
also having trouble with the fact that these patches don't apply
cleanly against the master branch, because they're stacked up on each
other. Maybe this will be easier once we get more of the ALTER TYPE 2
stuff in.
It's certainly tricky to review a bunch of patches in parallel when they have a
serial dependency chain. I'll merge the at0 and at2 bits per your request to
see what we can do there.
As for performance, coerce_to_target_type() is certainly in wide use, but it's
not exactly a hot path. ?I prepared and ran the attached bench-coerce-worst.sql
and bench-coerce-best.sql. ?Both are quite artificial. ?The first one basically
asks "Can we measure the new overhead at all?" ?The second one asks "Would any
ordinary query benefit from removing a superfluous cast from an expression
tree?" ?The worst case had an 4% _speedup_, and the best case had a 27% speedup,
suggesting answers of "no" and "yes (but not dramatically)". ?The "worst-case"
speedup doesn't make sense, so it's probably an artifact of some recent change
on master creating a larger performance dip in this pathological test case. ?I
could rebase the patch and rerun the benchmark, but I doubt an exact figure
would be much more meaningful. ?Unfortunately, I can't think of any more-natural
tests (help welcome) that would still illustrate any performance difference.That certainly seems promising, but I don't understand how the new
code can be faster on your constructed worst case. Thoughts?
I hadn't merged master into my at3 branch in awhile; my best guess is that some
recent change in master slowed that test case down by about 4%. I could merge
up to confirm that theory. Again, though, it's an abjectly silly test case.
Even if the test showed a 50% slowdown, we wouldn't have much cause for concern.
Perhaps a 300% slowdown would have been notable.
I'm more concerned about modularity than performance. Sticking a
field into every FuncExpr so that if it happens to get passed to an
ALTER TABLE statement we can pull the flag out seems really ugly to
me.
Understood. I agree that it's awfully specialized to be hanging around in
FuncExpr. Doing it this way seemed to minimize the global quantity of ugly
code, but you're right -- better to have somewhat-ugly code in tablecmds.c than
to expose this in FuncExpr. Most of the benefit of the current approach will be
gone when the optimization moves to eval_const_expressions(), anyway. One way
or another, the next version will not do this.
Thanks,
nm
On Thu, Jan 27, 2011 at 1:14 AM, Noah Misch <noah@leadboat.com> wrote:
Based on downthread discussion, I figure this will all change a good deal. I'll
still briefly explain the patch as written. Most of the patch is plumbing to
support the new syntax, catalog entries, and FuncExpr field. The important
changes are in parse_coerce.c. I modified find_coercion_pathway() and
find_typmod_coercion_function() to retrieve pg_cast.castexemptor alongside
pg_cast.castfunc. Their callers (coerce_type() and coerce_type_typmod(),
respectively) then call the new apply_exemptor_function(), which calls the
exemptor function, if any, returns the value to place in FuncExpr.funcexempt,
and possibly updates the CoercionPathType the caller is about to use.
build_coercion_expression(), unchanged except to populate FuncExpr.funcexempt,
remains responsible for creating the appropriate node (RelabelType, FuncExpr).
Finally, I change GetCoerceExemptions to use FuncExpr.funcexempt.
OK. I was thinking that instead moving this into
eval_const_expressions(), we just make the logic in
find_coercion_pathway() call the "exemptor" function (or whatever we
call it) right around here:
switch (castForm->castmethod)
{
case COERCION_METHOD_FUNCTION:
result = COERCION_PATH_FUNC;
*funcid = castForm->castfunc;
break;
case COERCION_METHOD_INOUT:
result = COERCION_PATH_COERCEVIAIO;
break;
case COERCION_METHOD_BINARY:
result = COERCION_PATH_RELABELTYPE;
break;
default:
elog(ERROR, "unrecognized
castmethod: %d",
(int) castForm->castmethod);
break;
}
If it's COERCION_METHOD_FUNCTION, then instead of unconditionally
setting the result to COERCION_PATH_FUNC, we inquire - based on the
typemods - whether it's OK to downgrade to a
COERCION_PATH_RELABELTYPE. The only fly in the ointment is that
find_coercion_pathway() doesn't current get the typemods. Not sure
how ugly that would be to fix.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
OK. I was thinking that instead moving this into
eval_const_expressions(), we just make the logic in
find_coercion_pathway() call the "exemptor" function (or whatever we
call it) right around here:
No, no, no, no. Didn't you read yesterday's discussion? parse_coerce
is entirely the wrong place for this.
regards, tom lane
On Thu, Jan 27, 2011 at 9:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
OK. I was thinking that instead moving this into
eval_const_expressions(), we just make the logic in
find_coercion_pathway() call the "exemptor" function (or whatever we
call it) right around here:No, no, no, no. Didn't you read yesterday's discussion? parse_coerce
is entirely the wrong place for this.
I not only read it, I participated in it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Jan 27, 2011 at 10:42 AM, Robert Haas <robertmhaas@gmail.com> wrote:
On Thu, Jan 27, 2011 at 9:46 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
OK. I was thinking that instead moving this into
eval_const_expressions(), we just make the logic in
find_coercion_pathway() call the "exemptor" function (or whatever we
call it) right around here:No, no, no, no. Didn't you read yesterday's discussion? parse_coerce
is entirely the wrong place for this.I not only read it, I participated in it.
To put that another way, there's a difference between reading
something and agreeing with it. I did the former, but not the latter.
It seems to me that this discussion is unnecessarily antagonistic.
Is it not OK for me to have a different idea about which way to go
with something than you do?
My personal view is that we ought to try to be increasing the number
of places where type modifiers behave like they're really part of the
type, rather than being an afterthought that we deal with when
convenient and otherwise ignore. Otherwise, I see the chances of any
substantive improvements in our type system as being just about zero.
However, the larger point here is simply this: I'm trying to make some
progress on reviewing and, where appropriate, committing the patches
that were submitted for this CommitFest. I'd like to try to avoid the
phenomenon where we're tripping over each other's feet. We're not
making out very well on that front at the moment.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
Is it not OK for me to have a different idea about which way to go
with something than you do?
Well, in this case I firmly believe that you're mistaken.
My personal view is that we ought to try to be increasing the number
of places where type modifiers behave like they're really part of the
type, rather than being an afterthought that we deal with when
convenient and otherwise ignore.
And this argument is 100% irrelevant to the problem. The problem is
that you want to put an optimization into the wrong phase of processing.
That is going to hurt us, tomorrow if not today, and it has got *no*
redeeming social value in terms of beng more flexible about what typmods
are or how "well integrated" they are.
regards, tom lane
On Thu, Jan 27, 2011 at 11:35 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
My personal view is that we ought to try to be increasing the number
of places where type modifiers behave like they're really part of the
type, rather than being an afterthought that we deal with when
convenient and otherwise ignore.And this argument is 100% irrelevant to the problem. The problem is
that you want to put an optimization into the wrong phase of processing.
That is going to hurt us, tomorrow if not today, and it has got *no*
redeeming social value in terms of beng more flexible about what typmods
are or how "well integrated" they are.
The only thing we're deciding here is whether or not a cast requires a
function. That's a function of the type OIDs and the typemods. I
don't see why it's wrong to do the portion that involves the types in
the same place as the portion that involves the typemods. Perhaps you
could explain.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Thu, Jan 27, 2011 at 09:02:26AM -0500, Robert Haas wrote:
OK. I was thinking that instead moving this into
eval_const_expressions(), we just make the logic in
find_coercion_pathway() call the "exemptor" function (or whatever we
call it) right around here:switch (castForm->castmethod)
{
case COERCION_METHOD_FUNCTION:
result = COERCION_PATH_FUNC;
*funcid = castForm->castfunc;
break;
case COERCION_METHOD_INOUT:
result = COERCION_PATH_COERCEVIAIO;
break;
case COERCION_METHOD_BINARY:
result = COERCION_PATH_RELABELTYPE;
break;
default:
elog(ERROR, "unrecognized
castmethod: %d",
(int) castForm->castmethod);
break;
}If it's COERCION_METHOD_FUNCTION, then instead of unconditionally
setting the result to COERCION_PATH_FUNC, we inquire - based on the
typemods - whether it's OK to downgrade to a
COERCION_PATH_RELABELTYPE. The only fly in the ointment is that
find_coercion_pathway() doesn't current get the typemods. Not sure
how ugly that would be to fix.
Basically, this is a stylistic variation of the approach from my original at3
patch. I believe I looked at that particular positioning and decided that the
extra arguments and effects on non-parse_coerce.c callers were undesirable.
Debatable as any style issue, though. Note that you need to do the same thing
in find_typmod_coercion_function().
On Wed, Jan 26, 2011 at 6:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I'm not sure how important that concern is though, because it's hard to
see how any such change wouldn't break existing cast implementation
functions anyway. If the API for length-coercing cast functions
changes, breaking their helper functions too hardly seems like an issue.
Or are you saying you want to punt this whole proposal till after that
happens? I had muttered something of the sort way upthread, but I
didn't think anyone else thought that way.
I've been thinking about this patch a little bit more and I'm coming
around to the viewpoint that we should mark this (and the successor
patches in the same series) Returned with Feedback, and revisit the
issue for 9.2. I'm not necessarily signing on to the viewpoint that
we should wait to do any of this work until after we refactor
typemods, but it does strike me that the fact that Tom and I have
completely different ideas of how this will interact with future
changes in this area probably means we need to take some more time to
talk about what those future enhancements might look like, rather than
rushing something now and maybe regretting it later. We may not need
to actually do all that work before we do this, but it sounds like at
a minimum we need some agreement on what the design should look like.
I think there is still hope for ALTER TYPE 2 (really ALTER TABLE 2 or
SET DATA TYPE 2; it's misnamed) to be committed this cycle and I'll
continue reviewing that one to see whether we can get at least that
much done for 9.1. But I think the rest of this just needs more time
than we have to give it right now. There are more than 60 patches
left open in the CommitFest at this point, and we have less than three
weeks left. If we don't focus our efforts on the things where we have
clear agreement and good, finished code, we're going to end up either
dragging this CommitFest out for months or else getting very little
done for the next few weeks and then indiscriminately punting whatever
is left over at the end. I don't want to do either of those things,
so that means it's time to start making some tough choices, and
unfortunately I think this is one that's going to have to get cut, for
lack of agreement on the basic design.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Fri, Jan 28, 2011 at 01:52:32PM -0500, Robert Haas wrote:
On Wed, Jan 26, 2011 at 6:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I'm not sure how important that concern is though, because it's hard to
see how any such change wouldn't break existing cast implementation
functions anyway. ?If the API for length-coercing cast functions
changes, breaking their helper functions too hardly seems like an issue.
Or are you saying you want to punt this whole proposal till after that
happens? ?I had muttered something of the sort way upthread, but I
didn't think anyone else thought that way.I've been thinking about this patch a little bit more and I'm coming
around to the viewpoint that we should mark this (and the successor
patches in the same series) Returned with Feedback, and revisit the
issue for 9.2.
This is just.
I'm not necessarily signing on to the viewpoint that
we should wait to do any of this work until after we refactor
typemods, but it does strike me that the fact that Tom and I have
completely different ideas of how this will interact with future
changes in this area probably means we need to take some more time to
talk about what those future enhancements might look like, rather than
rushing something now and maybe regretting it later. We may not need
to actually do all that work before we do this, but it sounds like at
a minimum we need some agreement on what the design should look like.
I've deferred comment due to my obvious bias, but I can't see any sense in
blocking a change like this one on account of the exceptionally-preliminary
discussions about enriching typmod. Suppose, like my original design, we make
no provision to insulate against future typmod-related changes. The number of
interfaces that deal in typmod are so great that the marginal effort to update
the new ones will be irrelevant. I still like Tom's idea of an Expr<->Expr
interface. I like it because it opens more opportunities now, not because it
will eliminate some modicum of effort from an enriched-typmod implementation.
nm
On Fri, Jan 28, 2011 at 04:49:39PM -0500, Noah Misch wrote:
On Fri, Jan 28, 2011 at 01:52:32PM -0500, Robert Haas wrote:
On Wed, Jan 26, 2011 at 6:06 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I'm not sure how important that concern is though, because it's hard to
see how any such change wouldn't break existing cast implementation
functions anyway. ?If the API for length-coercing cast functions
changes, breaking their helper functions too hardly seems like an issue.
Or are you saying you want to punt this whole proposal till after that
happens? ?I had muttered something of the sort way upthread, but I
didn't think anyone else thought that way.I've been thinking about this patch a little bit more and I'm coming
around to the viewpoint that we should mark this (and the successor
patches in the same series) Returned with Feedback, and revisit the
issue for 9.2.This is just.
One other thing: #7 does not depend on #3,4,5,6 or any design problems raised
thus far, so there's no need to treat it the same as that group.
On Fri, Jan 28, 2011 at 4:49 PM, Noah Misch <noah@leadboat.com> wrote:
I'm not necessarily signing on to the viewpoint that
we should wait to do any of this work until after we refactor
typemods, but it does strike me that the fact that Tom and I have
completely different ideas of how this will interact with future
changes in this area probably means we need to take some more time to
talk about what those future enhancements might look like, rather than
rushing something now and maybe regretting it later. We may not need
to actually do all that work before we do this, but it sounds like at
a minimum we need some agreement on what the design should look like.I've deferred comment due to my obvious bias, but I can't see any sense in
blocking a change like this one on account of the exceptionally-preliminary
discussions about enriching typmod. Suppose, like my original design, we make
no provision to insulate against future typmod-related changes. The number of
interfaces that deal in typmod are so great that the marginal effort to update
the new ones will be irrelevant. I still like Tom's idea of an Expr<->Expr
interface. I like it because it opens more opportunities now, not because it
will eliminate some modicum of effort from an enriched-typmod implementation.
Once we add syntax to support this feature, we have to support that
syntax for an extremely long time. We can't just remove it in the
next release and replace it with something else - pg_dump has to still
work, for example, and we have to able to reload whatever it produces.
Adding a user-visible API that we may want to turn around and change
*next release* is just a bad idea. If we were talking about
implementing this through some sort of hard-coded internal list of
types, it wouldn't be quite so much of an issue, but that's not where
we're at. Moreover, I fear that injecting this into
eval_const_expressions() is adding a frammish that has no practical
utility apart from this case, but we still have to pay the overhead;
even Tom expressed some initial doubt about whether that made sense,
and I'm certainly not sold on it either. I don't see any particular
reason why we can't resolve all of these issues, but it's going to
take more time than we have right now.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Sat, Jan 29, 2011 at 3:26 AM, Noah Misch <noah@leadboat.com> wrote:
One other thing: #7 does not depend on #3,4,5,6 or any design problems raised
thus far, so there's no need to treat it the same as that group.
Well, if you want to post an updated patch that's independent of the
rest of the series, I guess you can... but I think the chances of that
getting applied for this release are pretty much zero. That patch
relies on some subtle changes to the contract implied by an operator
family and what looks at first blush like a lot of grotty hackery. I
can't get very excited about spending a lot of time on that right now,
especially given the amount of difficulty we've had reaching a meeting
of the minds on cases that don't have those problems.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company