Thread: Need feedback on new feature (\for)

Need feedback on new feature (\for)

From
Martijn van Oosterhout
Date:
I've had some fun in the past where I've had to grant a lot of tables and
other similar system commands. Unfortunatly, you can't use queries to fill
in fields for you. Anyway, I've implemented a patch which allows the
following:

grant select on ":2" to ":1"
\for select usename, relname from pg_catalog.pg_user, pg_catalog.pg_class where relname not like 'pg_%';

Produces:

Expanded to: grant select on "test" to "postgres"
Expanded to: grant select on "test2" to "postgres"
[etc]

Anyway, I'm sure you can come up with all sorts of uses. It occurs to me to
be a way to produce a whole lot of commands to create very similar triggers
based on a list of tables. And other commands to remove them again. My
questions are as follows:

1. Due to the way that backslash commands are implemented, the entire second
query has to be on one line. I don't see a good way to deal with this.
2. If the results being substituted contain quotes, they might not
substitute cleanly. Do we care?
3. Do people even want something like this?
4. Should it list the commands being executed?
5. What should happen upon an error in the generated query?
6. Maybe \foreach is clearer?
7. There should probably be a test mode to just dump the queries without
executing.

Thoughts welcome.

Preliminary patch at:
http://svana.org/kleptog/pgsql/forloop1.patch

Anonymous CVS access isn't working for me so it's not against the latest,
hopefully it's not too changed. It's somewhere around 7.3.2.

Have a nice day,
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> "All that is needed for the forces of evil to triumph is for enough good
> men to do nothing." - Edmond Burke
> "The penalty good people pay for not being interested in politics is to be
> governed by people worse than themselves." - Plato

Attachment

Re: Need feedback on new feature (\for)

From
Karsten Hilbert
Date:
> 3. Do people even want something like this?
yes

> 4. Should it list the commands being executed?
perhaps just in not("\set export 1") mode ?

> 5. What should happen upon an error in the generated query?
IMHO all the generated queries should be in one transaction

> 7. There should probably be a test mode to just dump the queries without
> executing.
Yes.

> Thoughts welcome.
Thanks.

Karsten
--
GPG key ID E4071346 @ wwwkeys.pgp.net
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346

Re: Need feedback on new feature (\for)

From
Tom Lane
Date:
Martijn van Oosterhout <kleptog@svana.org> writes:
> grant select on ":2" to ":1"=20
> \for select usename, relname from pg_catalog.pg_user, pg_catalog.pg_class w=
> here relname not like 'pg_%';

> Thoughts welcome.

Interesting but it seems awfully messy as-is.  How about something like

\for
... control query here ...
\do
... one or more queries here ...
\done

This would eliminate the single-line restriction on the control query
and also allow more than one query in the loop body.  I don't have a
clear idea of what it would take to implement, but I'm visualizing
the \for and \do commands as setting flags that would prevent queries
from actually being sent to the backend; they'd just get stacked up in
a pair of buffers.  Then \done executes the loop and resets the flags.

> 2. If the results being substituted contain quotes, they might not
> substitute cleanly. Do we care?

Yes.  I would argue that the style of substitution you propose is all
wrong.  The substituter should not look inside single- or double-quoted
literals --- writing colon in a literal shouldn't become fraught with peril.
Rather, substitute for :n when it appears outside any quotes, and let
the substituted value include the needed quotes.
Maybe "for" could include some specification of the expected quoting
style, along the lines of
    \for string,string,number
"string" would imply that the corresponding :n symbol is replaced by
a correctly single-quoted literal; perhaps "name" to replace by a
correctly double-quoted literal; "number" to just substitute exactly
what comes back from the query.  (These choices of names could probably
be improved upon, but you get the idea --- sort of a weak form of
declaring datatypes for the parameters.)

> 4. Should it list the commands being executed?

Not by default, but I like the idea of a test mode.

> 5. What should happen upon an error in the generated query?

Abort the loop.

            regards, tom lane

