Sunday, June 15, 2008

Re: [HACKERS] psql: \edit-function

Abhijit Menon-Sen <ams@oryx.com> writes:
> The problem is, of course, generating the "CREATE OR REPLACE" statement.
> There is some code to do this in pg_dump.c:dumpFunc(), but it's strongly
> tied to pg_dump (global variables, output to Archive *, dependencies on
> other functions, etc.).

> I could either try to duplicate this code (and there's a lot of it), or
> rip dumpFunc() and its dependencies out of pg_dump into dumpfunc.c and
> make it callable both by pg_dump and psql. I've done some work towards
> the latter, so I know it's possible, but it's a lot of work, which I
> don't want to do if it won't be accepted anyway.

There's been a lot of talk (but no action) about refactoring pg_dump
into a library plus wrapper. ISTM you are proposing to do that for one
single bit of functionality that you've chosen at random, which somehow
doesn't seem like a good way to go at things. I'd much rather see that
tackled in a holistic fashion.

An alternative that would be worth considering is to implement a
--function switch for pg_dump (in itself a much-requested feature)
and then have psql execute "pg_dump --function whatever" as a subprocess
to obtain the initial function definition. But I'm not sure how to pass
down the database connection parameters (in particular I see no safe way
to pass down a password). Efficiency might be a problem too, since
pg_dump generally sucks out the entire database schema before worrying
about how much of it is actually needed.

Another line of thought is to implement a pg_get_functiondef() function
on the backend side and use that. I think this wouldn't help much for
pg_dump, since it has to be able to work against old backends, so you'd
still be looking at duplicating code for the foreseeable future :-(
But it would make the functionality easily available to other clients,
and there's something to be said for that.

regards, tom lane

PS: two trivial stylistic gripes:

> -static bool do_edit(const char *filename_arg, PQExpBuffer query_buf);
> +static bool do_edit(const char *, PQExpBuffer, bool *);

Do not remove parameter names from function prototypes. Whoever taught
you that that is good style was frightfully mistaken. (What happens
when you have multiple parameters of the same type? Without names, the
prototype becomes useless to readers.)

> - status = do_edit(fname, query_buf) ? PSQL_CMD_NEWEDIT : PSQL_CMD_ERROR;
> + if (do_edit(fname, query_buf, 0))

"0" is not the preferred spelling of bool "false".

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

No comments: