2015-04-08 4:55 GMT+02:00 Peter Eisentraut <pete...@gmx.net>:

> On 3/22/15 5:46 AM, Pavel Stehule wrote:
> > Isn't better doesn't support "TRANSFORM ALL" clause? If somebody would
> > to use transformations - then he have to explicitly enable it by
> > "TRANSFORM FOR TYPE" ? It is safe and without possible user
> unexpectations.
>
> Following our off-list conversation, here is a new patch that removes
> the TRANSFORM ALL/NONE clauses and requires an explicit list.
>

Nice, thank you very much

I checked the description of this feature in other databases and in
standard. There is little bit different situations - because there is not
possibility to use external languages without transformations. In
PostgreSQL we living without transformations long years, so we have to
solve a co-existence old code (or functions without the using
transformations) and new code (functions that use transformations). We have
to solve a possible compatibility issues. The last design requires to
specify explicitly list of types that are transformed. Nothing
transformation is used by default (implicitly). So old code have to work
without any issues and new code is clearly marked. Now I don't afraid of
introduction of transformations. It is safe.

Review
----------
1. What it does? - it introduce a secondary way, how to pass values between
PostgreSQL and PL languages.

2. Does we would this feature? Surely - we would. It is safe way for
passing complex types more cleverly and use it in PL more comfortably.
Enhancing work with hstore in PLPerl, PLPython is long desired feature.

3. It can enforce some compatibility issues? No, last design is safe - the
using transformation of any type must be explicitly enabled. It is clean
what types will be transformed, and when transformations is required and
when not.

4. I was able to apply patch cleanly - there are no compilation warnings.

5. This feature is documented well - new SQL statements, new system tables.

6. Code is clean and well documented.

7. This feature has enough regress tests - all tests passed without
problems.

8. It requires pg_dump support. I checked it - no problems

I have not any objection. I'll mark it as ready for commit.

Minor issue is missing support for \sf command in psql. I wrote small patch
that fix it.

Thank you very much for on this pretty nice feature.

Regards

Pavel
commit 7a0139a3d82b0d71682c317ff560799254986504
Author: Pavel Stehule <pavel.steh...@gooddata.com>
Date:   Wed Apr 8 09:50:30 2015 +0200

    fix \sf when function has trf types

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 3ee3b57..5ffb712 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -306,6 +306,7 @@ static text *pg_get_expr_worker(text *expr, Oid relid, const char *relname,
 static int print_function_arguments(StringInfo buf, HeapTuple proctup,
 						 bool print_table_args, bool print_defaults);
 static void print_function_rettype(StringInfo buf, HeapTuple proctup);
+static void print_function_trftypes(StringInfo buf, HeapTuple proctup);
 static void set_rtable_names(deparse_namespace *dpns, List *parent_namespaces,
 				 Bitmapset *rels_used);
 static bool refname_is_unique(char *refname, deparse_namespace *dpns,
@@ -1945,6 +1946,9 @@ pg_get_functiondef(PG_FUNCTION_ARGS)
 	(void) print_function_arguments(&buf, proctup, false, true);
 	appendStringInfoString(&buf, ")\n RETURNS ");
 	print_function_rettype(&buf, proctup);
+
+	print_function_trftypes(&buf, proctup);
+
 	appendStringInfo(&buf, "\n LANGUAGE %s\n",
 					 quote_identifier(get_language_name(proc->prolang, false)));
 
@@ -2342,6 +2346,30 @@ is_input_argument(int nth, const char *argmodes)
 }
 
 /*
+ * Append used transformated types to specified buffer
+ */
+static void
+print_function_trftypes(StringInfo buf, HeapTuple proctup)
+{
+	Oid	*trftypes;
+	int	ntypes;
+
+	ntypes = get_func_trftypes(proctup, &trftypes);
+	if (ntypes > 0)
+	{
+		int	i;
+
+		appendStringInfoString(buf, "\n TRANSFORM ");
+		for (i = 0; i < ntypes; i++)
+		{
+			if (i != 0)
+				appendStringInfoString(buf, ", ");
+				appendStringInfo(buf, "FOR TYPE %s", format_type_be(trftypes[i]));
+		}
+	}
+}
+
+/*
  * Get textual representation of a function argument's default value.  The
  * second argument of this function is the argument number among all arguments
  * (i.e. proallargtypes, *not* proargtypes), starting with 1, because that's
diff --git a/src/backend/utils/fmgr/funcapi.c b/src/backend/utils/fmgr/funcapi.c
index 204e124..ebd7ddd 100644
--- a/src/backend/utils/fmgr/funcapi.c
+++ b/src/backend/utils/fmgr/funcapi.c
@@ -877,6 +877,50 @@ get_func_arg_info(HeapTuple procTup,
 	return numargs;
 }
 
+/*
+ * get_func_trftypes
+ *
+ * Returns a number of transformated types used by function.
+ */
+int
+get_func_trftypes(HeapTuple procTup,
+				  Oid **p_trftypes)
+{
+
+	Form_pg_proc procStruct = (Form_pg_proc) GETSTRUCT(procTup);
+	Datum		protrftypes;
+	ArrayType  *arr;
+	int			nelems;
+	bool			isNull;
+
+	protrftypes = SysCacheGetAttr(PROCOID, procTup,
+									 Anum_pg_proc_protrftypes,
+									 &isNull);
+	if (!isNull)
+	{
+		/*
+		 * We expect the arrays to be 1-D arrays of the right types; verify
+		 * that.  For the OID and char arrays, we don't need to use
+		 * deconstruct_array() since the array data is just going to look like
+		 * a C array of values.
+		 */
+		arr = DatumGetArrayTypeP(protrftypes);		/* ensure not toasted */
+		nelems = ARR_DIMS(arr)[0];
+		if (ARR_NDIM(arr) != 1 ||
+			nelems < 0 ||
+			ARR_HASNULL(arr) ||
+			ARR_ELEMTYPE(arr) != OIDOID)
+			elog(ERROR, "protrftypes is not a 1-D Oid array");
+		Assert(nelems >= procStruct->pronargs);
+		*p_trftypes = (Oid *) palloc(nelems * sizeof(Oid));
+		memcpy(*p_trftypes, ARR_DATA_PTR(arr),
+			   nelems * sizeof(Oid));
+
+		return nelems;
+	}
+	else
+		return 0;
+}
 
 /*
  * get_func_input_arg_names
diff --git a/src/include/funcapi.h b/src/include/funcapi.h
index 38d1451..9031b09 100644
--- a/src/include/funcapi.h
+++ b/src/include/funcapi.h
@@ -176,6 +176,8 @@ extern int get_func_arg_info(HeapTuple procTup,
 extern int get_func_input_arg_names(Datum proargnames, Datum proargmodes,
 						 char ***arg_names);
 
+extern int get_func_trftypes(HeapTuple procTup,
+					    Oid **p_trftypes);
 extern char *get_func_result_name(Oid functionId);
 
 extern TupleDesc build_function_result_tupdesc_d(Datum proallargtypes,
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to