Re: [PATCHES] psql command aliases support
Bernd Helmle [EMAIL PROTECTED] writes:
please find attached a patch which implements psql command aliases. They
work the same way as on bash, zsh and others, for example:
I'm still not convinced that we want this sort of feature at all.
I quote from the current bash manual page:
ALIASES
...
The rules concerning the definition and use of aliases are somewhat
confusing.
...
For almost every purpose, aliases are superseded by shell functions.
The bash authors seem to wish they'd never adopted the feature, which
makes me wonder whether we really want to copy it slavishly.
Having said that, though, here are some code-review points:
* The patch intends that the expansion of an alias could be either a
backslash command or SQL command text, but I don't think you've thought
through the interactions of these cases. In particular, the patch doesn't
seem to behave very sanely if the backslash command comes when there is
already text on the current line or previous lines of the query buffer:
it just wipes that text out, which is surely not the desirable thing.
* Use of a VariableSpace for the stack of aliases-being-expanded seems
exceedingly klugy: variable spaces are intended for persistent storage
not transient state, so this fails to satisfy the principle of least
astonishment for readers of the code. Also, you're failing to manipulate
it as a stack: you shouldn't wipe a stack entry upon matching it, only
when control returns from having expanded it. Another problem is that
it doesn't get reset at all if the alias expansion is just SQL text and
not another backslash command, meaning successive uses of such an alias
won't work. I think you'd be better off if the expansion were done as a
separate recursive routine with the set of active aliases just passed as
a list threaded through local variables of the routine invocations.
* I think you should lose the separate PSQL_CMD_ALIAS status code and
just use PSQL_CMD_NEWEDIT. The one place where the codes act different
looks like a mistake to me anyway.
* Don't feed non-constant strings through gettext():
+ if (cmd)
+ {
+ puts(_(cmd));
+ success = true;
+ }
The best you can hope for is that it doesn't do anything.
regards, tom lane
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
--On Donnerstag, April 03, 2008 03:13:47 +0100 Gregory Stark [EMAIL PROTECTED] wrote: I think you're crazy to think shells are more likely to have conflicts. Shells require a whole token match, not just the first letter. In other words, any alias *starting with* the letters c, d, e, f, g, h, i, o, s, w, z would be a conflict. Just for maximum confusion the list of letters which cause conflicts when capitalized would be entirely different. Picture a newbie typoing on their \old alias and trying to figure out where all their data is going... Hopefully they weren't too attached to whatever was in their ldd file yesterday. Of course, the patch doesn't work this way. Only complete tokens delivered by the parser are substituted, illustrated here with your example: #= \alias old SELECT version(); #= \old version --- PostgreSQL 8.4devel on x86_64-unknown-linux-gnu, compiled by GCC gcc (GCC) 4.2.3 (Debian 4.2.3-2) (1 row) #= \o foo #= \old #= zsh: suspended psql % cat foo version --- PostgreSQL 8.4devel on x86_64-unknown-linux-gnu, compiled by GCC gcc (GCC) 4.2.3 (Debian 4.2.3-2) (1 row) -- Thanks Bernd -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
Bernd Helmle [EMAIL PROTECTED] writes: Picture a newbie typoing on their \old alias and trying to figure out where all their data is going... Hopefully they weren't too attached to whatever was in their ldd file yesterday. Of course, the patch doesn't work this way. Only complete tokens delivered by the parser are substituted, illustrated here with your example: To be more explicit what I meant was someone doing #= \alias old select version(); ... #= \oldd oops #= \old #= select 'where is all my output going?' #= select 'what happened to my ldd file?' -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
Am Donnerstag, 3. April 2008 schrieb Gregory Stark: To be more explicit what I meant was someone doing #= \alias old select version(); ... #= \oldd oops #= \old #= select 'where is all my output going?' #= select 'what happened to my ldd file?' This is a valid concern, but it is orthogonal to the alias feature. You have the same problem already if you mistype \oo instead of \o \ofoo instead of \obar \o instead of \p \oset instead of \pset or even more amusingly \o foo instead of \i foo -- check your keyboard layout and so on. If you want to guard against typos, you need to remove the \o command in its current form. Prohibiting the addition of new commands, by whichever means, is not going to help. And we have never had a policy or real analysis whether new or existing commands are typo-proof. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
Peter Eisentraut [EMAIL PROTECTED] writes: This is a valid concern, but it is orthogonal to the alias feature. You have the same problem already if you mistype \oo instead of \o \ofoo instead of \obar Not really. In these cases you know what \o is going to do, you've just typo'd the filename. The case I was saying was a conflict was because you were using an entirely different feature and might not never have even met \o. \o instead of \p \oset instead of \pset Sure, if you typo \o instead of select * from pg_class you might be surprised too. You can't protect against typing an entirely different command than intended. At least you can then go look up what \o does and figure out what happened. If you type \ofoo instead of \obar I think there's a big difference between having it accidentally do what you wanted it to do but to the wrong file and having it do something entirely unrelated to what you intended and end up overwriting a file instead of run a simple select. or even more amusingly \o foo instead of \i foo -- check your keyboard layout The point is here you've typed a different command entirely. Unsurprisingly it's going to do something different. \old means something *today*. In the proposed syntax by creating the alias you're changing what it means. You're not changing other \ofoo arguments though which is where the possibility for confusion arises. Consider instead if Debian decided to include a convenient \deb alias for a select query against the package repository. Now if I happen to have an eb table I will be surprised when \deb doesn't work. And lengthening the alias doesn't really help (aside from defeating the purpose of having aliases). If they define \debian they would still be interfering if I should have a table named ebian. As rule of thumb, I think if you try to execute an alias which doesn't exist, you should get an alias does not exist command. You should not get an Invalid command nor Did not find relation and certainly not some random command you've never met before being run reading or overwriting random files. I repeat my suggestion of having a \query command so you could do: \setquery deb select * from debian_packages where packagename like :1 \setquery bbdb select * from bbdb where name like :1 \query deb postgres \query bbdb peter to run a saved query. I'm not attached to setquery though. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's RemoteDBA services! -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
Am Donnerstag, 3. April 2008 schrieb Gregory Stark: \old means something *today*. In the proposed syntax by creating the alias you're changing what it means. Yes. There are two complementary responses to that: 1. That's an intentional, useful feature of aliases. 2. If you don't like it, don't use it. By your line of argument, we couldn't ever add any new commands in psql anyway. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Am Donnerstag, 3. April 2008 schrieb Gregory Stark: #= \oldd #= \old #= select 'where is all my output going?' #= select 'what happened to my ldd file?' psql allows you to omit the space between the command and argument? Does anybody else find that weird? What is the virtue of allowing such a syntax in the first place? I can't think of any other context where it's okay to issue a command together with arguments without some kind of delimiter, for the obvious reason that it introduces ambiguities such as those Greg described. Cheers, BJ -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.7 (GNU/Linux) Comment: http://getfiregpg.org iD8DBQFH9OWk5YBsbHkuyV0RAmZ4AKC9qjU+KqgLJxQSJ4sD7X4YaPEbHgCg8ipW BRedeq4kG/FpqZBoptrKlRw= =out8 -END PGP SIGNATURE- -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
Am Donnerstag, 3. April 2008 schrieb Brendan Jurd: psql allows you to omit the space between the command and argument? Does anybody else find that weird? What is the virtue of allowing such a syntax in the first place? Combatability with hysterial practice. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
--On Freitag, April 04, 2008 01:11:51 +1100 Brendan Jurd [EMAIL PROTECTED] wrote: What is the virtue of allowing such a syntax in the first place? I can't think of any other context where it's okay to issue a command together with arguments without some kind of delimiter, for the obvious reason that it introduces ambiguities such as those Greg described. Yeah, i always felt very uncomfortable with this behavior. Anyways, it's too late to complain about that and it shouldn't block new features which can increase usability on the other side. -- Thanks Bernd -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
--On Donnerstag, April 03, 2008 14:36:59 +0100 Gregory Stark [EMAIL PROTECTED] wrote: \o foo instead of \i foo -- check your keyboard layout The point is here you've typed a different command entirely. Unsurprisingly it's going to do something different. Uh well, you surely don't know that if you accidently hit o instead of ione reason more for an alias \output and \input ;) To be serious, i take your point, but i'm under the impression that you simply can't safe the user against all mistakes they are going to make ever. Your concerns apply to every existing alias implementation as well and you aren't safe against decisions of your distributor to add conflicting aliases to their default bashrc as well. -- Thanks Bernd -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
Bernd Helmle [EMAIL PROTECTED] writes: --On Donnerstag, April 03, 2008 14:36:59 +0100 Gregory Stark [EMAIL PROTECTED] wrote: \o foo instead of \i foo -- check your keyboard layout The point is here you've typed a different command entirely. Unsurprisingly it's going to do something different. Uh well, you surely don't know that if you accidently hit o instead of ione reason more for an alias \output and \input ;) To be serious, i take your point, but i'm under the impression that you simply can't safe the user against all mistakes they are going to make ever. Your concerns apply to every existing alias implementation as well and you aren't safe against decisions of your distributor to add conflicting aliases to their default bashrc as well. I think I'm not managing to express myself clearly here. Generalizing the complaint into simply if you typo something different happens is losing too much information. With shell aliases if you typo foo into bar then, yes, something different will happen than what you expected. However the new behaviour will still be of the same kind -- that is it runs a shell command, just the wrong shell command. If there's no command by that name you get an error not some unrelated shell functionality. In the proposed alias syntax if you typo \cold into \coldd it will print a mysterious error about trying to connect to a database or if you typo \old into \oldd it will write out a file and stop printing query output. The user might not even realize that \c and \o are commands since they think they're running commands \colld and \oldd. I think you have to find a syntax where the current commands continue to mean exactly what they always meant and where typos can't result in an entirely different kind of behaviour. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's 24x7 Postgres support! -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
Gregory Stark [EMAIL PROTECTED] writes: I think you have to find a syntax where the current commands continue to mean exactly what they always meant and where typos can't result in an entirely different kind of behaviour. Yeah, the fundamental difference between the backslash command situation and aliases in shells and suchlike is that, because we've historically allowed no space between command name and argument, it's not that easy to tell what string ought to be compared against alias names. I think that an alias facility would only be acceptably safe if we disallowed that syntax (ie, start to *require* a space between command and args). Are we ready to do that? This discussion really needs to be moved to someplace more widely read than -patches, btw. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
On Thu, Apr 03, 2008 at 01:19:21PM -0400, Tom Lane wrote: Gregory Stark [EMAIL PROTECTED] writes: I think you have to find a syntax where the current commands continue to mean exactly what they always meant and where typos can't result in an entirely different kind of behaviour. Yeah, the fundamental difference between the backslash command situation and aliases in shells and suchlike is that, because we've historically allowed no space between command name and argument, it's not that easy to tell what string ought to be compared against alias names. I think that an alias facility would only be acceptably safe if we disallowed that syntax (ie, start to *require* a space between command and args). Are we ready to do that? +1 -dg -- David Gould [EMAIL PROTECTED] 510 536 1443510 282 0869 If simplicity worked, the world would be overrun with insects. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
Peter Eisentraut [EMAIL PROTECTED] writes: Am Dienstag, 1. April 2008 schrieb Tom Lane: Do we really want such a thing? Yes! The space of backslash command names is so densely populated already that it's hard to imagine creating aliases without conflicting with existing (or future) command names --- as indeed your example does. It seems like mostly a recipe for confusion. This is a standard feature and effect on shells. Shells have even more likely commands and conflicts, and still aliases are widely used. If people are concerned about conflicts, they shouldn't use aliases. I think you're crazy to think shells are more likely to have conflicts. Shells require a whole token match, not just the first letter. In other words, any alias *starting with* the letters c, d, e, f, g, h, i, o, s, w, z would be a conflict. Just for maximum confusion the list of letters which cause conflicts when capitalized would be entirely different. Picture a newbie typoing on their \old alias and trying to figure out where all their data is going... Hopefully they weren't too attached to whatever was in their ldd file yesterday. I could see it being handy being able to save commonly executed queries and access them with a shortcut but I think you need to use a separate namespace from psql's backslash commands. Perhaps `query` or invent a single backslash command to execute them like \query current_query. Actually I like that last idea the most. One thing to think about is how to pass arguments to the aliases. Csh put us in the soup by hacking in arguments in a terrible way. As a result quoting in csh aliases is awful. Even if it's not implemented right away I think we should figure out what the syntax would be for passing arguments to be interpolated into the query before backing ourselves into a corner. I can't imagine much of a use case for being able to alias backslash commands themselves. They're already ridiculously terse anyways. How many keystrokes can you possibly save? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] psql command aliases support
Folks,
please find attached a patch which implements psql command aliases. They
work the same way as on bash, zsh and others, for example:
#= \alias d \dt+
#= \d
List of relations
Schema | Name | Type | Owner | Description
+--+---+---+-
public | foo | table | bernd |
(1 row)
=# \alias current_query SELECT current_query, NOW() - query_start FROM
pg_stat_activity WHERE current_query NOT LIKE 'IDLE%';
#= \current_query
current_query
| ?column?
---+--
SELECT current_query, NOW() - query_start FROM pg_stat_activity WHERE
current_query NOT LIKE 'IDLE%'; | 00:00:00
(1 row)
#= \unalias d
#= \d
List of relations
Schema | Name | Type | Owner
+--+---+---
public | foo | table | bernd
(1 row)
I hope i broke nothing and maybe we find this useful for 8.4, documentation
included.
--
Thanks
BerndIndex: doc/src/sgml/ref/psql-ref.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v
retrieving revision 1.199
diff -c -B -r1.199 psql-ref.sgml
*** doc/src/sgml/ref/psql-ref.sgml 30 Mar 2008 18:10:20 - 1.199
--- doc/src/sgml/ref/psql-ref.sgml 1 Apr 2008 14:05:05 -
***
*** 685,690
--- 685,708
/varlistentry
varlistentry
+ termliteral\alias replaceable class=parameteraliasname/replaceable replaceable class=parameteraliascommand/replaceable/literal/term
+ listitem
+ para
+ The literal\alias/literal backslash command allows defining your own backslash commands or an alias for an existing built-in command. The alias is allowed to call a SQL command or any other backslash command or aliases already defined. For example:
+ programlisting
+ =gt; userinput\alias watch_idle_tx SELECT xact_start, datid, datname, procpid, usesysid FROM pg_stat_activity WHERE current_query LIKE '%IDLE%';/userinput
+ =gt; userinput\watch_idle_tx/userinput
+ xact_start | datid | datname | procpid | usesysid
+ ---+---+-+-+--
+ 2008-03-07 14:59:25.618827+01 | 24576 | db | 28864 | 10
+ (1 row)
+ /programlisting
+ Calling command\alias/command without an replaceable class=parameteraliasname/replaceable just shows a list of all defined aliases, with an existing replaceable class=parameteraliasname/replaceable its assigned replaceable class=parameteraliascommand/replaceable is shown. You can use the command\unalias/command to remove an alias.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termliteral\cd [ replaceabledirectory/replaceable ]/literal/term
listitem
para
***
*** 1849,1854
--- 1867,1880
/listitem
/varlistentry
+ varlistentry
+ termliteral\unalias replaceable class=parameteraliascmd/replaceable/literal/term
+ listitem
+ para
+ Removes the specified alias from the list.
+ /para
+ /listitem
+ /varlistentry
varlistentry
termliteral\w/literal {replaceable class=parameterfilename/replaceable | replaceable class=parameter|command/replaceable}/term
Index: src/bin/psql/command.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.186
diff -c -B -r1.186 command.c
*** src/bin/psql/command.c 1 Jan 2008 19:45:55 - 1.186
--- src/bin/psql/command.c 1 Apr 2008 14:05:06 -
***
*** 199,204
--- 199,251
backslashResult status = PSQL_CMD_SKIP_LINE;
/*
+ * We want to look for aliases first, since we want them to override a
+ * built-in command, so our users are able to define their own behavior
+ * on their favourite backslash command.
+ */
+ const char *aliascmd = GetVariable(pset.aliases, cmd);
+
+ if (aliascmd)
+ {
+ /* Save the current alias substitution string.
+ * We do this to prevent infinite recursion into the aliases
+ * list.
+ */
+ const char *substalias = GetVariable(pset.substaliases, cmd);
+
+ if (substalias != NULL)
+ {
+ /* already substituted, nothing more to do */
+ DeleteVariable(pset.substaliases, cmd);
+ }
+ else
+ {
+ char *opt;
+ SetVariable(pset.substaliases, cmd, 1);
+
+ /* save the substituted command to the query buffer but save
+ * additional parameter lists also
+ */
+ resetPQExpBuffer(query_buf);
+ appendPQExpBuffer(query_buf, aliascmd);
+
+ if ((opt = psql_scan_slash_option(scan_state,
+ OT_WHOLE_LINE, NULL, false)))
+ {
+ appendPQExpBuffer(query_buf, );
+ appendPQExpBuffer(query_buf, opt);
+ }
+
+ success =
Re: [PATCHES] psql command aliases support
Bernd Helmle [EMAIL PROTECTED] writes: please find attached a patch which implements psql command aliases. They work the same way as on bash, zsh and others, for example: #= \alias d \dt+ #= \d Do we really want such a thing? The space of backslash command names is so densely populated already that it's hard to imagine creating aliases without conflicting with existing (or future) command names --- as indeed your example does. It seems like mostly a recipe for confusion. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
Am Dienstag, 1. April 2008 schrieb Tom Lane: Do we really want such a thing? Yes! The space of backslash command names is so densely populated already that it's hard to imagine creating aliases without conflicting with existing (or future) command names --- as indeed your example does. It seems like mostly a recipe for confusion. This is a standard feature and effect on shells. Shells have even more likely commands and conflicts, and still aliases are widely used. If people are concerned about conflicts, they shouldn't use aliases. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] psql command aliases support
--On Dienstag, April 01, 2008 11:39:59 -0400 Tom Lane [EMAIL PROTECTED] wrote: Do we really want such a thing? Well, i use aliases everytime and everywhere they got implemented and i found it quite useful to _extend_ existing behavior (integrating additional functionality in an easy way) The space of backslash command names is so densely populated already that it's hard to imagine creating aliases without conflicting with existing (or future) command names I often found existing backslash command sometimes overloaded or simply not providing information i really need (for example, an easy way to get information about current locales, encoding and user settings). You simply can't catch all requirements DBA's and users want within a all-catching implementation. Using this way, users are able to implement their own command shortcuts Overriding existing backslash commands (as my first example shows) is only an implementation-specific detail which could easily forbidden. However, defining your own shortcuts for your psql-sessions looks quite useful to me, like my 2nd example tries to illustrate. It seems like mostly a recipe for confusion. So what? This could happen in every shell that supports aliases as well. I don't get your point...? -- Thanks Bernd -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
