Re: [HACKERS] [PATCH] \ef function in psql
I wrote: ... define \ef with no argument as being the command that presents an empty CREATE FUNCTION command template to fill in. No complaints? I'll go make that happen. What about the general issue that neither \e nor \ef leave you with a presentation of what's in the query buffer? I haven't studied readline enough to know if that's fixable. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
At 2008-09-06 14:58:25 -0400, [EMAIL PROTECTED] wrote: I wrote: ... define \ef with no argument as being the command that presents an empty CREATE FUNCTION command template to fill in. No complaints? I'll go make that happen. No complaints, it sounds fine to me. What about the general issue that neither \e nor \ef leave you with a presentation of what's in the query buffer? I don't know how that can be fixed; but I agree with Brendan that it's behaviour that people are used to, and that it can be left alone for now. -- ams -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Tom Lane wrote: Brendan Jurd [EMAIL PROTECTED] writes: On Sat, Sep 6, 2008 at 10:10 AM, Tom Lane [EMAIL PROTECTED] wrote: ... I changed the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes the command to wait in the query buffer. The principle of least astonishment suggests that \ef should behave in the same way as \e. Quite. So, are they consistent now or do we need another patch? -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: Brendan Jurd [EMAIL PROTECTED] writes: The principle of least astonishment suggests that \ef should behave in the same way as \e. Quite. So, are they consistent now or do we need another patch? They are consistent, I just don't like either of them ;-) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Abhijit Menon-Sen wrote: At 2008-09-06 14:58:25 -0400, [EMAIL PROTECTED] wrote: What about the general issue that neither \e nor \ef leave you with a presentation of what's in the query buffer? I don't know how that can be fixed; but I agree with Brendan that it's behaviour that people are used to, and that it can be left alone for now. As far as it works to not execute the query when the user exits without saving the buffer, it should be OK. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Alvaro Herrera wrote: Abhijit Menon-Sen wrote: At 2008-09-06 14:58:25 -0400, [EMAIL PROTECTED] wrote: What about the general issue that neither \e nor \ef leave you with a presentation of what's in the query buffer? I don't know how that can be fixed; but I agree with Brendan that it's behaviour that people are used to, and that it can be left alone for now. As far as it works to not execute the query when the user exits without saving the buffer, it should be OK. Well, it works like \e now, which is good. The only complexity is that \e works differently depending on whether you use ';' or \g, meaning: SELECT 1; \e will execute the buffer on exit (saved or not), while SELECT 1 \g \e will not execute the buffer on editor exit. Our current \ef code does not add a trailing semicolon to the CREATE FUNCTION buffer contents so it works like the second case, which is probably the best we are going to do. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Abhijit Menon-Sen [EMAIL PROTECTED] writes: I have attached two patches: - funcdef.diff implements pg_get_functiondef() - edit.diff implements \ef function in psql based on (1). I've applied this with some corrections (mostly around proper quoting) and some outright editorialization: * the psql command seemed to have some ideas about supplying a blank CREATE OR REPLACE FUNCTION command for a nonexistent function, but this didn't actually work. In any case it seemed poorly thought out, because you'd really need to pay some attention to *why* the regproc/regprocedure lookup failed. I just ripped it out for the moment. I'm not averse to the concept, if you can get it implemented properly. * the way you had it set up, the CREATE OR REPLACE FUNCTION command would be sent to the backend instantaneously upon return from the editor, with no opportunity for the user to decide he didn't want his changes applied. This seemed unacceptably dangerous to me. I changed the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes the command to wait in the query buffer. Unfortunately there's no visual indication of that, other than a small change in the prompt status. It'd likely be better if we could get libreadline to redisplay the query buffer contents --- anyone have an idea how to do that? (I have some vague recollection that \e used to work that way, though it definitely fails to do so now.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
On Sat, Sep 6, 2008 at 10:10 AM, Tom Lane [EMAIL PROTECTED] wrote: * the way you had it set up, the CREATE OR REPLACE FUNCTION command would be sent to the backend instantaneously upon return from the editor, with no opportunity for the user to decide he didn't want his changes applied. This seemed unacceptably dangerous to me. I changed the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes the command to wait in the query buffer. The principle of least astonishment suggests that \ef should behave in the same way as \e. With \e (which I use a lot), the command(s) are immediately executed by the backend as soon as you write and exit from the editor. I don't find that dangerous, and anyone who uses \e will already be very much accustomed to it. If \ef did something different, it would just be weird. If you're not sure you want to execute the contents of your \e editor session after all, you can always delete the semicolon, or everything in the file, before quitting. Cheers, BJ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Brendan Jurd [EMAIL PROTECTED] writes: On Sat, Sep 6, 2008 at 10:10 AM, Tom Lane [EMAIL PROTECTED] wrote: ... I changed the exit code to PSQL_CMD_NEWEDIT instead of PSQL_CMD_SEND, which causes the command to wait in the query buffer. The principle of least astonishment suggests that \ef should behave in the same way as \e. Quite. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
I wrote: * the psql command seemed to have some ideas about supplying a blank CREATE OR REPLACE FUNCTION command for a nonexistent function, but this didn't actually work. In any case it seemed poorly thought out, because you'd really need to pay some attention to *why* the regproc/regprocedure lookup failed. I just ripped it out for the moment. I'm not averse to the concept, if you can get it implemented properly. While I was out at dinner, the obvious solution presented itself: define \ef with no argument as being the command that presents an empty CREATE FUNCTION command template to fill in. This isn't any more or less typing than where I think you were going, and it eliminates all the ambiguity about whether you meant to type a nonexistent function name or just mistyped. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
PL/LOLCODE [was Re: [HACKERS] [PATCH] \ef function in psql]
On Mon, Aug 04, 2008 at 10:31:10AM -0700, David Wheeler wrote: On Jul 31, 2008, at 00:07, Abhijit Menon-Sen wrote: I have attached two patches: - funcdef.diff implements pg_get_functiondef() - edit.diff implements \ef function in psql based on (1). Comments appreciated. +1 I like! The ability to easily edit a function on the fly in psql will be very welcome to DBAs I know. And I like the pg_get_functiondef() function, too, a that will simplify editing existing functions in other admin apps, like pgAdmin. I'm starting to get really excited for 8.4. I can haz cheezburger? You do understand you've just kicked off a discussion of shipping PL/LOLCODE by default. Oops, I mean, when does it ship? ;-P Christmas ;) Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
On Jul 31, 2008, at 00:07, Abhijit Menon-Sen wrote: I have attached two patches: - funcdef.diff implements pg_get_functiondef() - edit.diff implements \ef function in psql based on (1). Comments appreciated. +1 I like! The ability to easily edit a function on the fly in psql will be very welcome to DBAs I know. And I like the pg_get_functiondef() function, too, a that will simplify editing existing functions in other admin apps, like pgAdmin. I'm starting to get really excited for 8.4. I can haz cheezburger? Oops, I mean, when does it ship? ;-P Thanks, David -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
I have attached two patches:
- funcdef.diff implements pg_get_functiondef()
- edit.diff implements \ef function in psql based on (1).
Comments appreciated.
-- ams
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 1ba20b0..ccf0d68 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -551,6 +551,7 @@ extern Datum pg_get_expr(PG_FUNCTION_ARGS);
extern Datum pg_get_expr_ext(PG_FUNCTION_ARGS);
extern Datum pg_get_userbyid(PG_FUNCTION_ARGS);
extern Datum pg_get_serial_sequence(PG_FUNCTION_ARGS);
+extern Datum pg_get_functiondef(PG_FUNCTION_ARGS);
extern Datum pg_get_function_arguments(PG_FUNCTION_ARGS);
extern Datum pg_get_function_result(PG_FUNCTION_ARGS);
extern char *deparse_expression(Node *expr, List *dpcontext,
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 0d28310..71e601a 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -137,6 +137,7 @@ static char *pg_get_expr_worker(text *expr, Oid relid, char *relname,
int prettyFlags);
static int print_function_arguments(StringInfo buf, HeapTuple proctup,
bool print_table_args);
+static void print_function_rettype(StringInfo buf, HeapTuple proctup);
static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
int prettyFlags);
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
@@ -1398,6 +1399,137 @@ pg_get_serial_sequence(PG_FUNCTION_ARGS)
/*
+ * pg_get_functiondef
+ * Returns the CREATE OR REPLACE FUNCTION ... statement for the
+ * specified function.
+ */
+Datum
+pg_get_functiondef(PG_FUNCTION_ARGS)
+{
+ Oid funcid = PG_GETARG_OID(0);
+ StringInfoData buf;
+ StringInfoData dq;
+ HeapTuple proctup;
+ HeapTuple langtup;
+ Form_pg_proc proc;
+ Form_pg_language lang;
+ bool isnull;
+ Datum tmp;
+ const char *prosrc;
+ const char *name;
+ const char *nsp;
+ float4 cost;
+ int n;
+
+ initStringInfo(buf);
+
+ proctup = SearchSysCache(PROCOID, ObjectIdGetDatum(funcid), 0, 0, 0);
+ if (!HeapTupleIsValid(proctup))
+ elog(ERROR, cache lookup failed for function %u, funcid);
+ proc = (Form_pg_proc) GETSTRUCT(proctup);
+
+ langtup = SearchSysCache(LANGOID, ObjectIdGetDatum(proc-prolang), 0, 0, 0);
+ if (!HeapTupleIsValid(langtup))
+ elog(ERROR, cache lookup failed for language %u, proc-prolang);
+ lang = (Form_pg_language) GETSTRUCT(langtup);
+
+ name = NameStr(proc-proname);
+ nsp = get_namespace_name(proc-pronamespace);
+ appendStringInfo(buf, CREATE OR REPLACE FUNCTION %s(,
+ quote_qualified_identifier(nsp, name));
+ (void) print_function_arguments(buf, proctup, false);
+ appendStringInfoString(buf, )\n RETURNS );
+ print_function_rettype(buf, proctup);
+ appendStringInfo(buf, \n LANGUAGE '%s'\n, NameStr(lang-lanname));
+
+ n = 1;
+
+ switch (proc-provolatile) {
+ case PROVOLATILE_IMMUTABLE:
+ appendStringInfoString(buf, IMMUTABLE);
+ break;
+ case PROVOLATILE_STABLE:
+ appendStringInfoString(buf, STABLE);
+ break;
+ case PROVOLATILE_VOLATILE:
+ default:
+ n--;
+ break;
+ }
+
+ if (proc-proisstrict)
+ {
+ n++;
+ appendStringInfoString(buf, STRICT);
+ }
+
+ if (proc-prosecdef)
+ {
+ n++;
+ appendStringInfoString(buf, SECURITY DEFINER);
+ }
+
+ cost = 100;
+ if (proc-prolang == INTERNALlanguageId ||
+ proc-prolang == ClanguageId)
+ cost = 1;
+
+ if (proc-procost != cost)
+ {
+ n++;
+ appendStringInfo(buf, COST %.0f, proc-procost);
+ }
+
+ if (proc-prorows != 0 proc-prorows != 1000)
+ {
+ n++;
+ appendStringInfo(buf, ROWS %.0f, proc-prorows);
+ }
+
+ if (n != 0)
+ appendStringInfoString(buf, \n);
+
+ tmp = SysCacheGetAttr(PROCOID, proctup, Anum_pg_proc_proconfig, isnull);
+ if (!isnull)
+ {
+ int i;
+ ArrayType *a = DatumGetArrayTypeP(tmp);
+
+ for (i = 1; i = ARR_DIMS(a)[0]; i++)
+ {
+ Datum d;
+ bool isnull;
+
+ d = array_ref(a, 1, i, -1, -1, false, 'i', isnull);
+ if (!isnull)
+ {
+const char *s = TextDatumGetCString(d);
+appendStringInfo(buf, SET %s\n, s);
+ }
+ }
+ }
+
+ tmp = SysCacheGetAttr(PROCOID, proctup, Anum_pg_proc_prosrc, isnull);
+ if (isnull)
+ elog(ERROR, null prosrc);
+ prosrc = TextDatumGetCString(tmp);
+
+ initStringInfo(dq);
+ appendStringInfoString(dq, $);
+ while (strstr(prosrc, dq.data) != NULL)
+ appendStringInfoString(dq, x);
+ appendStringInfoString(dq, $);
+
+ appendStringInfo(buf, AS %s\n%s\n%s;, dq.data, prosrc, dq.data);
+
+ ReleaseSysCache(langtup);
+ ReleaseSysCache(proctup);
+
+ PG_RETURN_TEXT_P(string_to_text(buf.data));
+}
+
+
+/*
* pg_get_function_arguments
* Get a nicely-formatted list of arguments for a function.
* This is everything that would go between the parentheses in
@@ -1436,8 +1568,6 @@ pg_get_function_result(PG_FUNCTION_ARGS)
Oid funcid = PG_GETARG_OID(0);
StringInfoData buf;
HeapTuple proctup;
- Form_pg_proc procform;
- int ntabargs = 0;
initStringInfo(buf);
@@ -1446,32 +1576,46 @@
Re: [HACKERS] [PATCH] \ef function in psql
At 2008-07-29 15:42:27 +0530, [EMAIL PROTECTED] wrote:
OK, I have a mostly working pg_get_functiondef now, and some
questions about the remaining pieces:
While I look for answers to those questions, here's the patch as it
stands now, in case anyone feels like reading through it.
-- ams
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
index 1ba20b0..ccf0d68 100644
--- a/src/include/utils/builtins.h
+++ b/src/include/utils/builtins.h
@@ -551,6 +551,7 @@ extern Datum pg_get_expr(PG_FUNCTION_ARGS);
extern Datum pg_get_expr_ext(PG_FUNCTION_ARGS);
extern Datum pg_get_userbyid(PG_FUNCTION_ARGS);
extern Datum pg_get_serial_sequence(PG_FUNCTION_ARGS);
+extern Datum pg_get_functiondef(PG_FUNCTION_ARGS);
extern Datum pg_get_function_arguments(PG_FUNCTION_ARGS);
extern Datum pg_get_function_result(PG_FUNCTION_ARGS);
extern char *deparse_expression(Node *expr, List *dpcontext,
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 0d28310..dbfeff5 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -137,6 +137,7 @@ static char *pg_get_expr_worker(text *expr, Oid relid, char *relname,
int prettyFlags);
static int print_function_arguments(StringInfo buf, HeapTuple proctup,
bool print_table_args);
+static void print_function_rettype(StringInfo buf, HeapTuple proctup);
static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
int prettyFlags);
static void make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
@@ -1398,6 +1399,129 @@ pg_get_serial_sequence(PG_FUNCTION_ARGS)
/*
+ * pg_get_functiondef
+ * Returns the CREATE FUNCTION ... statement for a function.
+ */
+Datum
+pg_get_functiondef(PG_FUNCTION_ARGS)
+{
+ Oid funcid = PG_GETARG_OID(0);
+ StringInfoData buf;
+ HeapTuple proctup;
+ HeapTuple langtup;
+ Form_pg_proc proc;
+ Form_pg_language lang;
+ bool isnull;
+ Datum tmp;
+ const char *prosrc;
+ const char *name;
+ const char *nsp;
+ float4 cost;
+ int n;
+
+ initStringInfo(buf);
+
+ proctup = SearchSysCache(PROCOID, ObjectIdGetDatum(funcid), 0, 0, 0);
+ if (!HeapTupleIsValid(proctup))
+ elog(ERROR, cache lookup failed for function %u, funcid);
+ proc = (Form_pg_proc) GETSTRUCT(proctup);
+
+ langtup = SearchSysCache(LANGOID, ObjectIdGetDatum(proc-prolang), 0, 0, 0);
+ if (!HeapTupleIsValid(langtup))
+ elog(ERROR, cache lookup failed for language %u, proc-prolang);
+ lang = (Form_pg_language) GETSTRUCT(langtup);
+
+ name = NameStr(proc-proname);
+ nsp = get_namespace_name(proc-pronamespace);
+ appendStringInfo(buf, CREATE FUNCTION %s(,
+ quote_qualified_identifier(nsp, name));
+ (void) print_function_arguments(buf, proctup, false);
+ appendStringInfoString(buf, )\n RETURNS );
+ print_function_rettype(buf, proctup);
+ appendStringInfo(buf, \n LANGUAGE '%s'\n, NameStr(lang-lanname));
+
+ n = 1;
+
+ switch (proc-provolatile) {
+ case PROVOLATILE_IMMUTABLE:
+ appendStringInfoString(buf, IMMUTABLE);
+ break;
+ case PROVOLATILE_STABLE:
+ appendStringInfoString(buf, STABLE);
+ break;
+ case PROVOLATILE_VOLATILE:
+ default:
+ n--;
+ break;
+ }
+
+ if (proc-proisstrict)
+ {
+ n++;
+ appendStringInfoString(buf, STRICT);
+ }
+
+ if (proc-prosecdef)
+ {
+ n++;
+ appendStringInfoString(buf, SECURITY DEFINER);
+ }
+
+ cost = 100;
+ if (proc-prolang == INTERNALlanguageId ||
+ proc-prolang == ClanguageId)
+ cost = 1;
+
+ if (proc-procost != cost)
+ {
+ n++;
+ appendStringInfo(buf, COST %f, proc-procost);
+ }
+
+ if (proc-prorows != 0 proc-prorows != 1000)
+ {
+ n++;
+ appendStringInfo(buf, ROWS %f, proc-prorows);
+ }
+
+ if (n != 0)
+ appendStringInfoString(buf, \n);
+
+ tmp = SysCacheGetAttr(PROCOID, proctup, Anum_pg_proc_proconfig, isnull);
+ if (!isnull)
+ {
+ int i;
+ ArrayType *a = DatumGetArrayTypeP(tmp);
+
+ for (i = 1; i = ARR_DIMS(a)[0]; i++)
+ {
+ Datum d;
+ bool isnull;
+
+ d = array_ref(a, 1, i, -1, -1, false, 'i', isnull);
+ if (!isnull)
+ {
+const char *s = TextDatumGetCString(d);
+appendStringInfo(buf, SET %s\n, s);
+ }
+ }
+ }
+
+ tmp = SysCacheGetAttr(PROCOID, proctup, Anum_pg_proc_prosrc, isnull);
+ if (isnull)
+ elog(ERROR, null prosrc);
+ prosrc = TextDatumGetCString(tmp);
+
+ appendStringInfo(buf, AS $$\n%s\n$$;, prosrc);
+
+ ReleaseSysCache(langtup);
+ ReleaseSysCache(proctup);
+
+ PG_RETURN_TEXT_P(string_to_text(buf.data));
+}
+
+
+/*
* pg_get_function_arguments
* Get a nicely-formatted list of arguments for a function.
* This is everything that would go between the parentheses in
@@ -1436,8 +1560,6 @@ pg_get_function_result(PG_FUNCTION_ARGS)
Oid funcid = PG_GETARG_OID(0);
StringInfoData buf;
HeapTuple proctup;
- Form_pg_proc procform;
- int ntabargs = 0;
initStringInfo(buf);
@@ -1446,32 +1568,46 @@ pg_get_function_result(PG_FUNCTION_ARGS)
0, 0, 0);
if (!HeapTupleIsValid(proctup))
elog(ERROR, cache lookup failed for function %u, funcid);
Re: [HACKERS] [PATCH] \ef function in psql
At 2008-07-23 10:50:03 -0400, [EMAIL PROTECTED] wrote: What would the function return? CREATE OR REPLACE FUNCTION ...? I think I'd go with CREATE FUNCTION for simplicity. OK, I have a mostly working pg_get_functiondef now, and some questions about the remaining pieces: 1. Why is prorows a float4? Should I print it that way, i.e. %f? 2. Can I print the contents of proconfig as just SET %s? It seems to work for the variables I've tried (e.g. DateStyle=iso), but I wonder if they'll always be quoted correctly (i.e., if the split on '=' thing pg_dump does is necessary for an 8.4 function). 3. Is there a function I can use from ruleutils.c to do dollar quoting of prosrc/probin? If not, and I have to write one, where should it live? 4. What exactly is probin? Do I need to worry about it at all? Thanks. -- ams -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Marko is talking about types created with CREATE TYPE CREATE FUNCTION fraud.get_user_status( i_key_user text ) RETURNS ret_get_user_status AS $$ Current pg_dump annoyingly removes schem reference from type. On Wed, Jul 23, 2008 at 6:19 PM, Tom Lane [EMAIL PROTECTED] wrote: Marko Kreen [EMAIL PROTECTED] writes: [ re pg_get_functiondef ] Please make it use full qualified names (schema.name) for both function name and result types. Current search_path juggling the pg_dump does is major PITA. Qualifying the function name seems like a good idea, but I'd advise against tinkering with the datatype references. It'll be hard to do correctly and it will make things very substantially uglier. Do you really want to show, eg, pg_catalog.int4 rather than integer? If you leave the backend code do what it wants to do here, the only way that there would be a problem is if someone changed their search_path in between pg_get_functiondef and trying to re-load the function definition. Which certainly ain't gonna happen for \ef, and it seems a bit implausible for any other use-case either. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Not so sure about omitting OR REPLACE. In my experience it is more often needed than not. Main argument for omitting might be to protect hackers from carelesse users :) On Wed, Jul 23, 2008 at 5:50 PM, Tom Lane [EMAIL PROTECTED] wrote: Abhijit Menon-Sen [EMAIL PROTECTED] writes: At 2008-07-17 18:28:19 -0400, [EMAIL PROTECTED] wrote: It wouldn't take a whole lot to convince me that a pg_get_functiondef would be useful, although I don't foresee either of those applications wanting to use it because of their backward-compatibility constraints. What would the function return? CREATE OR REPLACE FUNCTION ...? Would that be good enough for everyone who might want to call it? I think I'd go with CREATE FUNCTION for simplicity. It would be easy enough for something like \ef to splice in OR REPLACE before shipping the command back to the server. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
+1 for CREATE OR REPLACE ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
On Tue, Jul 29, 2008 at 02:21:18PM -0400, Robert Haas wrote: +1 for CREATE OR REPLACE ...Robert +1 for CREATE OR REPLACE from me, too :) Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
At 2008-07-17 18:28:19 -0400, [EMAIL PROTECTED] wrote: It wouldn't take a whole lot to convince me that a pg_get_functiondef would be useful, although I don't foresee either of those applications wanting to use it because of their backward-compatibility constraints. If the code lives in psql (as with my patch), then it has some chance of working with older servers, but if you're happy with pg_get_functiondef, then I'm happy enough to use it to get \ef working. I agree that pg_dump wouldn't want to use it, of course, but I guess it doesn't matter very much if \ef doesn't work on older servers. What would the function return? CREATE OR REPLACE FUNCTION ...? Would that be good enough for everyone who might want to call it? (BTW, psql from 8.3 is already somewhat broken with 8.1: archiveopteryx= \d access_keys ERROR: column i.indisvalid does not exist And 8.2 as well: archiveopteryx= \d access_keys ERROR: column t.tgconstraint does not exist LINE 3: WHERE t.tgrelid = '16847' AND t.tgconstraint = 0 ^ Oh, I see they've both been fixed in CVS. Sorry for the noise.) -- ams -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Abhijit Menon-Sen [EMAIL PROTECTED] writes: At 2008-07-17 18:28:19 -0400, [EMAIL PROTECTED] wrote: It wouldn't take a whole lot to convince me that a pg_get_functiondef would be useful, although I don't foresee either of those applications wanting to use it because of their backward-compatibility constraints. What would the function return? CREATE OR REPLACE FUNCTION ...? Would that be good enough for everyone who might want to call it? I think I'd go with CREATE FUNCTION for simplicity. It would be easy enough for something like \ef to splice in OR REPLACE before shipping the command back to the server. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
On 7/23/08, Tom Lane [EMAIL PROTECTED] wrote: Abhijit Menon-Sen [EMAIL PROTECTED] writes: At 2008-07-17 18:28:19 -0400, [EMAIL PROTECTED] wrote: It wouldn't take a whole lot to convince me that a pg_get_functiondef would be useful, although I don't foresee either of those applications wanting to use it because of their backward-compatibility constraints. What would the function return? CREATE OR REPLACE FUNCTION ...? Would that be good enough for everyone who might want to call it? I think I'd go with CREATE FUNCTION for simplicity. It would be easy enough for something like \ef to splice in OR REPLACE before shipping the command back to the server. Please make it use full qualified names (schema.name) for both function name and result types. Current search_path juggling the pg_dump does is major PITA. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Marko Kreen [EMAIL PROTECTED] writes: [ re pg_get_functiondef ] Please make it use full qualified names (schema.name) for both function name and result types. Current search_path juggling the pg_dump does is major PITA. Qualifying the function name seems like a good idea, but I'd advise against tinkering with the datatype references. It'll be hard to do correctly and it will make things very substantially uglier. Do you really want to show, eg, pg_catalog.int4 rather than integer? If you leave the backend code do what it wants to do here, the only way that there would be a problem is if someone changed their search_path in between pg_get_functiondef and trying to re-load the function definition. Which certainly ain't gonna happen for \ef, and it seems a bit implausible for any other use-case either. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
At 2008-07-15 20:28:39 +0530, [EMAIL PROTECTED] wrote: I doubt we'd consider accepting a patch done this way. Yes, it's much too ugly to live. Though I must say it would have been even MORE horrible to copy all this code into the backend to make pg_get_functiondef(), notwithstanding the extra utility of a generally-callable function. But what I'm wondering, since Gavin said he once had a working version of this patch (i.e. \ef) which he somehow lost, is how he approached the problem at the time. Gavin? Do you remember? Was it horrible? -- ams -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
Abhijit Menon-Sen [EMAIL PROTECTED] writes: Though I must say it would have been even MORE horrible to copy all this code into the backend to make pg_get_functiondef(), notwithstanding the extra utility of a generally-callable function. FWIW, I just found myself forced to invent pg_get_function_arguments() and pg_get_function_result(), because the TABLE function patch has pushed the complexity of printing function argument and result types well beyond the bounds of sanity. (Pavel had hacked up pg_dump and ignored psql's \df ...) It wouldn't take a whole lot to convince me that a pg_get_functiondef would be useful, although I don't foresee either of those applications wanting to use it because of their backward-compatibility constraints. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [PATCH] \ef function in psql
Refactoring pg_dump was more work than I had time to do right now, and I
wanted \ef to work, so I hacked up the attached (by copying dumpFunc and
its dependencies to src/bin/psql/dumpfunc.[ch]).
-- ams
*** a/src/bin/psql/Makefile
--- b/src/bin/psql/Makefile
***
*** 21,27 override CPPFLAGS := -I$(srcdir) -I$(libpq_srcdir) -I$(top_srcdir)/src/bin/pg_du
OBJS= command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
startup.o prompt.o variables.o large_obj.o print.o describe.o \
! psqlscan.o tab-complete.o mbprint.o dumputils.o $(WIN32RES)
EXTRA_OBJS = $(top_builddir)/src/backend/parser/keywords.o
--- 21,27
OBJS= command.o common.o help.o input.o stringutils.o mainloop.o copy.o \
startup.o prompt.o variables.o large_obj.o print.o describe.o \
! psqlscan.o tab-complete.o mbprint.o dumputils.o dumpfunc.o $(WIN32RES)
EXTRA_OBJS = $(top_builddir)/src/backend/parser/keywords.o
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
***
*** 38,43
--- 38,44
#include libpq-fe.h
#include pqexpbuffer.h
#include dumputils.h
+ #include dumpfunc.h
#include common.h
#include copy.h
***
*** 56,62
static backslashResult exec_command(const char *cmd,
PsqlScanState scan_state,
PQExpBuffer query_buf);
! static bool do_edit(const char *filename_arg, PQExpBuffer query_buf);
static bool do_connect(char *dbname, char *user, char *host, char *port);
static bool do_shell(const char *command);
--- 57,64
static backslashResult exec_command(const char *cmd,
PsqlScanState scan_state,
PQExpBuffer query_buf);
! static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
! bool *edited);
static bool do_connect(char *dbname, char *user, char *host, char *port);
static bool do_shell(const char *command);
***
*** 444,454 exec_command(const char *cmd,
expand_tilde(fname);
if (fname)
canonicalize_path(fname);
! status = do_edit(fname, query_buf) ? PSQL_CMD_NEWEDIT : PSQL_CMD_ERROR;
free(fname);
}
}
/* \echo and \qecho */
else if (strcmp(cmd, echo) == 0 || strcmp(cmd, qecho) == 0)
{
--- 446,521
expand_tilde(fname);
if (fname)
canonicalize_path(fname);
! if (do_edit(fname, query_buf, NULL))
! status = PSQL_CMD_NEWEDIT;
! else
! status = PSQL_CMD_ERROR;
free(fname);
}
}
+ /*
+ * \ef -- edit the named function in $EDITOR.
+ */
+
+ else if (strcmp(cmd, ef) == 0)
+ {
+ Oid foid;
+ char *func;
+
+ func = psql_scan_slash_option(scan_state, OT_WHOLE_LINE, NULL, true);
+ if (!func)
+ {
+ psql_error(no function name specified\n);
+ status = PSQL_CMD_ERROR;
+ }
+ else if (!lookup_function_oid(pset.db, func, foid))
+ {
+ psql_error(PQerrorMessage(pset.db));
+ status = PSQL_CMD_ERROR;
+ }
+ else {
+ termPQExpBuffer(query_buf);
+ if (foid)
+ {
+ char *s = create_or_replace_function_text(pset.db, foid);
+ if (s)
+ {
+ appendPQExpBufferStr(query_buf, s);
+ free(s);
+ }
+ else
+ status = PSQL_CMD_ERROR;
+ }
+ else
+ {
+ printfPQExpBuffer(query_buf,
+ CREATE FUNCTION %s%s RETURNS ... AS $$\n
+ ...\n
+ $$ LANGUAGE '...'\n,
+ func, strchr(func,'(') ? : (...) );
+ }
+ }
+
+ if (status != PSQL_CMD_ERROR)
+ {
+ bool edited = false;
+ if (!do_edit(0, query_buf, edited))
+ {
+ status = PSQL_CMD_ERROR;
+ }
+ else if (!edited)
+ {
+ printf(No changes\n);
+ }
+ else
+ {
+ status = PSQL_CMD_SEND;
+ }
+ free(func);
+ }
+ }
+
/* \echo and \qecho */
else if (strcmp(cmd, echo) == 0 || strcmp(cmd, qecho) == 0)
{
***
*** 1410,1416 editFile(const char *fname)
/* call this one */
static bool
! do_edit(const char *filename_arg, PQExpBuffer query_buf)
{
char fnametmp[MAXPGPATH];
FILE *stream = NULL;
--- 1477,1483
/* call this one */
static bool
! do_edit(const char *filename_arg, PQExpBuffer query_buf, bool *edited)
{
char fnametmp[MAXPGPATH];
FILE *stream = NULL;
***
*** 1532,1537 do_edit(const char *filename_arg, PQExpBuffer query_buf)
--- 1599,1608
psql_error(%s: %s\n, fname, strerror(errno));
error = true;
}
+ else if (edited)
+ {
+ *edited = true;
+ }
fclose(stream);
}
*** /dev/null
--- b/src/bin/psql/dumpfunc.c
***
*** 0
--- 1,496
+ #include dumpfunc.h
+
+ #include libpq-fe.h
+ #include pqexpbuffer.h
+ #include dumputils.h
+ #include common.h
+ #include catalog/pg_proc.h
+
+ #define atooid(x) ((Oid) strtoul((x), NULL, 10))
+
+ /*
+ * This function takes a function description, e.g. x or x(int), and
+ * issues a query on the given connection to retrieve the function's oid
+ * using a cast to regproc or regprocedure (as appropriate). The
Re: [HACKERS] [PATCH] \ef function in psql
Abhijit Menon-Sen [EMAIL PROTECTED] writes: Refactoring pg_dump was more work than I had time to do right now, and I wanted \ef to work, so I hacked up the attached (by copying dumpFunc and its dependencies to src/bin/psql/dumpfunc.[ch]). I doubt we'd consider accepting a patch done this way. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] \ef function in psql
At 2008-07-15 10:33:02 -0400, [EMAIL PROTECTED] wrote: I doubt we'd consider accepting a patch done this way. Yes, it's much too ugly to live. I was posting it only for the record, I should have made that clear. -- ams -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
