[PATCH] "\ef <function>" in psql

Abhijit Menon-Sen <ams@oryx.com> 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

At 2008-07-15 10:33:02 -0400, tgl@sss.pgh.pa.us 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

At 2008-07-15 20:28:39 +0530, ams@oryx.com 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

Abhijit Menon-Sen <ams@oryx.com> 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

At 2008-07-17 18:28:19 -0400, tgl@sss.pgh.pa.us 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

Abhijit Menon-Sen <ams@oryx.com> writes:

At 2008-07-17 18:28:19 -0400, tgl@sss.pgh.pa.us 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

On 7/23/08, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Abhijit Menon-Sen <ams@oryx.com> writes:

At 2008-07-17 18:28:19 -0400, tgl@sss.pgh.pa.us 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

"Marko Kreen" <markokr@gmail.com> 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

At 2008-07-23 10:50:03 -0400, tgl@sss.pgh.pa.us 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

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 <tgl@sss.pgh.pa.us> wrote:

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 <tgl@sss.pgh.pa.us> wrote:

+1 for CREATE OR REPLACE

...Robert

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 <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

At 2008-07-29 15:42:27 +0530, ams@oryx.com 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

I have attached two patches:

- funcdef.diff implements pg_get_functiondef()
- edit.diff implements "\ef function" in psql based on (1).

Comments appreciated.

-- ams

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

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 <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Abhijit Menon-Sen <ams@oryx.com> 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

On Sat, Sep 6, 2008 at 10:10 AM, Tom Lane <tgl@sss.pgh.pa.us> 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