Re: Re: Support for %TYPE in CREATE FUNCTION

This patch adds support for %TYPE in CREATE FUNCTION argument and
return types.

%TYPE is already supported by PL/pgSQL when declaring variables.
However, that does not help with the argument and return types in
CREATE FUNCTION.

Using %TYPE makes it easier to write a function which is independent
of the definition of a table. That is, minor changes to the types
used in the table may not require changes to the function.

For example, this trivial function will work whenever `table' which
has columns named `name' and `value', no matter what the types of the
columns are.

CREATE FUNCTION lookup (table.name%TYPE)
RETURNS table.value%TYPE
AS 'select value from table where name = $1'
LANGUAGE 'sql';

This patch includes changes to the testsuite and the documentation.

This work was sponsored by Zembu.

Ian

Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.126
diff -p -u -r1.126 parsenodes.h
--- src/include/nodes/parsenodes.h 2001/03/23 04:49:56 1.126
+++ src/include/nodes/parsenodes.h 2001/04/28 03:38:21
@@ -945,6 +945,7 @@ typedef struct TypeName
bool setof; /* is a set? */
int32 typmod; /* type modifier */
List *arrayBounds; /* array bounds */
+ char *attrname; /* field name when using %TYPE */
} TypeName;

/*
Index: src/backend/parser/analyze.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.183
diff -p -u -r1.183 analyze.c
--- src/backend/parser/analyze.c 2001/03/22 06:16:15 1.183
+++ src/backend/parser/analyze.c 2001/04/28 03:38:23
@@ -27,6 +27,7 @@
#include "parser/parse_relation.h"
#include "parser/parse_target.h"
#include "parser/parse_type.h"
+#include "parser/parse_expr.h"
#include "rewrite/rewriteManip.h"
#include "utils/builtins.h"
#include "utils/fmgroids.h"
@@ -49,7 +50,10 @@ static Node *transformSetOperationTree(P
static Query *transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt);
static Query *transformCreateStmt(ParseState *pstate, CreateStmt *stmt);
static Query *transformAlterTableStmt(ParseState *pstate, AlterTableStmt *stmt);
+static Node *transformTypeRefs(ParseState *pstate, Node *stmt);

+static void transformTypeRefsList(ParseState *pstate, List *l);
+static void transformTypeRef(ParseState *pstate, TypeName *tn);
static List *getSetColTypes(ParseState *pstate, Node *node);
static void transformForUpdate(Query *qry, List *forUpdate);
static void transformFkeyGetPrimaryKey(FkConstraint *fkconstraint);
@@ -230,6 +234,18 @@ transformStmt(ParseState *pstate, Node *
(SelectStmt *) parseTree);
break;

+ /*
+ * Convert use of %TYPE in statements where it is permitted.
+ */
+ case T_ProcedureStmt:
+ case T_CommentStmt:
+ case T_RemoveFuncStmt:
+ case T_DefineStmt:
+ result = makeNode(Query);
+ result->commandType = CMD_UTILITY;
+ result->utilityStmt = transformTypeRefs(pstate, parseTree);
+ break;
+
default:

/*
@@ -2607,6 +2623,104 @@ transformAlterTableStmt(ParseState *psta
}
qry->utilityStmt = (Node *) stmt;
return qry;
+}
+
+/*
+ * Transform uses of %TYPE in a statement.
+ */
+static Node *
+transformTypeRefs(ParseState *pstate, Node *stmt)
+{
+ switch (nodeTag(stmt))
+ {
+ case T_ProcedureStmt:
+ {
+ ProcedureStmt *ps = (ProcedureStmt *) stmt;
+
+ transformTypeRefsList(pstate, ps->argTypes);
+ transformTypeRef(pstate, (TypeName *) ps->returnType);
+ transformTypeRefsList(pstate, ps->withClause);
+ }
+ break;
+
+ case T_CommentStmt:
+ {
+ CommentStmt *cs = (CommentStmt *) stmt;
+
+ transformTypeRefsList(pstate, cs->objlist);
+ }
+ break;
+
+ case T_RemoveFuncStmt:
+ {
+ RemoveFuncStmt *rs = (RemoveFuncStmt *) stmt;
+
+ transformTypeRefsList(pstate, rs->args);
+ }
+ break;
+
+ case T_DefineStmt:
+ {
+ DefineStmt *ds = (DefineStmt *) stmt;
+ List *ele;
+
+ foreach(ele, ds->definition)
+ {
+ DefElem *de = (DefElem *) lfirst(ele);
+
+ if (de->arg != NULL
+ && IsA(de->arg, TypeName))
+ {
+ transformTypeRef(pstate, (TypeName *) de->arg);
+ }
+ }
+ }
+ break;
+
+ default:
+ elog(ERROR, "Unsupported type %d in transformTypeRefs",
+ nodeTag(stmt));
+ break;
+ }
+
+ return stmt;
+}
+
+/*
+ * Transform uses of %TYPE in a list.
+ */
+static void
+transformTypeRefsList(ParseState *pstate, List *l)
+{
+ List *ele;
+
+ foreach(ele, l)
+ {
+ if (IsA(lfirst(ele), TypeName))
+ transformTypeRef(pstate, (TypeName *) lfirst(ele));
+ }
+}
+
+/*
+ * Transform a TypeName to not use %TYPE.
+ */
+static void
+transformTypeRef(ParseState *pstate, TypeName *tn)
+{
+ Attr *att;
+ Node *n;
+ Var *v;
+
+ if (tn->attrname == NULL)
+ return;
+ att = makeAttr(tn->name, tn->attrname);
+ n = transformExpr(pstate, (Node *) att, EXPR_COLUMN_FIRST);
+ if (! IsA(n, Var))
+ elog(ERROR, "unsupported expression in %%TYPE");
+ v = (Var *) n;
+ tn->name = typeidTypeName(v->vartype);
+ tn->typmod = v->vartypmod;
+ tn->attrname = NULL;
}

/* exported so planner can check again after rewriting, query pullup, etc */
Index: src/backend/parser/gram.y
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.221
diff -p -u -r2.221 gram.y
--- src/backend/parser/gram.y 2001/02/18 18:06:10 2.221
+++ src/backend/parser/gram.y 2001/04/28 03:38:26
@@ -192,7 +192,7 @@ static void doNegateFloat(Value *v);
def_list, opt_indirection, group_clause, TriggerFuncArgs,
select_limit, opt_select_limit

-%type <typnam> func_arg, func_return, aggr_argtype
+%type <typnam> func_arg, func_return, func_type, aggr_argtype

%type <boolean> opt_arg, TriggerForOpt, TriggerForType, OptTemp

@@ -2462,7 +2462,7 @@ func_args_list: func_arg
{ $$ = lappend($1, $3); }
;

-func_arg: opt_arg Typename
+func_arg: opt_arg func_type
{
/* We can catch over-specified arguments here if we want to,
* but for now better to silently swallow typmod, etc.
@@ -2470,7 +2470,7 @@ func_arg: opt_arg Typename
*/
$$ = $2;
}
- | Typename
+ | func_type
{
$$ = $1;
}
@@ -2498,7 +2498,7 @@ func_as: Sconst
{ $$ = makeList2(makeString($1), makeString($3)); }
;

