Обсуждение: Improved \df(+) in psql + backward-compatibility
Folks, I've noticed that \df doesn't do quite what it might when a function is created with named input parameters. Please find enclosed a patch against CVS TIP that does this better. On a slightly related note, I've noticed that psql isn't backward compatible. I know that it's *very* late to be introducing things, but I consider this a bug, and would like to send in more fixes. What do you all think? Cheers, D -- David Fetter david@fetter.org http://fetter.org/ phone: +1 510 893 6100 mobile: +1 415 235 3778 Remember to vote!
Вложения
David Fetter <david@fetter.org> writes:
> On a slightly related note, I've noticed that psql isn't backward
> compatible.
We have never expected psql's \d commands to work against older server
versions, and two months after feature freeze isn't the time to start
making that happen.
regards, tom lane
On Monday 29 August 2005 00:33, Tom Lane wrote:
> David Fetter <david@fetter.org> writes:
> > On a slightly related note, I've noticed that psql isn't backward
> > compatible.
>
> We have never expected psql's \d commands to work against older server
> versions, and two months after feature freeze isn't the time to start
> making that happen.
>
That said, number of folks have looked at this problem and agree it would be
nice to do, they just haven't formed a consensus on how to do it. If you
have a plan for how you would want to approach this in 8.2, feel free to post
it.
--
Robert Treat
Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL
On Mon, Aug 29, 2005 at 08:12:37AM -0400, Robert Treat wrote:
> On Monday 29 August 2005 00:33, Tom Lane wrote:
> > David Fetter <david@fetter.org> writes:
> > > On a slightly related note, I've noticed that psql isn't
> > > backward compatible.
> >
> > We have never expected psql's \d commands to work against older
> > server versions, and two months after feature freeze isn't the
> > time to start making that happen.
Tom, good point on the timing. I wish I'd come up with this at a
better moment for 8.1. I still contend that this falls squarely in
the realm of bug fixes, and would be happy to get as
backward-compatible a bunch of \ functions as possible, even
back-porting, little as I believe that should be encouraged.
> That said, number of folks have looked at this problem and agree it
> would be nice to do, they just haven't formed a consensus on how to
> do it. If you have a plan for how you would want to approach this
> in 8.2, feel free to post it.
I'd be curious as to what's been proposed before, but briefly:
switch (pset.sversion/100) {
case 801:
...
break;
case 800:
...
break;
.
.
.
default:
...
break;
}
for each version-specific chunk of SQL that a backslash command would
issue. I know it makes the code bigger, but stacking things that way
makes it relatively easy to prepend new conditions at the top as
needed. Also TBD, I think it would be a Very Good Idea(TM) to see
about how this (and other front ends) can play nice with the
newsysviews project. It really chaps me to see all these wheels being
reinvented. :/
Cheers,
D
--
David Fetter david@fetter.org http://fetter.org/
phone: +1 510 893 6100 mobile: +1 415 235 3778
Remember to vote!
David Fetter wrote: >On Mon, Aug 29, 2005 at 08:12:37AM -0400, Robert Treat wrote: > > >>On Monday 29 August 2005 00:33, Tom Lane wrote: >> >> >>>David Fetter <david@fetter.org> writes: >>> >>> >>>>On a slightly related note, I've noticed that psql isn't >>>>backward compatible. >>>> >>>> >>>We have never expected psql's \d commands to work against older >>>server versions, and two months after feature freeze isn't the >>>time to start making that happen. >>> >>> > >Tom, good point on the timing. I wish I'd come up with this at a >better moment for 8.1. I still contend that this falls squarely in >the realm of bug fixes > [ -patches removed ] I don't see how, if it is not functionality that has been explicitly or implicitly promised. The fact that it isn't what you expected doesn't make it a bug. There's a natural tendency to want to call things bugs at this stage of the cycle so that they qualify for application, but there's a reason we have a freeze, and it needs to be adhered to. If we're going to do backwards compatibility for psql then we need to do it in a fairly comprehensive way, not bit by bit, because we can reasonably say either "we support backwards compatibility" or "we don't support backwards compatibility", but we cannot reasonably say "we support backwards compatibility just for these commands" - that's way too confusing. The task is probably non-trivial - just look at pg_dump. Might be another good starting hackers project. cheers andreew
Andrew Dunstan <andrew@dunslane.net> writes:
> If we're going to do backwards compatibility for psql then we need to do
> it in a fairly comprehensive way, not bit by bit, because we can
> reasonably say either "we support backwards compatibility" or "we don't
> support backwards compatibility", but we cannot reasonably say "we
> support backwards compatibility just for these commands" - that's way
> too confusing.
Yeah. It would be good to set some parameters before starting: how far
back is reasonable to support? pg_dump goes back to 7.0 but that's now
mostly for historical reasons, ie, 7.0 was the immediately previous
release when we started making it do backwards-compatible dumps. I'm
not sure it's worth the trouble to make psql go that far back. "Back
to 8.0" would be a nice round figure...
> The task is probably non-trivial - just look at pg_dump.
> Might be another good starting hackers project.
If you consulted the back branches of psql source code it wouldn't be
too hard I would think, though surely tedious.
As an aside, I would most certainly NOT use the "switch" coding style
suggested upthread, as that is guaranteed to break completely every time
there's a version bump. Do it the way pg_dump does, with a series of
"if (version >= something)" tests. Then you only have to change a given
piece of code when there's a direct reason to change it.
regards, tom lane
Tom Lane wrote: > It would be good to set some parameters before starting: how far >back is reasonable to support? pg_dump goes back to 7.0 but that's now >mostly for historical reasons, ie, 7.0 was the immediately previous >release when we started making it do backwards-compatible dumps. I'm >not sure it's worth the trouble to make psql go that far back. "Back >to 8.0" would be a nice round figure... > > > Seems fair for an initial target of 8.2. Going forward a policy of "3 years or 3 releases back" seems about right. If people can't bring themselves to upgrade in that time they can be left on their own. cheers andrew
David Fetter <david@fetter.org> writes:
> I've noticed that \df doesn't do quite what it might when a function
> is created with named input parameters. Please find enclosed a patch
> against CVS TIP that does this better.
Meanwhile, getting back to the actual merits of the patch ... this is
not right yet, because it will do the wrong thing when there are OUT
parameters. (The proargnames array includes both IN and OUT params,
and you can't assume that proargnames and proargtypes have corresponding
subscripts.) It would probably be a good idea to discuss what display
we want for a function with OUT parameters, anyway. The strict columnar
representation that \df currently uses doesn't scale very well :-(
regards, tom lane
On Mon, Aug 29, 2005 at 11:13:29AM -0400, Tom Lane wrote: > David Fetter <david@fetter.org> writes: > > I've noticed that \df doesn't do quite what it might when a > > function is created with named input parameters. Please find > > enclosed a patch against CVS TIP that does this better. > > Meanwhile, getting back to the actual merits of the patch ... this > is not right yet, because it will do the wrong thing when there are > OUT parameters. Right. I'd tried doing something with INOUT and OUT parameters, but I wasn't able to figure out how to do with oid[] what I'd do with oidvector. On the bright side, what I did does do the right thing if there are named IN parameters, which was part of what I was trying to fix. > (The proargnames array includes both IN and OUT params, and you > can't assume that proargnames and proargtypes have corresponding > subscripts.) It would probably be a good idea to discuss what > display we want for a function with OUT parameters, anyway. The > strict columnar representation that \df currently uses doesn't scale > very well :-( Speaking of said psql's columnar representations, what about the alignment thing proposed earlier where an embedded newline doesn't mess up the alignment of everything else? Is there some generic way to handle this? Cheers, D -- David Fetter david@fetter.org http://fetter.org/ phone: +1 510 893 6100 mobile: +1 415 235 3778 Remember to vote!
David Fetter <david@fetter.org> writes:
> Speaking of said psql's columnar representations, what about the
> alignment thing proposed earlier where an embedded newline doesn't
> mess up the alignment of everything else? Is there some generic way
> to handle this?
If that's not on TODO already, it certainly should be --- but again,
I don't see that happening for 8.1. We've lived with it for years
so it hardly counts as "must fix for 8.1".
Were you thinking of proposing a multiline output format for \df when
there are named parameters? We could plan on doing that in 8.2,
assuming someone steps up to fix the formatting code by then.
regards, tom lane
Seems this item will have to remain for 8.2. I have added this to TODO:
o Display IN, INOUT, and OUT parameters in \df+
It probably requires psql to output newlines in the proper
column, which is already on the TODO list.
---------------------------------------------------------------------------
Tom Lane wrote:
> David Fetter <david@fetter.org> writes:
> > I've noticed that \df doesn't do quite what it might when a function
> > is created with named input parameters. Please find enclosed a patch
> > against CVS TIP that does this better.
>
> Meanwhile, getting back to the actual merits of the patch ... this is
> not right yet, because it will do the wrong thing when there are OUT
> parameters. (The proargnames array includes both IN and OUT params,
> and you can't assume that proargnames and proargtypes have corresponding
> subscripts.) It would probably be a good idea to discuss what display
> we want for a function with OUT parameters, anyway. The strict columnar
> representation that \df currently uses doesn't scale very well :-(
>
> regards, tom lane
>
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
>
> http://archives.postgresql.org
>
--
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