-----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+KqgLJxQSJ4sD7X 4YaPEbHgCg8ipW
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

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

--On Freitag, April 04, 2008 01:11:51 +1100 Brendan Jurd
<direvus@gmail.com> 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

--On Donnerstag, April 03, 2008 14:36:59 +0100 Gregory Stark
<stark@enterprisedb.com> 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
i....one 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

"Bernd Helmle" <mailings@oopsware.de> writes:

> --On Donnerstag, April 03, 2008 14:36:59 +0100 Gregory Stark
> <stark@enterprisedb.com> 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 i....one
> 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

Gregory Stark <stark@enterprisedb.com> 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

On Thu, Apr 03, 2008 at 01:19:21PM -0400, Tom Lane wrote:
> Gregory Stark <stark@enterprisedb.com> 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 daveg@sonic.net 510 536 1443 510 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

Bernd Helmle <mailings@oopsware.de> 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