-func_return: Typename
+func_return: func_type
{
/* We can catch over-specified arguments here if we want to,
* but for now better to silently swallow typmod, etc.
@@ -2508,6 +2508,18 @@ func_return: Typename
}
;

+func_type: Typename
+ {
+ $$ = $1;
+ }
+ | IDENT '.' ColId '%' TYPE_P
+ {
+ $$ = makeNode(TypeName);
+ $$->name = $1;
+ $$->typmod = -1;
+ $$->attrname = $3;
+ }
+ ;

/*****************************************************************************
*
Index: src/backend/parser/parse_expr.c
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/parser/parse_expr.c,v
retrieving revision 1.92
diff -p -u -r1.92 parse_expr.c
--- src/backend/parser/parse_expr.c 2001/03/22 03:59:41 1.92
+++ src/backend/parser/parse_expr.c 2001/04/28 03:38:26
@@ -939,6 +939,7 @@ parser_typecast_expression(ParseState *p
char *
TypeNameToInternalName(TypeName *typename)
{
+ Assert(typename->attrname == NULL);
if (typename->arrayBounds != NIL)
{

Index: src/test/regress/input/create_function_2.source
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/input/create_function_2.source,v
retrieving revision 1.12
diff -p -u -r1.12 create_function_2.source
--- src/test/regress/input/create_function_2.source 2000/11/20 20:36:54 1.12
+++ src/test/regress/input/create_function_2.source 2001/04/28 03:38:27
@@ -13,6 +13,12 @@ CREATE FUNCTION hobby_construct(text, te
LANGUAGE 'sql';


+CREATE FUNCTION hobbies_by_name(hobbies_r.name%TYPE)
+ RETURNS hobbies_r.person%TYPE
+ AS 'select person from hobbies_r where name = $1'
+ LANGUAGE 'sql';
+
+
CREATE FUNCTION equipment(hobbies_r)
RETURNS setof equipment_r
AS 'select * from equipment_r where hobby = $1.name'
Index: src/test/regress/input/misc.source
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/input/misc.source,v
retrieving revision 1.14
diff -p -u -r1.14 misc.source
--- src/test/regress/input/misc.source 2000/11/20 20:36:54 1.14
+++ src/test/regress/input/misc.source 2001/04/28 03:38:28
@@ -214,6 +214,7 @@ SELECT user_relns() AS user_relns

--SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer'))) AS equip_name;

+SELECT hobbies_by_name('basketball');

--
-- check that old-style C functions work properly with TOASTed values
Index: src/test/regress/output/create_function_2.source
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/output/create_function_2.source,v
retrieving revision 1.13
diff -p -u -r1.13 create_function_2.source
--- src/test/regress/output/create_function_2.source 2000/11/20 20:36:54 1.13
+++ src/test/regress/output/create_function_2.source 2001/04/28 03:38:28
@@ -9,6 +9,10 @@ CREATE FUNCTION hobby_construct(text, te
RETURNS hobbies_r
AS 'select $1 as name, $2 as hobby'
LANGUAGE 'sql';
+CREATE FUNCTION hobbies_by_name(hobbies_r.name%TYPE)
+ RETURNS hobbies_r.person%TYPE
+ AS 'select person from hobbies_r where name = $1'
+ LANGUAGE 'sql';
CREATE FUNCTION equipment(hobbies_r)
RETURNS setof equipment_r
AS 'select * from equipment_r where hobby = $1.name'
Index: src/test/regress/output/misc.source
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/output/misc.source,v
retrieving revision 1.27
diff -p -u -r1.27 misc.source
--- src/test/regress/output/misc.source 2000/11/20 20:36:54 1.27
+++ src/test/regress/output/misc.source 2001/04/28 03:38:28
@@ -656,6 +656,12 @@ SELECT user_relns() AS user_relns
(90 rows)

--SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer'))) AS equip_name;
+SELECT hobbies_by_name('basketball');
+ hobbies_by_name
+-----------------
+ joe
+(1 row)
+
--
-- check that old-style C functions work properly with TOASTed values
--
Index: doc/src/sgml/ref/create_function.sgml
===================================================================
RCS file: /home/projects/pgsql/cvsroot/pgsql/doc/src/sgml/ref/create_function.sgml,v
retrieving revision 1.21
diff -p -u -r1.21 create_function.sgml
--- doc/src/sgml/ref/create_function.sgml 2000/12/25 23:15:26 1.21
+++ doc/src/sgml/ref/create_function.sgml 2001/04/28 03:38:31
@@ -58,10 +58,16 @@ CREATE FUNCTION <replaceable class="para
<listitem>
<para>
The data type(s) of the function's arguments, if any.
- The input types may be base or complex types, or
- <firstterm>opaque</firstterm>.
+ The input types may be base or complex types,
+ <firstterm>opaque</firstterm>, or the same as the type of an
+ existing column.
<literal>Opaque</literal> indicates that the function
accepts arguments of a non-SQL type such as <type>char *</type>.
+ The type of a column is indicated using <replaceable
+ class="parameter">tablename</replaceable>.<replaceable
+ class="parameter">columnname</replaceable><literal>%TYPE</literal>;
+ using this can sometimes help make a function independent from
+ changes to the definition of a table.
</para>
</listitem>
</varlistentry>
@@ -72,7 +78,8 @@ CREATE FUNCTION <replaceable class="para
The return data type.
The output type may be specified as a base type, complex type,
<option>setof type</option>,
- or <option>opaque</option>.
+ <option>opaque</option>, or the same as the type of an
+ existing column.
The <option>setof</option>
modifier indicates that the function will return a set of items,
rather than a single item.

On Fri, Apr 27, 2001 at 08:45:25PM -0700, Ian Lance Taylor wrote:
> This patch adds support for %TYPE in CREATE FUNCTION argument and
> return types.
>
> %TYPE is already supported by PL/pgSQL when declaring variables.
> However, that does not help with the argument and return types in
> CREATE FUNCTION.
>
> Using %TYPE makes it easier to write a function which is independent
> of the definition of a table. That is, minor changes to the types
> used in the table may not require changes to the function.

Wow! This would be _very_ useful! It's something I wish PostgreSQL
had and I miss it everytime I write functions and remember PL/SQL.

Thanks a lot Ian, I hope this one makes it in (hopefully for 7.1.1)

-Roberto
--
+----| http://fslc.usu.edu USU Free Software & GNU/Linux Club |------+
Roberto Mello - Computer Science, USU - http://www.brasileiro.net
http://www.sdl.usu.edu - Space Dynamics Lab, Developer
-----*'. (Explosive Tagline)

> > Using %TYPE makes it easier to write a function which is independent
> > of the definition of a table. That is, minor changes to the types
> > used in the table may not require changes to the function.
>
> Wow! This would be _very_ useful! It's something I wish PostgreSQL
> had and I miss it everytime I write functions and remember PL/SQL.
>
> Thanks a lot Ian, I hope this one makes it in (hopefully for 7.1.1)

Sorry, only in 7.2. No new features in minor releases unless they are
very safe.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

On Sat, Apr 28, 2001 at 06:45:39PM -0400, Bruce Momjian wrote:
>
> Sorry, only in 7.2. No new features in minor releases unless they are
> very safe.

So how was that patch not safe?
It sure would make porting Oracle apps to PostgreSQL _much_ easier.
How far down the line is 7.2 (my guess is a few months away at least)?
Is there a doc with what's planned for 7.2 somewhere? I know Jan Wieck
mentioned improvements in the procedural languages.

-Roberto
--
+----| http://fslc.usu.edu USU Free Software & GNU/Linux Club |------+
Roberto Mello - Computer Science, USU - http://www.brasileiro.net
http://www.sdl.usu.edu - Space Dynamics Lab, Developer
Keyboard not connected, press F1 to continue.

> On Sat, Apr 28, 2001 at 06:45:39PM -0400, Bruce Momjian wrote:
> >
> > Sorry, only in 7.2. No new features in minor releases unless they are
> > very safe.
>
> So how was that patch not safe?
> It sure would make porting Oracle apps to PostgreSQL _much_ easier.
> How far down the line is 7.2 (my guess is a few months away at least)?
> Is there a doc with what's planned for 7.2 somewhere? I know Jan Wieck
> mentioned improvements in the procedural languages.

The TODO list has a list of things we think need doing. There is an
Urgent section that I hope we can focus on for 7.2. We can't promise
what will be in 7.2 because we don't know what people will volunteer to
work on. I would guess 7.2 is 4-6 months away, at least.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Sorry, looks like this patch has to be rejected because it can not
handle table changes.

> This patch adds support for %TYPE in CREATE FUNCTION argument and
> return types.
>
> %TYPE is already supported by PL/pgSQL when declaring variables.
> However, that does not help with the argument and return types in
> CREATE FUNCTION.
>
> Using %TYPE makes it easier to write a function which is independent
> of the definition of a table. That is, minor changes to the types
> used in the table may not require changes to the function.
>
> For example, this trivial function will work whenever `table' which
> has columns named `name' and `value', no matter what the types of the
> columns are.
>
> CREATE FUNCTION lookup (table.name%TYPE)
> RETURNS table.value%TYPE
> AS 'select value from table where name = $1'
> LANGUAGE 'sql';
>
> This patch includes changes to the testsuite and the documentation.
>
> This work was sponsored by Zembu.
>
> Ian
>
> Index: src/include/nodes/parsenodes.h
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
> retrieving revision 1.126
> diff -p -u -r1.126 parsenodes.h
> --- src/include/nodes/parsenodes.h 2001/03/23 04:49:56 1.126
> +++ src/include/nodes/parsenodes.h 2001/04/28 03:38:21
> @@ -945,6 +945,7 @@ typedef struct TypeName
> bool setof; /* is a set? */
> int32 typmod; /* type modifier */
> List *arrayBounds; /* array bounds */
> + char *attrname; /* field name when using %TYPE */
> } TypeName;
>
> /*
> Index: src/backend/parser/analyze.c
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/parser/analyze.c,v
> retrieving revision 1.183
> diff -p -u -r1.183 analyze.c
> --- src/backend/parser/analyze.c 2001/03/22 06:16:15 1.183
> +++ src/backend/parser/analyze.c 2001/04/28 03:38:23
> @@ -27,6 +27,7 @@
> #include "parser/parse_relation.h"
> #include "parser/parse_target.h"
> #include "parser/parse_type.h"
> +#include "parser/parse_expr.h"
> #include "rewrite/rewriteManip.h"
> #include "utils/builtins.h"
> #include "utils/fmgroids.h"
> @@ -49,7 +50,10 @@ static Node *transformSetOperationTree(P
> static Query *transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt);
> static Query *transformCreateStmt(ParseState *pstate, CreateStmt *stmt);
> static Query *transformAlterTableStmt(ParseState *pstate, AlterTableStmt *stmt);
> +static Node *transformTypeRefs(ParseState *pstate, Node *stmt);
>
> +static void transformTypeRefsList(ParseState *pstate, List *l);
> +static void transformTypeRef(ParseState *pstate, TypeName *tn);
> static List *getSetColTypes(ParseState *pstate, Node *node);
> static void transformForUpdate(Query *qry, List *forUpdate);
> static void transformFkeyGetPrimaryKey(FkConstraint *fkconstraint);
> @@ -230,6 +234,18 @@ transformStmt(ParseState *pstate, Node *
> (SelectStmt *) parseTree);
> break;
>
> + /*
> + * Convert use of %TYPE in statements where it is permitted.
> + */
> + case T_ProcedureStmt:
> + case T_CommentStmt:
> + case T_RemoveFuncStmt:
> + case T_DefineStmt:
> + result = makeNode(Query);
> + result->commandType = CMD_UTILITY;
> + result->utilityStmt = transformTypeRefs(pstate, parseTree);
> + break;
> +
> default:
>
> /*
> @@ -2607,6 +2623,104 @@ transformAlterTableStmt(ParseState *psta
> }
> qry->utilityStmt = (Node *) stmt;
> return qry;
> +}
> +
> +/*
> + * Transform uses of %TYPE in a statement.
> + */
> +static Node *
> +transformTypeRefs(ParseState *pstate, Node *stmt)
> +{
> + switch (nodeTag(stmt))
> + {
> + case T_ProcedureStmt:
> + {
> + ProcedureStmt *ps = (ProcedureStmt *) stmt;
> +
> + transformTypeRefsList(pstate, ps->argTypes);
> + transformTypeRef(pstate, (TypeName *) ps->returnType);
> + transformTypeRefsList(pstate, ps->withClause);
> + }
> + break;
> +
> + case T_CommentStmt:
> + {
> + CommentStmt *cs = (CommentStmt *) stmt;
> +
> + transformTypeRefsList(pstate, cs->objlist);
> + }
> + break;
> +
> + case T_RemoveFuncStmt:
> + {
> + RemoveFuncStmt *rs = (RemoveFuncStmt *) stmt;
> +
> + transformTypeRefsList(pstate, rs->args);
> + }
> + break;
> +
> + case T_DefineStmt:
> + {
> + DefineStmt *ds = (DefineStmt *) stmt;
> + List *ele;
> +
> + foreach(ele, ds->definition)
> + {
> + DefElem *de = (DefElem *) lfirst(ele);
> +
> + if (de->arg != NULL
> + && IsA(de->arg, TypeName))
> + {
> + transformTypeRef(pstate, (TypeName *) de->arg);
> + }
> + }
> + }
> + break;
> +
> + default:
> + elog(ERROR, "Unsupported type %d in transformTypeRefs",
> + nodeTag(stmt));
> + break;
> + }
> +
> + return stmt;
> +}
> +
> +/*
> + * Transform uses of %TYPE in a list.
> + */
> +static void
> +transformTypeRefsList(ParseState *pstate, List *l)
> +{
> + List *ele;
> +
> + foreach(ele, l)
> + {
> + if (IsA(lfirst(ele), TypeName))
> + transformTypeRef(pstate, (TypeName *) lfirst(ele));
> + }
> +}
> +
> +/*
> + * Transform a TypeName to not use %TYPE.
> + */
> +static void
> +transformTypeRef(ParseState *pstate, TypeName *tn)
> +{
> + Attr *att;
> + Node *n;
> + Var *v;
> +
> + if (tn->attrname == NULL)
> + return;
> + att = makeAttr(tn->name, tn->attrname);
> + n = transformExpr(pstate, (Node *) att, EXPR_COLUMN_FIRST);
> + if (! IsA(n, Var))
> + elog(ERROR, "unsupported expression in %%TYPE");
> + v = (Var *) n;
> + tn->name = typeidTypeName(v->vartype);
> + tn->typmod = v->vartypmod;
> + tn->attrname = NULL;
> }
>
> /* exported so planner can check again after rewriting, query pullup, etc */
> Index: src/backend/parser/gram.y
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/parser/gram.y,v
> retrieving revision 2.221
> diff -p -u -r2.221 gram.y
> --- src/backend/parser/gram.y 2001/02/18 18:06:10 2.221
> +++ src/backend/parser/gram.y 2001/04/28 03:38:26
> @@ -192,7 +192,7 @@ static void doNegateFloat(Value *v);
> def_list, opt_indirection, group_clause, TriggerFuncArgs,
> select_limit, opt_select_limit
>
> -%type <typnam> func_arg, func_return, aggr_argtype
> +%type <typnam> func_arg, func_return, func_type, aggr_argtype
>
> %type <boolean> opt_arg, TriggerForOpt, TriggerForType, OptTemp
>
> @@ -2462,7 +2462,7 @@ func_args_list: func_arg
> { $$ = lappend($1, $3); }
> ;
>
> -func_arg: opt_arg Typename
> +func_arg: opt_arg func_type
> {
> /* We can catch over-specified arguments here if we want to,
> * but for now better to silently swallow typmod, etc.
> @@ -2470,7 +2470,7 @@ func_arg: opt_arg Typename
> */
> $$ = $2;
> }
> - | Typename
> + | func_type
> {
> $$ = $1;
> }
> @@ -2498,7 +2498,7 @@ func_as: Sconst
> { $$ = makeList2(makeString($1), makeString($3)); }
> ;
>
> -func_return: Typename
> +func_return: func_type
> {
> /* We can catch over-specified arguments here if we want to,
> * but for now better to silently swallow typmod, etc.
> @@ -2508,6 +2508,18 @@ func_return: Typename
> }
> ;
>
> +func_type: Typename
> + {
> + $$ = $1;
> + }
> + | IDENT '.' ColId '%' TYPE_P
> + {
> + $$ = makeNode(TypeName);
> + $$->name = $1;
> + $$->typmod = -1;
> + $$->attrname = $3;
> + }
> + ;
>
> /*****************************************************************************
> *
> Index: src/backend/parser/parse_expr.c
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/backend/parser/parse_expr.c,v
> retrieving revision 1.92
> diff -p -u -r1.92 parse_expr.c
> --- src/backend/parser/parse_expr.c 2001/03/22 03:59:41 1.92
> +++ src/backend/parser/parse_expr.c 2001/04/28 03:38:26
> @@ -939,6 +939,7 @@ parser_typecast_expression(ParseState *p
> char *
> TypeNameToInternalName(TypeName *typename)
> {
> + Assert(typename->attrname == NULL);
> if (typename->arrayBounds != NIL)
> {
>
> Index: src/test/regress/input/create_function_2.source
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/input/create_function_2.source,v
> retrieving revision 1.12
> diff -p -u -r1.12 create_function_2.source
> --- src/test/regress/input/create_function_2.source 2000/11/20 20:36:54 1.12
> +++ src/test/regress/input/create_function_2.source 2001/04/28 03:38:27
> @@ -13,6 +13,12 @@ CREATE FUNCTION hobby_construct(text, te
> LANGUAGE 'sql';
>
>
> +CREATE FUNCTION hobbies_by_name(hobbies_r.name%TYPE)
> + RETURNS hobbies_r.person%TYPE
> + AS 'select person from hobbies_r where name = $1'
> + LANGUAGE 'sql';
> +
> +
> CREATE FUNCTION equipment(hobbies_r)
> RETURNS setof equipment_r
> AS 'select * from equipment_r where hobby = $1.name'
> Index: src/test/regress/input/misc.source
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/input/misc.source,v
> retrieving revision 1.14
> diff -p -u -r1.14 misc.source
> --- src/test/regress/input/misc.source 2000/11/20 20:36:54 1.14
> +++ src/test/regress/input/misc.source 2001/04/28 03:38:28
> @@ -214,6 +214,7 @@ SELECT user_relns() AS user_relns
>
> --SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer'))) AS equip_name;
>
> +SELECT hobbies_by_name('basketball');
>
> --
> -- check that old-style C functions work properly with TOASTed values
> Index: src/test/regress/output/create_function_2.source
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/output/create_function_2.source,v
> retrieving revision 1.13
> diff -p -u -r1.13 create_function_2.source
> --- src/test/regress/output/create_function_2.source 2000/11/20 20:36:54 1.13
> +++ src/test/regress/output/create_function_2.source 2001/04/28 03:38:28
> @@ -9,6 +9,10 @@ CREATE FUNCTION hobby_construct(text, te
> RETURNS hobbies_r
> AS 'select $1 as name, $2 as hobby'
> LANGUAGE 'sql';
> +CREATE FUNCTION hobbies_by_name(hobbies_r.name%TYPE)
> + RETURNS hobbies_r.person%TYPE
> + AS 'select person from hobbies_r where name = $1'
> + LANGUAGE 'sql';
> CREATE FUNCTION equipment(hobbies_r)
> RETURNS setof equipment_r
> AS 'select * from equipment_r where hobby = $1.name'
> Index: src/test/regress/output/misc.source
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/src/test/regress/output/misc.source,v
> retrieving revision 1.27
> diff -p -u -r1.27 misc.source
> --- src/test/regress/output/misc.source 2000/11/20 20:36:54 1.27
> +++ src/test/regress/output/misc.source 2001/04/28 03:38:28
> @@ -656,6 +656,12 @@ SELECT user_relns() AS user_relns
> (90 rows)
>
> --SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer'))) AS equip_name;
> +SELECT hobbies_by_name('basketball');
> + hobbies_by_name
> +-----------------
> + joe
> +(1 row)
> +
> --
> -- check that old-style C functions work properly with TOASTed values
> --
> Index: doc/src/sgml/ref/create_function.sgml
> ===================================================================
> RCS file: /home/projects/pgsql/cvsroot/pgsql/doc/src/sgml/ref/create_function.sgml,v
> retrieving revision 1.21
> diff -p -u -r1.21 create_function.sgml
> --- doc/src/sgml/ref/create_function.sgml 2000/12/25 23:15:26 1.21
> +++ doc/src/sgml/ref/create_function.sgml 2001/04/28 03:38:31
> @@ -58,10 +58,16 @@ CREATE FUNCTION <replaceable class="para
> <listitem>
> <para>
> The data type(s) of the function's arguments, if any.
> - The input types may be base or complex types, or
> - <firstterm>opaque</firstterm>.
> + The input types may be base or complex types,
> + <firstterm>opaque</firstterm>, or the same as the type of an
> + existing column.
> <literal>Opaque</literal> indicates that the function
> accepts arguments of a non-SQL type such as <type>char *</type>.
> + The type of a column is indicated using <replaceable
> + class="parameter">tablename</replaceable>.<replaceable
> + class="parameter">columnname</replaceable><literal>%TYPE</literal>;
> + using this can sometimes help make a function independent from
> + changes to the definition of a table.
> </para>
> </listitem>
> </varlistentry>
> @@ -72,7 +78,8 @@ CREATE FUNCTION <replaceable class="para
> The return data type.
> The output type may be specified as a base type, complex type,
> <option>setof type</option>,
> - or <option>opaque</option>.
> + <option>opaque</option>, or the same as the type of an
> + existing column.
> The <option>setof</option>
> modifier indicates that the function will return a set of items,
> rather than a single item.
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://www.postgresql.org/search.mpl
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:

> Sorry, looks like this patch has to be rejected because it can not
> handle table changes.

> > This patch adds support for %TYPE in CREATE FUNCTION argument and
> > return types.

Does anybody want to suggest how to handle table changes? Does
anybody want to work with me to make this patch acceptable? Or is
this functionality of no interest to the Postgres development team?

Ian

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>
> > Sorry, looks like this patch has to be rejected because it can not
> > handle table changes.
>
> > > This patch adds support for %TYPE in CREATE FUNCTION argument and
> > > return types.
>
> Does anybody want to suggest how to handle table changes? Does
> anybody want to work with me to make this patch acceptable? Or is
> this functionality of no interest to the Postgres development team?

I think the major problem was that our pg_proc table doesn't have any
way of handling arg changes. In fact, we need a ALTER FUNCTION
capability first so we can recreate functions in place with the same
OID. We may then be able to recreate the function on table change, but
I think we will need this TODO item done also:

* Add pg_depend table to track object dependencies

So it seems we need two items done first, then we would have the tools
to properly implement this functionality.

So, yes, the functionality is desired, but it has to be done with the
proper groundwork already in place.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

At 03:47 PM 5/28/01 -0700, Ian Lance Taylor wrote:
>Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>
>> Sorry, looks like this patch has to be rejected because it can not
>> handle table changes.
>
>> > This patch adds support for %TYPE in CREATE FUNCTION argument and
>> > return types.
>
>Does anybody want to suggest how to handle table changes? Does
>anybody want to work with me to make this patch acceptable? Or is
>this functionality of no interest to the Postgres development team?

I don't know about the Postgres development team, but it is of great
interest to the OpenACS project team. We've got hundreds or perhaps
thousands of PL/SQL procs and funcs in our code base that use this
notation and it would be very, very nice if we could use this construct
in our PostgreSQL code base.

I suspect any organization or project attempting to either migrate
from Oracle to Postgres or trying to support both databases (as we
do at OpenACS) will find this very useful.

We're deep in the midst of our rewrite of the Ars Digita code base that
we've inherited so don't have any resources to offer to help solve the
problem.

But we can offer encouragement and appreciation!

- Don Baccus, Portland OR <dhogaza(at)pacifier(dot)com>
Nature photos, on-line guides, Pacific Northwest
Rare Bird Alert Service and other goodies at
http://donb.photo.net.

Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> I think the major problem was that our pg_proc table doesn't have any
> way of handling arg changes. In fact, we need a ALTER FUNCTION
> capability first so we can recreate functions in place with the same
> OID.

Actually that's the least of the issues. The real problem is that
because of function overloading, myfunc(int4) and myfunc(int2) (for
example) are considered completely different functions. It is thus
not at all clear what should happen if I create myfunc(foo.f1%TYPE)
and later alter the type of foo.f1 from int4 to int2. Does myfunc(int4)
stop existing? What if a conflicting myfunc(int2) already exists?
What happens to type-specific references to myfunc(int4) --- for
example, what if it's used as the implementation function for an
operator declared on int4?

Worrying about implementation issues is premature when you haven't
got an adequate definition.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > I think the major problem was that our pg_proc table doesn't have any
> > way of handling arg changes. In fact, we need a ALTER FUNCTION
> > capability first so we can recreate functions in place with the same
> > OID.
>
> Actually that's the least of the issues. The real problem is that
> because of function overloading, myfunc(int4) and myfunc(int2) (for
> example) are considered completely different functions. It is thus
> not at all clear what should happen if I create myfunc(foo.f1%TYPE)
> and later alter the type of foo.f1 from int4 to int2. Does myfunc(int4)
> stop existing? What if a conflicting myfunc(int2) already exists?
> What happens to type-specific references to myfunc(int4) --- for
> example, what if it's used as the implementation function for an
> operator declared on int4?
>
> Worrying about implementation issues is premature when you haven't
> got an adequate definition.

It's pretty easy to define what to do in each of the cases you
describe. The options are: 1) leave the function unchanged; 2) alter
the function to use the new type; 3) define a copy of the function
with the new type. In cases 2 or 3 you have to consider whether there
is already a function with the new type; if there is, you have to
either: 23a) replace the new function; 23b) issue a NOTICE; 23c) issue
a NOTICE and drop the old function. In case 2 you also have to
consider whether something is using the old function; if there is, you
have to 2a) leave the old function there; 2b) issue a NOTICE while
dropping the old function.

I propose this: if a table definition changes, alter the function to
use the new type (choice 2). If there is already a function with the
new type, issue a NOTICE and drop the old function (choice 23b). If
something is using the old function, issue a NOTICE while dropping the
old function (choice 2b).

Of course, this is made much easier if there is a pg_depends table
which accurately records dependencies.

I have a meta-point: the choices to be made here are not all that
interesting. They do have to be defined. But almost any definition
is OK. Users are not going to routinely redefine tables with attached
functions; when they do, they must be prepared to consider the
consequences. If anybody thinks that different choices should be made
in this case, that is certainly fine with me.

If you agree with me on the meta-point, then this is just a quibble
about my original patch (which made choice 1 above). If you disagree
with me, I'd like to understand why.

Ian

> Of course, this is made much easier if there is a pg_depends table
> which accurately records dependencies.

Yes, that was a nifty idea.

> I have a meta-point: the choices to be made here are not all that
> interesting. They do have to be defined. But almost any definition
> is OK. Users are not going to routinely redefine tables with attached
> functions; when they do, they must be prepared to consider the
> consequences. If anybody thinks that different choices should be made
> in this case, that is certainly fine with me.
>
> If you agree with me on the meta-point, then this is just a quibble
> about my original patch (which made choice 1 above). If you disagree
> with me, I'd like to understand why.

I agree that having problems when a table is defined is acceptable. It
is not like someone is _forced_ to use the feature.

So far that is three or four people who like the feature, and I have
only heard one opposed.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

Ian Lance Taylor <ian(at)airs(dot)com> writes:
> I have a meta-point: the choices to be made here are not all that
> interesting. They do have to be defined. But almost any definition
> is OK.

Well, that implicit assumption is exactly the one I was questioning;
*is* it OK not to be very concerned about what the behavior is? ISTM
that how the system handles these cases will constrain the use of the
%TYPE feature into certain pathways. The limitations arising from your
original patch presumably don't matter for your intended use, but they
may nonetheless be surprising for people who try to use it differently.
(We've seen cases before where someone does a quick-and-dirty feature
addition that fails to act as other people expect it to.)

I wanted to see a clear understanding of what the corner-case behavior
is, and a consensus that that behavior is acceptable all 'round. If
the quick-and-dirty route will be satisfactory over the long run, fine;
but I don't much want to install a new feature that is immediately going
to draw bug reports/upgrade requests/whatever you want to call 'em.

regards, tom lane

At 12:30 PM 5/30/01 -0400, Tom Lane wrote:
>Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
>> I think the major problem was that our pg_proc table doesn't have any
>> way of handling arg changes. In fact, we need a ALTER FUNCTION
>> capability first so we can recreate functions in place with the same
>> OID.
>
>Actually that's the least of the issues. The real problem is that
>because of function overloading, myfunc(int4) and myfunc(int2) (for
>example) are considered completely different functions. It is thus
>not at all clear what should happen if I create myfunc(foo.f1%TYPE)
>and later alter the type of foo.f1 from int4 to int2. Does myfunc(int4)
>stop existing?

What happens now with PL/pgSQL variables? Does it continue to point
int4 as long as the backend stays alive, but switch in new backends
as they come to life, the function gets called, and the body recompiled?

(Compiled bytes are stored on a per-backend basis, right? Or wrong? :)

That's not particularly relevant to the parameter case other than to
point out that we may already have some weirdness in PL/pgSQL in
this regard.

- Don Baccus, Portland OR <dhogaza(at)pacifier(dot)com>
Nature photos, on-line guides, Pacific Northwest
Rare Bird Alert Service and other goodies at
http://donb.photo.net.

Don Baccus <dhogaza(at)pacifier(dot)com> writes:

> At 12:30 PM 5/30/01 -0400, Tom Lane wrote:
> >Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> >> I think the major problem was that our pg_proc table doesn't have any
> >> way of handling arg changes. In fact, we need a ALTER FUNCTION
> >> capability first so we can recreate functions in place with the same
> >> OID.
> >
> >Actually that's the least of the issues. The real problem is that
> >because of function overloading, myfunc(int4) and myfunc(int2) (for
> >example) are considered completely different functions. It is thus
> >not at all clear what should happen if I create myfunc(foo.f1%TYPE)
> >and later alter the type of foo.f1 from int4 to int2. Does myfunc(int4)
> >stop existing?
>
> What happens now with PL/pgSQL variables? Does it continue to point
> int4 as long as the backend stays alive, but switch in new backends
> as they come to life, the function gets called, and the body recompiled?
>
> (Compiled bytes are stored on a per-backend basis, right? Or wrong? :)
>
> That's not particularly relevant to the parameter case other than to
> point out that we may already have some weirdness in PL/pgSQL in
> this regard.

I assume you mean: what happens now with a PL/pgSQL variable which is
declared using table.row%TYPE?

As you suspect, the answer is that any existing backend which has
already compiled the function will continue to use the old
definition. Any new backend will recompile the function and get the
new definition.

As far as I can see in a quick look, there is currently no interface
to direct PL/pgSQL that it must reparse a function. And there is no
way for PL/pgSQL to register interest in table changes.

Ian

Tom Lane wrote:
> Ian Lance Taylor <ian(at)airs(dot)com> writes:
> > I have a meta-point: the choices to be made here are not all that
> > interesting. They do have to be defined. But almost any definition
> > is OK.
>
> Well, that implicit assumption is exactly the one I was questioning;
> *is* it OK not to be very concerned about what the behavior is? ISTM
> that how the system handles these cases will constrain the use of the
> %TYPE feature into certain pathways. The limitations arising from your
> original patch presumably don't matter for your intended use, but they
> may nonetheless be surprising for people who try to use it differently.
> (We've seen cases before where someone does a quick-and-dirty feature
> addition that fails to act as other people expect it to.)

IMHO the possible confusion added by supporting %TYPE in our
utility statements is too high a risk.

What most of those if favor for doing it right now want is an
easy Oracle->PostgreSQL one-time porting path. Reasonable,
but solveable with some external preprocessor/script too.

I see that the currently discussed implementation add's more
Oracle incompatibility than compatibility. This is because
there are different times between the interpretation of %TYPE
inside and out of a procedures body. Inside the PL/pgSQL
declarations, it's parsed at each first call of a function
per session, so there is at least some chance that changes
propagate up (at reconnect time).

But used in the utility statement to specify arguments,
column types and the like they are interpreted just once and
stored as that in our catalog. We don't remember the
original CREATE statement, that created it. So even if we
remember that this thing once depended on another, we don't
know what to do if that other is altered.

Thus, usage of %TYPE inside of a PL/pgSQL function is OK,
because it behaves more or less like expected - at least
after reconnecting. Using it outside IMHO isn't, because the
type reference cannot be stored as that, but has to be
resolved once and forever with possible code breakage if the
referenced objects type changes. The kind of breakage could
be extremely tricky and the code might appear to work but
does the wrong thing internally (think about changing a
column from DOUBLE to NUMERIC and assuming that everything
working with this column is doing exact precision from now on
- it might NOT).

A "No" from here.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:

> What most of those if favor for doing it right now want is an
> easy Oracle->PostgreSQL one-time porting path. Reasonable,
> but solveable with some external preprocessor/script too.

Can you explain how an external preprocessor/script addresses the
issue of %TYPE in a function definition? Presumably the preprocessor
has to translate %TYPE into some definite type when it creates the
function. But how can a preprocessor address the issue of what to do
when the table definition changes? There still has to be an entry in
pg_proc for the procedure. What happens to that entry when the table
changes?

You seem to be saying that %TYPE can be implemented via some other
mechanism. That is fine with me, but how would that other mechanism
work? Why it would not raise the exact same set of issues?

Ian

Ian Lance Taylor wrote:
> [...]
> I propose this: if a table definition changes, alter the function to
> use the new type (choice 2). If there is already a function with the
> new type, issue a NOTICE and drop the old function (choice 23b). If
> something is using the old function, issue a NOTICE while dropping the
> old function (choice 2b).

Altering a function definition in any language other than
PL/pgSQL really scares me. What do you expect a "C" function
declared to take a VARCHAR argument to do if you just change
the pg_proc entry telling it now takes a NAME? I'd expect it
to generate a signal 11 most of it's calls, and nothing
really useful the other times.

And you have no chance of limiting your implementation to
functions defined in PL/pgSQL. It's a loadable PL so you
don't even know the languages or handlers Oid at compile
time.

> If you agree with me on the meta-point, then this is just a quibble
> about my original patch (which made choice 1 above). If you disagree
> with me, I'd like to understand why.

The possible SIGSEGV above. Please don't take it personally,
I'm talking tech here, but it seems you forgot that PL/pgSQL
is just *one* of many possible languages.

And please forget about a chance to finally track all
dependencies. You'll never be able to know if some PL/Tcl or
PL/Python function/trigger uses that function. So not getting
your NOTICE doesn't tell if really nothing broke. As soon as
you tell me you can I'd implement PL/Forth or PL/Pascal -
maybe PL/COBOL or PL/RPL (using an embedded HP48 emulator)
just to tell "you can't" again :-)

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:

> Altering a function definition in any language other than
> PL/pgSQL really scares me. What do you expect a "C" function
> declared to take a VARCHAR argument to do if you just change
> the pg_proc entry telling it now takes a NAME? I'd expect it
> to generate a signal 11 most of it's calls, and nothing
> really useful the other times.

Good point.

That brings me back to choice 1 in my original message: don't try to
change the function if the table definition changes.

In fact, it's possible to do better. A procedural language could
define a hook to handle table definition changes. The Postgres
backend could define a way to register to receive notification of
table definition changes (this would essentially be an entry in a
table like the proposed pg_depends). The procedural language itself
could then handle the table changes by redefining the function or
whatever.

When defining a function using %TYPE, the procedural language would be
notified that %TYPE was used. It could then record a dependency, if
it was prepared to handle one.

This would permit PL/pgSQL to redefine the function defined using
%TYPE if that seems desirable. It would also permit PL/pgSQL to
behave more reasonably with regard to variables defined using %TYPE.

This would also permit the C function handler to issue a NOTICE when a
C function was defined using %TYPE and the table definition was
changed.

> > If you agree with me on the meta-point, then this is just a quibble
> > about my original patch (which made choice 1 above). If you disagree
> > with me, I'd like to understand why.
>
> The possible SIGSEGV above. Please don't take it personally,
> I'm talking tech here, but it seems you forgot that PL/pgSQL
> is just *one* of many possible languages.

Actually, I don't see this as a disagreement about my meta-point.
Users who use %TYPE must watch out if they change a table definition.
A SIGSEGV is just an extreme case.

> And please forget about a chance to finally track all
> dependencies. You'll never be able to know if some PL/Tcl or
> PL/Python function/trigger uses that function. So not getting
> your NOTICE doesn't tell if really nothing broke. As soon as
> you tell me you can I'd implement PL/Forth or PL/Pascal -
> maybe PL/COBOL or PL/RPL (using an embedded HP48 emulator)
> just to tell "you can't" again :-)

I don't entirely understand this. I can break the system just as
easily using DROP FUNCTION. At some point, I think the programmer has
to take responsibility.

I return to the question of whether the Postgres development team is
interested in support for %TYPE. If the team is not interested, then
I'm wasting my time. I'm seeing a no from you and Tom Lane, and a
maybe from Bruce Momjian.

Ian

Ian Lance Taylor wrote:
> Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:
>
> > What most of those if favor for doing it right now want is an
> > easy Oracle->PostgreSQL one-time porting path. Reasonable,
> > but solveable with some external preprocessor/script too.
>
> Can you explain how an external preprocessor/script addresses the
> issue of %TYPE in a function definition? Presumably the preprocessor
> has to translate %TYPE into some definite type when it creates the
> function. But how can a preprocessor address the issue of what to do
> when the table definition changes? There still has to be an entry in
> pg_proc for the procedure. What happens to that entry when the table
> changes?
>
> You seem to be saying that %TYPE can be implemented via some other
> mechanism. That is fine with me, but how would that other mechanism
> work? Why it would not raise the exact same set of issues?

What I (wanted to have) said is that the "one-time porting"
can be solved by external preprocessing/translation of %TYPE
into the resolved type at porting time. That is *porting*
instead of making the target system emulate the original
platform. You know, today you can run a mainframe application
on an Intel architecture by running IBM's OS390 emulator
under Linux - but is that porting?

And I repeat what I've allways said over the past years. I
don't feel the need for all the catalog mucking with most of
the ALTER commands. Changing column types here and there,
dropping and renaming columns and tables somewhere else and
kicking the entire schema while holding data around during
application coding doesn't have anything to do with
development or software engineering. It's pure script-kiddy
hacking or even worse quality. There seems to be no business
process description, no data model or any other "plan", just
this "let's code around until something seems to work all of
the sudden". Where's the problem description, application
spec, all the stuff the DB schema resulted from? Oh - it
resulted from "I need another column because I have this
unexpected value I need to keep - and if there'll be more of
them I can ALTER it to be an array". Well, if that's what
people consider "development", all they really need is

ALTER n% OF SCHEMA AT RANDOM;

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:

> What I (wanted to have) said is that the "one-time porting"
> can be solved by external preprocessing/translation of %TYPE
> into the resolved type at porting time. That is *porting*
> instead of making the target system emulate the original
> platform. You know, today you can run a mainframe application
> on an Intel architecture by running IBM's OS390 emulator
> under Linux - but is that porting?

Ah. My personal interest is not in doing a straight port from Oracle
to Postgres and never going back. I'm sure there are people
interested in that. Personally, I'm interested in supporting people
who want to use either Oracle or Postgres, or both, with the same
application.

> And I repeat what I've allways said over the past years. I
> don't feel the need for all the catalog mucking with most of
> the ALTER commands. Changing column types here and there,
> dropping and renaming columns and tables somewhere else and
> kicking the entire schema while holding data around during
> application coding doesn't have anything to do with
> development or software engineering. It's pure script-kiddy
> hacking or even worse quality. There seems to be no business
> process description, no data model or any other "plan", just
> this "let's code around until something seems to work all of
> the sudden". Where's the problem description, application
> spec, all the stuff the DB schema resulted from? Oh - it
> resulted from "I need another column because I have this
> unexpected value I need to keep - and if there'll be more of
> them I can ALTER it to be an array". Well, if that's what
> people consider "development", all they really need is
>
> ALTER n% OF SCHEMA AT RANDOM;

It is desirable to have some reasonable mechanism for changing the
schema without requiring data to be dumped and reloaded. Otherwise it
is very difficult to upgrade a system which needs to be up 24/7, such
as many web sites today.

It is not acceptable for eBay to shut down their system for even just
a few hours for maintenance. Shouldn't it be possible for eBay to run
on top of Postgres?

Ian

Ian Lance Taylor wrote:
> Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:
>
> > Altering a function definition in any language other than
> > PL/pgSQL really scares me. What do you expect a "C" function
> > declared to take a VARCHAR argument to do if you just change
> > the pg_proc entry telling it now takes a NAME? I'd expect it
> > to generate a signal 11 most of it's calls, and nothing
> > really useful the other times.
>
> Good point.
>
> That brings me back to choice 1 in my original message: don't try to
> change the function if the table definition changes.
>
> In fact, it's possible to do better. A procedural language could
> define a hook to handle table definition changes. The Postgres
> backend could define a way to register to receive notification of
> table definition changes (this would essentially be an entry in a
> table like the proposed pg_depends). The procedural language itself
> could then handle the table changes by redefining the function or
> whatever.
>
> When defining a function using %TYPE, the procedural language would be
> notified that %TYPE was used. It could then record a dependency, if
> it was prepared to handle one.

When defining a function, there is absolutely no language
dependant code invoked (except for 'sql'). So at the time
you do the CREATE FUNCTION, the PL/pgSQL handler doesn't even
get loaded. All the utility does is creating the pg_proc
entry.

When the analyzis of a query results in this pg_proc entries
oid to appear in a Func node and that Func node get's hit
during the queries execution, then the function manager will
load the PL handler and call it.

What you describe above is a general schema change callback
entry point into a procedural language module. It get's
called at CREATE/DROP FUNCTION and any other catalog change -
right? And the backend loads all declared procedural language
handlers at startup time so they can register themself for
callback - right? Sound's more like a bigger project than a
small grammar change.

> This would permit PL/pgSQL to redefine the function defined using
> %TYPE if that seems desirable. It would also permit PL/pgSQL to
> behave more reasonably with regard to variables defined using %TYPE.

Ah - so the CREATE FUNCTION utility doesn't create the
pg_proc entry any more, but just calls some function in the
PL handler doing all the job? Of course, one language might,
while another uses the backward compatibility mode of the
existing CREATE FUNCTION - that's neat. And since the general
schema change callback informs one PL (the one that want's to
get informed), every language could decide on it's own if
it's better to create another overload function, drop the
existing, modify the existing or just abort the transaction
if it gets confused.

> This would also permit the C function handler to issue a NOTICE when a
> C function was defined using %TYPE and the table definition was
> changed.

Seems I missed some code changes in the past, so where's this
new C function handler located and how does it work?

> I return to the question of whether the Postgres development team is
> interested in support for %TYPE. If the team is not interested, then
> I'm wasting my time. I'm seeing a no from you and Tom Lane, and a
> maybe from Bruce Momjian.

I don't say we shouldn't have support for %TYPE. But if we
have it, ppl will assume it tracks later schema changes, but
with what I've seen so far it either could have severe side
effects on other languages or just doesn't do it. A change
like %TYPE support is a little too fundamental to get this
quick yes/no decision just in a few days.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

> > I return to the question of whether the Postgres development team is
> > interested in support for %TYPE. If the team is not interested, then
> > I'm wasting my time. I'm seeing a no from you and Tom Lane, and a
> > maybe from Bruce Momjian.
>
> I don't say we shouldn't have support for %TYPE. But if we
> have it, ppl will assume it tracks later schema changes, but
> with what I've seen so far it either could have severe side
> effects on other languages or just doesn't do it. A change
> like %TYPE support is a little too fundamental to get this
> quick yes/no decision just in a few days.

Can't we just throw a NOTICE and let them do it. Seems harmless to me.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 853-3000
+ If your life is a hard drive, | 830 Blythe Avenue
+ Christ can be your backup. | Drexel Hill, Pennsylvania 19026

At 02:22 PM 5/30/01 -0700, Ian Lance Taylor wrote:

>Ah. My personal interest is not in doing a straight port from Oracle
>to Postgres and never going back. I'm sure there are people
>interested in that. Personally, I'm interested in supporting people
>who want to use either Oracle or Postgres, or both, with the same
>application.

Which is what we're doing with the OpenACS toolkit. We can (and have,
actually) stripped these out of the parameter lists but the resulting
function definitions are less clear.

Even with %TYPE we won't actually share datamodel sources, of course,
but the less that's different, the easier it is for folks to work
on the code.

- Don Baccus, Portland OR <dhogaza(at)pacifier(dot)com>
Nature photos, on-line guides, Pacific Northwest
Rare Bird Alert Service and other goodies at
http://donb.photo.net.

Ian Lance Taylor <ian(at)airs(dot)com> writes:
> It is desirable to have some reasonable mechanism for changing the
> schema without requiring data to be dumped and reloaded. Otherwise it
> is very difficult to upgrade a system which needs to be up 24/7, such
> as many web sites today.

> It is not acceptable for eBay to shut down their system for even just
> a few hours for maintenance. Shouldn't it be possible for eBay to run
> on top of Postgres?

What's that got to do with the argument at hand? On-the-fly schema
changes aren't free either; at the very least you have to lock down the
tables involved while you change them. When the change cascades across
multiple tables and functions (if it doesn't, this feature is hardly
of any use!), ISTM you still end up shutting down your operation for as
long as it takes to do the changes.

regards, tom lane

Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:

> What you describe above is a general schema change callback
> entry point into a procedural language module. It get's
> called at CREATE/DROP FUNCTION and any other catalog change -
> right? And the backend loads all declared procedural language
> handlers at startup time so they can register themself for
> callback - right? Sound's more like a bigger project than a
> small grammar change.

Yes. But since it doesn't look like the small grammar change will get
into the sources, the bigger project appears to be needed.

> I don't say we shouldn't have support for %TYPE. But if we
> have it, ppl will assume it tracks later schema changes, but
> with what I've seen so far it either could have severe side
> effects on other languages or just doesn't do it. A change
> like %TYPE support is a little too fundamental to get this
> quick yes/no decision just in a few days.

Understood. I don't need a quick yes/no decision on the patch--after
all, I submitted it a month ago.

What would help a lot, though, is some indication of whether this
patch is of interest. Should I put the time into doing something
along the lines that I outlined? Would that get accepted? Or would I
be wasting my time, and should I just keep my much simpler patch as a
local change?

I've been doing the free software thing for over a decade, both as a
contributor and as a maintainer, with many different projects. For
any given functionality, I've normally been able to say ``this would
be good'' or ``this would be bad'' or ``this would be too hard to
maintain'' or ``this is irrelevant, but it's OK if you do all the
work.'' I'm having trouble getting a feel for how Postgres
development is done. In general, I would like to see a roadmap, and I
would like to see where Oracle compatibility falls on that roadmap.
In specific, I'm trying to understand what the feeling is about this
particular functionality.

Ian

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Ian Lance Taylor <ian(at)airs(dot)com> writes:
> > It is desirable to have some reasonable mechanism for changing the
> > schema without requiring data to be dumped and reloaded. Otherwise it
> > is very difficult to upgrade a system which needs to be up 24/7, such
> > as many web sites today.
>
> > It is not acceptable for eBay to shut down their system for even just
> > a few hours for maintenance. Shouldn't it be possible for eBay to run
> > on top of Postgres?
>
> What's that got to do with the argument at hand? On-the-fly schema
> changes aren't free either; at the very least you have to lock down the
> tables involved while you change them. When the change cascades across
> multiple tables and functions (if it doesn't, this feature is hardly
> of any use!), ISTM you still end up shutting down your operation for as
> long as it takes to do the changes.

That's a lot better than a dump and restore.

I was just responding to Jan's comments about ALTER statements. Jan's
comments didn't appear to have anything to do with %TYPE, and mine
didn't either. Apologies if I misunderstood.

Ian

On Wed, May 30, 2001 at 12:30:23PM -0400, Tom Lane wrote:
> Actually that's the least of the issues. The real problem is that
> because of function overloading, myfunc(int4) and myfunc(int2) (for
> example) are considered completely different functions. It is thus
> not at all clear what should happen if I create myfunc(foo.f1%TYPE)
> and later alter the type of foo.f1 from int4 to int2. Does myfunc(int4)
> stop existing? What if a conflicting myfunc(int2) already exists?
> What happens to type-specific references to myfunc(int4) --- for
> example, what if it's used as the implementation function for an
> operator declared on int4?

Would the idea of %TYPE being considered a "default" type, so it won't
conflict with any more specific functions be out of the question?

For example, if I call myfunc(int4), it'll first check if there's a
myfunc(int4), then failing that, check if there's a myfunc(foo.bar%TYPE).

Umm.. of course, there's no reason why it should search in that order,
because checking for myfunc(foo.bar%TYPE) first would be just as valid,
but either way, it's a well defined semantic.

--
Michael Samuel <michael(at)miknet(dot)net>

Ian Lance Taylor wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>
> > Ian Lance Taylor <ian(at)airs(dot)com> writes:
> > > It is desirable to have some reasonable mechanism for changing the
> > > schema without requiring data to be dumped and reloaded. Otherwise it
> > > is very difficult to upgrade a system which needs to be up 24/7, such
> > > as many web sites today.
> >
> > > It is not acceptable for eBay to shut down their system for even just
> > > a few hours for maintenance. Shouldn't it be possible for eBay to run
> > > on top of Postgres?
> >
> > What's that got to do with the argument at hand? On-the-fly schema
> > changes aren't free either; at the very least you have to lock down the
> > tables involved while you change them. When the change cascades across
> > multiple tables and functions (if it doesn't, this feature is hardly
> > of any use!), ISTM you still end up shutting down your operation for as
> > long as it takes to do the changes.
>
> That's a lot better than a dump and restore.

Indeed.

> I was just responding to Jan's comments about ALTER statements. Jan's
> comments didn't appear to have anything to do with %TYPE, and mine
> didn't either. Apologies if I misunderstood.

That's what happens when ppl run out of arguments, and
developers are human beeings too - unfortunately ;-}

I think Bruce made a point in his other tread about imperfect
fixes. This is of course no fix but a feature. Then again we
have to think about "imperfect features" as well, and looking
at the past (foreign key, PL/pgSQL itself and lztext - just
to blame myself) I realize that I've not been that much of a
perfectionist I claim to be in recent posts.

And Bruce is right, the speed we demonstrated in gaining
features wouldn't have been possible if we'd insisted on
perfectionism all the time like we currently seem to do.

I can understand Ian. Working for some time on a feature,
posting a patch and watching it going down in the flames of
discussion is frustrating. Even more frustrating is it if you
asked for discussion before and nobody responded with more
than a *shrug* - then when you've done the work the
discussion starts.

At least we know by now that we want to have that feature.
And we know that we can't do it perfect now. Since we know
that doing a halfhearted tracking could severely break other
things, it's out of discussion. So the question we have to
answer is if we accept the %TYPE syntax with immediate type
resolution and delay the real fix until the FAQ's force
someone to do it. It doesn't hurt as long as you don't use it
AND expect it to do more than that. So a NOTICE at the
actual usage, telling that x%TYPE for y got resolved to
basetype z and will currently NOT follow later changes to x
should do it.

Jan

--

#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me. #
#================================================== JanWieck(at)Yahoo(dot)com #

_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com

Hi,

I've been following this discussion with interest. As a member of the
OpenACS community I'd like to see the %TYPE feature in PG ASAP. I also
understand the reluctance of some of the PG team members in implementing
something that is not anywhere near 'perfect'.

I like Jans' (and Ian?) suggestion of ONLY doing resolution at create
time, as a full 'tracking-the-current-definition' seems to too tough for
now. I think it will be very acceptable to a lot of us out there to have
to drop and re-create our own dependancies. A lot of times, the changes
may not require recoding of the function (except for languages like C).
For OpenACS, schema changes on production machines will mostly be managed
by upgrade sql scripts. Although not 'perfect', having to drop and
recreate functions during upgrade are only minor problems.

> AND expect it to do more than that. So a NOTICE at the
> actual usage, telling that x%TYPE for y got resolved to
> basetype z and will currently NOT follow later changes to x
> should do it.

So if you could implement it like that, we will be VERY happy.

Regards,
Pascal Scheffers

I've been thinking about this, and I think the smartest way to implement
%TYPE would be to have it as a special-case data type. So, the C
representation of it would be something like this:

struct PercentType {
int datatype;
void *data;
};

Note: I made the datatype field an int, but that may/may not be the
correct datatype to use there.

And basically, postgres can resolve at runtime what it should point to,
and the code should have to deal with it, either via casting, or throwing
an exception if it's unacceptable.

Of course, there'd be a small overhead within the function, but it's a
small price to pay for a robust implementation.

As for operator overloading, a decision must be made whether you search
for a more specific function first, or for a matching %TYPE.

Of course, this may be too many special cases to be coded cleanly...

--
Michael Samuel <michael(at)miknet(dot)net>

Michael Samuel <michael(at)miknet(dot)net> writes:

> I've been thinking about this, and I think the smartest way to implement
> %TYPE would be to have it as a special-case data type. So, the C
> representation of it would be something like this:
>
> struct PercentType {
> int datatype;
> void *data;
> };
>
> Note: I made the datatype field an int, but that may/may not be the
> correct datatype to use there.
>
> And basically, postgres can resolve at runtime what it should point to,
> and the code should have to deal with it, either via casting, or throwing
> an exception if it's unacceptable.
>
> Of course, there'd be a small overhead within the function, but it's a
> small price to pay for a robust implementation.
>
> As for operator overloading, a decision must be made whether you search
> for a more specific function first, or for a matching %TYPE.

Functions are stored in the pg_proc table. That table has 16 fields
which hold the OIDs of the types of the arguments. When searching for
a function, the types of the parameters are used to search the table.
We would have to figure out a way to store the %TYPE field instead.

Perhaps one approach would be to have a separate table which just held
%TYPE entries. Then pg_proc could hold the OID of the row in that
table. The parser code which hooks up function calls with function
definitions would have to recognize this case and convert the %TYPE
into the real type at that time. This would only be done if there was
no exact match, so there would only be a performance penalty when
%TYPE was used.

The code could be written such that a function which specified the
exact type would always be chosen before a function which used %TYPE.
However, a function which used %TYPE to specify the exact type would
be chosen before a function which specified a coerceable type.

Probably several other places would have to be prepared to convert an
entry in the new %TYPE table to an entry in the pg_type field. But
that could be encapsulated in a function.

Whether this is of any interest or not, I don't know.

Ian