Re: Need feedback on new feature (\for)

From
Karsten Hilbert
Date:
> > 5. What should happen upon an error in the generated query?
> IMHO all the generated queries should be in one transaction
which should get aborted, of course

Karsten
--
GPG key ID E4071346 @ wwwkeys.pgp.net
E167 67FD A291 2BEA 73BD  4537 78B9 A9F9 E407 1346

Re: Need feedback on new feature (\for)

From
Scott Lamb
Date:
Martijn van Oosterhout wrote:

> I've had some fun in the past where I've had to grant a lot of tables and
> other similar system commands. Unfortunatly, you can't use queries to fill
> in fields for you. Anyway, I've implemented a patch which allows the
> following:
>
> grant select on ":2" to ":1"
> \for select usename, relname from pg_catalog.pg_user, pg_catalog.pg_class where relname not like 'pg_%';

That's definitely a useful thing to do, but I'm not sure I like your
syntax. As someone else mentioned, the ":2" is confusing; it's like a
bind variable, but isn't. And real bind variables don't work, as you are
substituting identifiers, not literals.

You're not completely out in the cold doing something like this without
a patch. Right now, I believe you can do something like (in Oracle
PL/SQL-ish syntax; it's more familiar to me):

     declare
         grantcursor cursor as
         select    usename, relname
         from      pg_catalog.pg_user, pg_catalog.pg_class
         where     relname not like 'pg_%';
     begin
         for grantline in grantcursor loop
             execute immediate 'grant select on '
                 || quoteident(grantline.relname)
                 || ' to ' || quoteident(grantline.usename) || '"';
         end loop;
     end;

(I'm not sure how to do an anonymous plpgsql block. Anyone?)

This is more wordy, but should work.

Scott


Re: Need feedback on new feature (\for)

From
Martijn van Oosterhout
Date:
On Sun, Aug 17, 2003 at 02:04:03PM -0600, Scott Lamb wrote:
> Martijn van Oosterhout wrote:
> >grant select on ":2" to ":1"
> >\for select usename, relname from pg_catalog.pg_user, pg_catalog.pg_class
> >where relname not like 'pg_%';
>
> That's definitely a useful thing to do, but I'm not sure I like your
> syntax. As someone else mentioned, the ":2" is confusing; it's like a
> bind variable, but isn't. And real bind variables don't work, as you are
> substituting identifiers, not literals.
>
> You're not completely out in the cold doing something like this without
> a patch. Right now, I believe you can do something like (in Oracle
> PL/SQL-ish syntax; it's more familiar to me):

Hmm, I didn't know you could execute pl/sql from the prompt like that.
Still, I was looking for something that was short and easy to type. Not to
mention something I can remember :)
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> "All that is needed for the forces of evil to triumph is for enough good
> men to do nothing." - Edmond Burke
> "The penalty good people pay for not being interested in politics is to be
> governed by people worse than themselves." - Plato

Attachment

Re: Need feedback on new feature (\for)

From
Martijn van Oosterhout
Date:
On Sun, Aug 17, 2003 at 12:40:56PM -0400, Tom Lane wrote:
> Martijn van Oosterhout <kleptog@svana.org> writes:
> > grant select on ":2" to ":1"=20
> > \for select usename, relname from pg_catalog.pg_user, pg_catalog.pg_class w=
> > here relname not like 'pg_%';
>
> > Thoughts welcome.
>
> Interesting but it seems awfully messy as-is.  How about something like
>
> \for
> ... control query here ...
> \do
> ... one or more queries here ...
> \done

Indeed, I thought of this after I'd turned my machine off. You could do it
by keeping some buffers in the background. You'd need to indicate what
context you're in.

> This would eliminate the single-line restriction on the control query
> and also allow more than one query in the loop body.  I don't have a
> clear idea of what it would take to implement, but I'm visualizing
> the \for and \do commands as setting flags that would prevent queries
> from actually being sent to the backend; they'd just get stacked up in
> a pair of buffers.  Then \done executes the loop and resets the flags.

Something like that.

> > 2. If the results being substituted contain quotes, they might not
> > substitute cleanly. Do we care?
>
> Yes.  I would argue that the style of substitution you propose is all
> wrong.  The substituter should not look inside single- or double-quoted
> literals --- writing colon in a literal shouldn't become fraught with peril.
> Rather, substitute for :n when it appears outside any quotes, and let
> the substituted value include the needed quotes.
> Maybe "for" could include some specification of the expected quoting
> style, along the lines of
>     \for string,string,number
> "string" would imply that the corresponding :n symbol is replaced by
> a correctly single-quoted literal; perhaps "name" to replace by a
> correctly double-quoted literal; "number" to just substitute exactly
> what comes back from the query.  (These choices of names could probably
> be improved upon, but you get the idea --- sort of a weak form of
> declaring datatypes for the parameters.)

Yes, I see that but it also limits what you could do. For example:

\for
select oid from <some funky query here>
\do
drop constraint "RI_ConstraintTrigger_:1"
\done

(Actually, the clash of \do and \done with the ordinary \d commands will get
very irritating. Need better names.) I was actually leaning the other way,
always substitute but add escapes if inside a string. Incidently, the above
case could be handled by performing the concatintation in the query.

Alternatively, allow you to name the variables but then you get a parsing
problem. If the variable is "var", do you substitute :variable? How do you
choose the other style (like the shell has $hello and ${h}ello).

I'd could probably live with being strict and require you to do all your
trickery in the control query. It will look a little strange if you do:

\for
select tablename, '<begin code>' || field || 'end code' from <blah>
\do
create trigger on table :1 as :2
\done

But it may be worth it for the robustness provided. It's not like we are
conforming to any standard here.

> > 4. Should it list the commands being executed?
>
> Not by default, but I like the idea of a test mode.

OK

> > 5. What should happen upon an error in the generated query?
>
> Abort the loop.

OK

Thanks for your ideas.
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> "All that is needed for the forces of evil to triumph is for enough good
> men to do nothing." - Edmond Burke
> "The penalty good people pay for not being interested in politics is to be
> governed by people worse than themselves." - Plato

Attachment

Re: Need feedback on new feature (\for)

From
Tom Lane
Date:
I said:
> ... I think it makes much more sense to push the
> complexity into the control query and keep the substitution rules
> non-surprising.

BTW, taking that approach as gospel, we could forget my idea of
declaring datatypes for the control-query results.  Just substitute
'em literally.  The quoting transformations could be done easily in
the control query with quote_literal() or quote_ident().

            regards, tom lane

Re: Need feedback on new feature (\for)

From
Tom Lane
Date:
Martijn van Oosterhout <kleptog@svana.org> writes:
> Yes, I see that but it also limits what you could do. For example:

> \for
> select oid from <some funky query here>
> \do
> drop constraint "RI_ConstraintTrigger_:1"
> \done

> .. Incidently, the above
> case could be handled by performing the concatintation in the query.

Yes.  I'd argue that that's a much cleaner approach.  I really dislike
the notion of doing substitutions inside quoted strings.  (You'll notice
that psql does not do that now for :name variables --- I think that
these substitutions should follow the exact same rules.)

In general, you can do any sort of calculation on the strings with ease
in the control query, while the substituter in psql will necessarily be
pretty stupid.  So I think it makes much more sense to push the
complexity into the control query and keep the substitution rules
non-surprising.

            regards, tom lane

Re: Need feedback on new feature (\for)

From
Scott Lamb
Date:
Martijn van Oosterhout wrote:

> On Sun, Aug 17, 2003 at 02:04:03PM -0600, Scott Lamb wrote:
>
>>Martijn van Oosterhout wrote:
>>
>>>grant select on ":2" to ":1"
>>>\for select usename, relname from pg_catalog.pg_user, pg_catalog.pg_class
>>>where relname not like 'pg_%';
>>
>>That's definitely a useful thing to do, but I'm not sure I like your
>>syntax. As someone else mentioned, the ":2" is confusing; it's like a
>>bind variable, but isn't. And real bind variables don't work, as you are
>>substituting identifiers, not literals.
>>
>>You're not completely out in the cold doing something like this without
>>a patch. Right now, I believe you can do something like (in Oracle
>>PL/SQL-ish syntax; it's more familiar to me):
>
>
> Hmm, I didn't know you could execute pl/sql from the prompt like that.
> Still, I was looking for something that was short and easy to type. Not to
> mention something I can remember :)

You can in Oracle, so I can assume you can in PostgreSQL, too, but the
exact syntax escapes me.

I understand what you mean about short and easy to type. As far as
memory, I have to remember the other way anyway (at least in Oracle) for
  triggers in such.


Re: Need feedback on new feature (\for)

From
Stephan Szabo
Date:
On Mon, 18 Aug 2003, Scott Lamb wrote:

> Martijn van Oosterhout wrote:
>
> > On Sun, Aug 17, 2003 at 02:04:03PM -0600, Scott Lamb wrote:
> >
> >>Martijn van Oosterhout wrote:
> >>
> >>>grant select on ":2" to ":1"
> >>>\for select usename, relname from pg_catalog.pg_user, pg_catalog.pg_class
> >>>where relname not like 'pg_%';
> >>
> >>That's definitely a useful thing to do, but I'm not sure I like your
> >>syntax. As someone else mentioned, the ":2" is confusing; it's like a
> >>bind variable, but isn't. And real bind variables don't work, as you are
> >>substituting identifiers, not literals.
> >>
> >>You're not completely out in the cold doing something like this without
> >>a patch. Right now, I believe you can do something like (in Oracle
> >>PL/SQL-ish syntax; it's more familiar to me):
> >
> >
> > Hmm, I didn't know you could execute pl/sql from the prompt like that.
> > Still, I was looking for something that was short and easy to type. Not to
> > mention something I can remember :)
>
> You can in Oracle, so I can assume you can in PostgreSQL, too, but the
> exact syntax escapes me.

AFAIK there isn't a way to do that currently apart from generating a
function, calling it, and dropping it.


Re: Need feedback on new feature (\for)

From
Bruce Momjian
Date:
Tom Lane wrote:
> I said:
> > ... I think it makes much more sense to push the
> > complexity into the control query and keep the substitution rules
> > non-surprising.
>
> BTW, taking that approach as gospel, we could forget my idea of
> declaring datatypes for the control-query results.  Just substitute
> 'em literally.  The quoting transformations could be done easily in
> the control query with quote_literal() or quote_ident().

Has the patch author looked at how psql variables adds quotes, usually
like:

    "\"":var"\""

It is ugly, but it seems like the clearest way.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: Need feedback on new feature (\for)

From
Martijn van Oosterhout
Date:
On Sun, Aug 24, 2003 at 09:29:05PM -0400, Bruce Momjian wrote:
> Tom Lane wrote:
> > I said:
> > > ... I think it makes much more sense to push the
> > > complexity into the control query and keep the substitution rules
> > > non-surprising.
> >
> > BTW, taking that approach as gospel, we could forget my idea of
> > declaring datatypes for the control-query results.  Just substitute
> > 'em literally.  The quoting transformations could be done easily in
> > the control query with quote_literal() or quote_ident().
>
> Has the patch author looked at how psql variables adds quotes, usually
> like:
>
>     "\"":var"\""
>
> It is ugly, but it seems like the clearest way.

That's another solution. I've been a bit short of time recently but at some
stage I'm going to rework it to take some of the suggestions people made.
--
Martijn van Oosterhout   <kleptog@svana.org>   http://svana.org/kleptog/
> "All that is needed for the forces of evil to triumph is for enough good
> men to do nothing." - Edmond Burke
> "The penalty good people pay for not being interested in politics is to be
> governed by people worse than themselves." - Plato

Attachment