Subject: patch: fix to use ``pg_get_functiondef()``

From: Andrej Antonov <[email-masked]>
Date: Tue, 08 Dec 2015 11:50:00 +0300

patch: fix to use ``pg_get_functiondef()`` [see attachment file]

it is copy of pull-request https://github.com/postgres/pgadmin3/pull/12

thank you!

-- 
Андрей Антонов,
инженер-программист отдела информационных технологий и программирования,
компания «Импульс М»

From c8330fefa05278b73f8a4f79645f4c4e92f34375 Mon Sep 17 00:00:00 2001
From: "Andrej Antonov (impulsm.work)" <[email-masked]>
Date: Mon, 7 Dec 2015 18:26:52 +0300
Subject: [PATCH] using ``pg_get_functiondef(..)``

---
 pgadmin/include/schema/pgFunction.h | 10 +++++++++-
 pgadmin/schema/pgFunction.cpp       | 31 ++++++++++++++++++++++++-------
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/pgadmin/include/schema/pgFunction.h b/pgadmin/include/schema/pgFunction.h
index 7cd6ffd..bc39865 100644
--- a/pgadmin/include/schema/pgFunction.h
+++ b/pgadmin/include/schema/pgFunction.h
@@ -118,6 +118,14 @@ public:
 	{
 		source = s;
 	}
+	wxString GetFunctionDefByPg() const
+	{
+		return functionDefByPg;
+	}
+	void iSetFunctionDefByPg(const wxString &s)
+	{
+		functionDefByPg = s;
+	}
 	wxString GetBin() const
 	{
 		return bin;
@@ -251,7 +259,7 @@ protected:
 	pgFunction(pgSchema *newSchema, int newType, const wxString &newName = wxT(""));
 
 private:
-	wxString returnType, language, volatility, source, bin;
+	wxString returnType, language, volatility, source, functionDefByPg, bin;
 	wxArrayString argNamesArray, argTypesArray, argModesArray, argDefsArray;
 	bool returnAsSet, secureDefiner, isStrict, isWindow, isLeakProof;
 	long argCount, cost, rows, argDefValCount, procType;
diff --git a/pgadmin/schema/pgFunction.cpp b/pgadmin/schema/pgFunction.cpp
index bd0342a..5440a09 100644
--- a/pgadmin/schema/pgFunction.cpp
+++ b/pgadmin/schema/pgFunction.cpp
@@ -287,12 +287,18 @@ wxString pgFunction::GetSql(ctlTree *browser)
 		wxString qtSig = GetQuotedFullIdentifier()  + wxT("(") + GetArgSigList() + wxT(")");
 
 		sql = wxT("-- Function: ") + qtSig + wxT("\n\n")
-		      + wxT("-- DROP FUNCTION ") + qtSig + wxT(";")
-		      + wxT("\n\nCREATE OR REPLACE FUNCTION ") + qtName;
-
-		// Use Oracle style syntax for edb-spl functions
-		if (GetLanguage() == wxT("edbspl") && GetProcType() == 2)
+		      + wxT("-- DROP FUNCTION ") + qtSig + wxT(";\n\n");
+		
+		if (!!GetFunctionDefByPg())
+		{
+			sql += GetFunctionDefByPg().Trim();
+		}
+		else if (GetLanguage() == wxT("edbspl") && GetProcType() == 2)
 		{
+			sql += wxT("CREATE OR REPLACE FUNCTION ") + qtName;
+
+			// Use Oracle style syntax for edb-spl functions
+
 			sql += wxT("\nRETURN ");
 			sql += GetReturnType();
 
@@ -304,6 +310,8 @@ wxString pgFunction::GetSql(ctlTree *browser)
 		}
 		else
 		{
+			sql += wxT("CREATE OR REPLACE FUNCTION ") + qtName;
+
 			sql += wxT("\n  RETURNS ");
 			if (GetReturnAsSet() && !GetReturnType().StartsWith(wxT("TABLE")))
 				sql += wxT("SETOF ");
@@ -643,7 +651,7 @@ pgFunction *pgFunctionFactory::AppendFunctions(pgObject *obj, pgSchema *schema,
 	cacheMap typeCache, exprCache;
 
 	pgFunction *function = 0;
-	wxString argNamesCol, argDefsCol, proConfigCol, proType, seclab;
+	wxString argNamesCol, argDefsCol, proConfigCol, proType, functionDefByPgSelect, seclab;
 	if (obj->GetConnection()->BackendMinimumVersion(8, 0))
 		argNamesCol = wxT("proargnames, ");
 	if (obj->GetConnection()->HasFeature(FEATURE_FUNCTION_DEFAULTS) && !obj->GetConnection()->BackendMinimumVersion(8, 4))
@@ -654,6 +662,11 @@ pgFunction *pgFunctionFactory::AppendFunctions(pgObject *obj, pgSchema *schema,
 		proConfigCol = wxT("proconfig, ");
 	if (obj->GetConnection()->EdbMinimumVersion(8, 1))
 		proType = wxT("protype, ");
+	if (obj->GetConnection()->BackendMinimumVersion(8, 4))
+	{
+		functionDefByPgSelect = wxT(",\n")
+		         wxT("pg_get_functiondef(pr.oid) AS function_def_by_pg");
+	}
 	if (obj->GetConnection()->BackendMinimumVersion(9, 1))
 	{
 		seclab = wxT(",\n")
@@ -664,7 +677,7 @@ pgFunction *pgFunctionFactory::AppendFunctions(pgObject *obj, pgSchema *schema,
 	pgSet *functions = obj->GetDatabase()->ExecuteSet(
 	                       wxT("SELECT pr.oid, pr.xmin, pr.*, format_type(TYP.oid, NULL) AS typname, typns.nspname AS typnsp, lanname, ") +
 	                       argNamesCol  + argDefsCol + proConfigCol + proType +
-	                       wxT("       pg_get_userbyid(proowner) as funcowner, description") + seclab + wxT("\n")
+	                       wxT("       pg_get_userbyid(proowner) as funcowner, description") + functionDefByPgSelect + seclab + wxT("\n")
 	                       wxT("  FROM pg_proc pr\n")
 	                       wxT("  JOIN pg_type typ ON typ.oid=prorettype\n")
 	                       wxT("  JOIN pg_namespace typns ON typns.oid=typ.typnamespace\n")
@@ -948,6 +961,10 @@ pgFunction *pgFunctionFactory::AppendFunctions(pgObject *obj, pgSchema *schema,
 			function->iSetReturnAsSet(functions->GetBool(wxT("proretset")));
 			function->iSetIsStrict(functions->GetBool(wxT("proisstrict")));
 			function->iSetSource(functions->GetVal(wxT("prosrc")));
+			if (!!functionDefByPgSelect)
+			{
+				function->iSetFunctionDefByPg(functions->GetVal(wxT("function_def_by_pg")));
+			}
 			function->iSetBin(functions->GetVal(wxT("probin")));
 
 			wxString vol = functions->GetVal(wxT("provolatile"));
-- 
2.6.3

-- 
Sent via pgadmin-hackers mailing list ([email-masked])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers

---

From: Andrej Antonov <[email-masked]>
Date: Tue, 08 Dec 2015 18:27:21 +0300

|||||||||||||||||||| COMMENT-1 ON GITHUB [BEGIN] ||||||||||||||||||||

<<< Dmitriy [email-masked] >>>

BTW

>  pg_get_functiondef() -- less wrong

what's wrong with this function? :)

|||||||||||||||||||| COMMENT-2 ON GITHUB [BEGIN] ||||||||||||||||||||

ok. a few examples:

example "A"
===========

write function:

     CREATE FUNCTION test123()
      RETURNS TABLE(my_col text)
      LANGUAGE plpgsql
     AS $xxx$
     BEGIN

     my_col := 'abc';
     RETURN NEXT;

     my_col := 'def';
     RETURN NEXT;

     END;
     $xxx$;

     --SELECT * FROM test123();

pgAdmin3 transforming it to function:

     -- Function: test123()

     -- DROP FUNCTION test123();

     CREATE OR REPLACE FUNCTION test123()
       RETURNS SETOF text AS
     $BODY$
     BEGIN

     my_col := 'abc';
     RETURN NEXT;

     my_col := 'def';
     RETURN NEXT;

     END;
     $BODY$
       LANGUAGE plpgsql VOLATILE
       COST 100
       ROWS 1000;
     ALTER FUNCTION test123()
       OWNER TO postgres;

and of course with error:

     ERROR:  "my_col" is not a known variable
     LINE 10: my_col := 'abc';
              ^

     ********** Error **********

     ERROR: "my_col" is not a known variable
     SQL state: 42601
     Character: 129

example "B"
===========

     CREATE FUNCTION test234(arr text[] DEFAULT ARRAY['sss', 'ddd'])
      RETURNS SETOF text
      LANGUAGE plpgsql
     AS $xxx$
     DECLARE
       r record;
     BEGIN

     FOR r IN SELECT UNNEST(arr) LOOP
       RETURN NEXT r.unnest;
     END LOOP;

     END;
     $xxx$;

     --SELECT * FROM test234();

pgAdmin3 transforming it to function:

     -- Function: test234(text[])

     -- DROP FUNCTION test234(text[]);

     CREATE OR REPLACE FUNCTION test234(arr text[] DEFAULT 
ARRAY['sss'::text)
       RETURNS SETOF text AS
     $BODY$
     DECLARE
       r record;
     BEGIN

     FOR r IN SELECT UNNEST(arr) LOOP
       RETURN NEXT r.unnest;
     END LOOP;

     END;
     $BODY$
       LANGUAGE plpgsql VOLATILE
       COST 100
       ROWS 1000;
     ALTER FUNCTION test234(text[])
       OWNER TO postgres;

...with error:

     ERROR:  syntax error at or near ")"
     LINE 5: ...PLACE FUNCTION test234(arr text[] DEFAULT 
ARRAY['sss'::text)
                                                                          
  ^
     ********** Error **********

     ERROR: syntax error at or near ")"
     SQL state: 42601
     Character: 137

other minor discomfort
======================

words -- ``VOLATILE``, ``COST 100``, ``ROWS 1000`` -- are extra 
(unnecessary, redundant).

|||||||||||||||||||| COMMENT-3 ON GITHUB [BEGIN] ||||||||||||||||||||

but this sql code was generated by pgadmin, isn't it? I thought you are 
talking about native postgres function pg_get_functiondef() )

|||||||||||||||||||| COMMENT-4 ON GITHUB [BEGIN] ||||||||||||||||||||

> but this sql code was generated by pgadmin, isn't it?
> I thought you are talking about native postgres function 
> pg_get_functiondef()

sorry for my ambiguity message..

yes. my examples -- about pgadmin-generation (without 
``pg_get_functiondef()`` )

I wanted to say: I do not know about bad sides of 
``pg_get_functiondef()`` , but if they exists -- I think them less then 
bad sides of original-pgadmin-generation .

|||||||||||||||||||| END OF COMMENTS ON GITHUB ||||||||||||||||||||

---

From: Dmitriy Olshevskiy <[email-masked]>
Date: Sun, 13 Dec 2015 18:04:52 +0300

Hi, Andrej!
Here is small fix of your patch - can you check it please?
I think there must be wxwidgets function IsEmpty() instead of double 
negation,
because type of the variable is wxstring. Also I added the Trim() 
function before
check if function definition is empty or not.

-- 
Dmitriy Olshevskiy

diff --git a/pgadmin/schema/pgFunction.cpp b/pgadmin/schema/pgFunction.cpp
index ba49f79..c287c12 100644
--- a/pgadmin/schema/pgFunction.cpp
+++ b/pgadmin/schema/pgFunction.cpp
@@ -289,9 +289,10 @@ wxString pgFunction::GetSql(ctlTree *browser)
 		sql = wxT("-- Function: ") + qtSig + wxT("\n\n")
 		      + wxT("-- DROP FUNCTION ") + qtSig + wxT(";\n\n");
 		
-		if (!!GetFunctionDefByPg())
+		wxString functionDefByPgTrim = GetFunctionDefByPg().Trim();
+		if (!functionDefByPgTrim.IsEmpty())
 		{
-			sql += GetFunctionDefByPg().Trim();
+			sql += functionDefByPgTrim;
 		}
 		else if (GetLanguage() == wxT("edbspl") && GetProcType() == 2)
 		{
@@ -959,7 +960,7 @@ pgFunction *pgFunctionFactory::AppendFunctions(pgObject *obj, pgSchema *schema,
 			function->iSetReturnAsSet(functions->GetBool(wxT("proretset")));
 			function->iSetIsStrict(functions->GetBool(wxT("proisstrict")));
 			function->iSetSource(functions->GetVal(wxT("prosrc")));
-			if (!!functionDefByPgSelect)
+			if (!functionDefByPgSelect.IsEmpty())
 			{
 				function->iSetFunctionDefByPg(functions->GetVal(wxT("function_def_by_pg")));
 			}

---

From: Andrej Antonov <[email-masked]>
Date: Mon, 14 Dec 2015 10:51:15 +0300

thank you, Dmitriy. I agree -- this-fix should works better.

I applyed this-fix to my local-git-branches ("REL-1_20_0-impulsm" and 
"fix-to-use-pg_get_functiondef"). works good.

Dmitriy Olshevskiy писал 2015-12-13 18:04:
>> it is copy of pull-request
>> https://github.com/postgres/pgadmin3/pull/12 [1]
>> 
>> thank you!
> 
> --
> Dmitriy Olshevskiy
> 
> 
> Links:
> ------
> [1] https://github.com/postgres/pgadmin3/pull/12

---

From: Andrej Antonov <[email-masked]>
Date: Fri, 18 Dec 2015 12:22:37 +0300

small fix ( diff see here: 
https://github.com/postgres-impulsm/pgadmin3/commit/72f381aa1964d5630f3ada44768bc562911102e9 
)

if we using ``pg_get_functiondef(func_oid)`` --
   in this case -- no need to write additional ``ALTER FUNCTION ... = ... 
;``.

From 93170fb2ac121da60a10e53585bee180bc2dc652 Mon Sep 17 00:00:00 2001
From: "Andrej Antonov (impulsm.work)" <[email-masked]>
Date: Mon, 7 Dec 2015 18:26:52 +0300
Subject: [PATCH] using ``pg_get_functiondef(..)``

---
 pgadmin/include/schema/pgFunction.h | 10 ++++++-
 pgadmin/schema/pgFunction.cpp       | 53 ++++++++++++++++++++++++++-----------
 2 files changed, 46 insertions(+), 17 deletions(-)

 private:
index bd0342a..2a39079 100644
@@ -287,12 +287,19 @@ wxString pgFunction::GetSql(ctlTree *browser)

@@ -304,6 +311,8 @@ wxString pgFunction::GetSql(ctlTree *browser)
 		}
 		else
 		{
+			sql += wxT("CREATE OR REPLACE FUNCTION ") + qtName;
+
 			sql += wxT("\n  RETURNS ");
 			if (GetReturnAsSet() && !GetReturnType().StartsWith(wxT("TABLE")))
 				sql += wxT("SETOF ");
@@ -347,16 +356,19 @@ wxString pgFunction::GetSql(ctlTree *browser)
 		if (!sql.Strip(wxString::both).EndsWith(wxT(";")))
 			sql += wxT(";");
 
-		size_t i;
-		for (i = 0 ; i < configList.GetCount() ; i++)
+		if (functionDefByPgTrim.IsEmpty())
 		{
-			if (configList.Item(i).BeforeFirst('=') != wxT("search_path") &&
-			        configList.Item(i).BeforeFirst('=') != wxT("temp_tablespaces"))
-				sql += wxT("\nALTER FUNCTION ") + qtSig
-				       + wxT(" SET ") + configList.Item(i).BeforeFirst('=') + wxT("='") + configList.Item(i).AfterFirst('=') + wxT("';\n");
-			else
-				sql += wxT("\nALTER FUNCTION ") + qtSig
-				       + wxT(" SET ") + configList.Item(i).BeforeFirst('=') + wxT("=") + configList.Item(i).AfterFirst('=') + wxT(";\n");
+			size_t i;
+			for (i = 0 ; i < configList.GetCount() ; i++)
+			{
+				if (configList.Item(i).BeforeFirst('=') != wxT("search_path") &&
+					configList.Item(i).BeforeFirst('=') != wxT("temp_tablespaces"))
+					sql += wxT("\nALTER FUNCTION ") + qtSig
+					       + wxT(" SET ") + configList.Item(i).BeforeFirst('=') + wxT("='") + configList.Item(i).AfterFirst('=') + wxT("';\n");
+				else
+					sql += wxT("\nALTER FUNCTION ") + qtSig
+					       + wxT(" SET ") + configList.Item(i).BeforeFirst('=') + wxT("=") + configList.Item(i).AfterFirst('=') + wxT(";\n");
+			}
 		}
 
 		sql += wxT("\n")
@@ -643,7 +655,7 @@ pgFunction *pgFunctionFactory::AppendFunctions(pgObject *obj, pgSchema *schema,
 	cacheMap typeCache, exprCache;
 
@@ -654,6 +666,11 @@ pgFunction *pgFunctionFactory::AppendFunctions(pgObject *obj, pgSchema *schema,
@@ -664,7 +681,7 @@ pgFunction *pgFunctionFactory::AppendFunctions(pgObject *obj, pgSchema *schema,
@@ -948,6 +965,10 @@ pgFunction *pgFunctionFactory::AppendFunctions(pgObject *obj, pgSchema *schema,
 
2.6.4

---

From: Dmitriy Olshevskiy <[email-masked]>
Date: Wed, 30 Dec 2015 22:46:00 +0400

Hello, dear colleagues)
Could you please review this patch (see the 
previous letter)?
As I remember, there are several similar, but 
unclosed issues
in pgadmin mailing list yet. Really, there is a 
problem with
function's definition and this patch probably 
could solve it
(at least for postgresql 8.4 and higher). What do 
you think?
Thank you.

On 18.12.2015 13:22, Andrej Antonov wrote:
>   in this case -- no need to write additional 
> ``ALTER FUNCTION ... = ... ;``.
>
>

-- 
Dmitriy Olshevskiy