Adding \ev view editor?
All,
I've grown to adore the new \ef function editor.
It doesn't seem like it would be that difficult to add a view editor as
\ev. While editors would also be good for other objects, I don't think
we can do \et or \er etc. because those objects don't support CREATE OR
REPLACE.
Opinions? Other objects which could take \e?
--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com
On Tue, Sep 01, 2009 at 11:41:31AM -0700, Josh Berkus wrote:
All,
I've grown to adore the new \ef function editor.
It doesn't seem like it would be that difficult to add a view editor as
\ev. While editors would also be good for other objects, I don't think
we can do \et or \er etc. because those objects don't support CREATE OR
REPLACE.Opinions? Other objects which could take \e?
For objects that don't take CREATE OR REPLACE, \e could include the
object definition at the top in a multi-line comment, then start with
ALTER [OBJECT]
Cheers,
David.
--
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david.fetter@gmail.com
Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate
Josh Berkus wrote:
All,
I've grown to adore the new \ef function editor.
It doesn't seem like it would be that difficult to add a view editor as
\ev. While editors would also be good for other objects, I don't think
we can do \et or \er etc. because those objects don't support CREATE OR
REPLACE.Opinions? Other objects which could take \e?
That would make it even more desirable to make my proposed change to
pg_get_viewdef to put line breaks between each target item.
cheers
andrew
On tis, 2009-09-01 at 11:41 -0700, Josh Berkus wrote:
Opinions? Other objects which could take \e?
All of them.
Hi,
Peter Eisentraut <peter_e@gmx.net> writes:
On tis, 2009-09-01 at 11:41 -0700, Josh Berkus wrote:
Opinions? Other objects which could take \e?
All of them.
Well, I'd vote against \e table. Are you going to propose the CREATE
TABLE statement and have magics to produce the ALTER TABLE that would
resort? I hope not, because in some cases I fail to see how you'd do it
(CREATE TABLE AS SELECT ... comes to mind, and CREATE TABLE ... (LIKE
...) INHERITS ...; too, let alone where to put the column and table
constraints: separate commands or embedded ones?).
So you'd propose ALTER TABLE ... and nothing more, and I don't see any
gain in that, but maybe it's just me.
Editing an index is interesting too, do you generate a DROP INDEX then
the new CREATE INDEX, do you add a CREATE INDEX CONCURRENTLY somewhere
in the mix, etc...
More generally, as said upthread, \e should certainly only be available
for objects we can CREATE OR REPLACE...
Regards,
--
dim
On Tue, Sep 1, 2009 at 2:41 PM, Josh Berkus<josh@agliodbs.com> wrote:
All,
I've grown to adore the new \ef function editor.
It doesn't seem like it would be that difficult to add a view editor as
\ev. While editors would also be good for other objects, I don't think
we can do \et or \er etc. because those objects don't support CREATE OR
REPLACE.Opinions? Other objects which could take \e?
One reason I don't like this proposal is that postgresql does not
preserve the original 'create view' statement for editing. The
resulting sql that is given to you to edit is hopelessly mangled and I
think it's not good to encourage people to modify views in this
fashion.
Then again, we are only reproducing what pgadmin can already do, so why not? :-)
merlin
On Sep 2, 2009, at 5:07 AM, Merlin Moncure wrote:
One reason I don't like this proposal is that postgresql does not
preserve the original 'create view' statement for editing. The
resulting sql that is given to you to edit is hopelessly mangled and I
think it's not good to encourage people to modify views in this
fashion.Then again, we are only reproducing what pgadmin can already do, so
why not? :-)
Is there any reason that the CREATE VIEW output to pg_dump and,
presumably, \ev, couldn't be refactored to pretty-print the VIEW
declaration?
Best,
David
David E. Wheeler wrote:
On Sep 2, 2009, at 5:07 AM, Merlin Moncure wrote:
One reason I don't like this proposal is that postgresql does not
preserve the original 'create view' statement for editing. The
resulting sql that is given to you to edit is hopelessly mangled and I
think it's not good to encourage people to modify views in this
fashion.Then again, we are only reproducing what pgadmin can already do, so
why not? :-)Is there any reason that the CREATE VIEW output to pg_dump and,
presumably, \ev, couldn't be refactored to pretty-print the VIEW
declaration?
Tom just said a day or two ago that he doesn't want pg_dump using any
pretty printing, and I think there is some sense in that.
As discussed last week, the current viewdef pretty printing doesn't put
each target in the select list on it own line (or lines) - hence my
proposal to make it do so. I think this would be even more desirable if
we have \ev. Without that I think \ev is likely to be fairly useless.
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
David E. Wheeler wrote:
Is there any reason that the CREATE VIEW output to pg_dump and,
presumably, \ev, couldn't be refactored to pretty-print the VIEW
declaration?
Tom just said a day or two ago that he doesn't want pg_dump using any
pretty printing, and I think there is some sense in that.
It might be worth pointing out that what I don't want pg_dump doing
is suppressing "useless" parentheses. Adding whitespace ought to be
safe enough. So if anyone wanted to do the work of decoupling those
two effects of the prettyprint option, we could have "semi pretty
printed" output in pg_dump. Dunno if it's worth it.
I don't see any particular reason to say that \ev shouldn't use fully
pretty printed output, though. It doesn't have the same sorts of
future-proofing worries that pg_dump does.
regards, tom lane
Tom Lane wrote:
It might be worth pointing out that what I don't want pg_dump doing
is suppressing "useless" parentheses. Adding whitespace ought to be
safe enough. So if anyone wanted to do the work of decoupling those
two effects of the prettyprint option, we could have "semi pretty
printed" output in pg_dump. Dunno if it's worth it.
The attached patch goes part of the way towards doing this, by adding
white space unconditionally to the target list of a viewdef.
The nice thing about that is that it's the part of a viewdef which
basically isn't pretty-printed now, even if you ask for pretty printing.
That would certainly
make editing the view with a \ev command a lot nicer.
Along the way it suppresses putting a newline before a CASE expression
in pretty printing mode, which seems to be an odd thing to do and is
inconsistent with the way other expressions are treated.
Sample output:
andrew=# select pg_get_viewdef('foo',true);
pg_get_viewdef
------------------------------
SELECT 'a'::text AS b,
( SELECT 1
FROM dual) AS x,
random() AS y,
CASE
WHEN true THEN 1
ELSE 0
END AS c,
1 AS d
FROM dual;
(1 row)
cheers
andrew
Attachments:
prettyprint2.patchtext/x-patch; charset=iso-8859-1; name=prettyprint2.patchDownload
Index: src/backend/utils/adt/ruleutils.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v
retrieving revision 1.306
diff -c -r1.306 ruleutils.c
*** src/backend/utils/adt/ruleutils.c 1 Aug 2009 19:59:41 -0000 1.306
--- src/backend/utils/adt/ruleutils.c 21 Sep 2009 18:22:01 -0000
***************
*** 2648,2659 ****
TupleDesc resultDesc)
{
StringInfo buf = context->buf;
char *sep;
! int colno;
ListCell *l;
sep = " ";
colno = 0;
foreach(l, targetList)
{
TargetEntry *tle = (TargetEntry *) lfirst(l);
--- 2648,2671 ----
TupleDesc resultDesc)
{
StringInfo buf = context->buf;
+ StringInfoData sep_indent;
char *sep;
! int colno, len;
ListCell *l;
sep = " ";
colno = 0;
+
+ /*
+ * Separator to make each target appear on its own line, regardless
+ * of pretty printing.
+ * Try to make them all line up at the same line position.
+ */
+ initStringInfo(&sep_indent);
+ appendStringInfoString(&sep_indent,",\n ");
+ for (len = buf->len -1; len >= 0 && buf->data[len]!= '\n'; len--)
+ appendStringInfoChar(&sep_indent,' ');
+
foreach(l, targetList)
{
TargetEntry *tle = (TargetEntry *) lfirst(l);
***************
*** 2664,2670 ****
continue; /* ignore junk entries */
appendStringInfoString(buf, sep);
! sep = ", ";
colno++;
/*
--- 2676,2682 ----
continue; /* ignore junk entries */
appendStringInfoString(buf, sep);
! sep = sep_indent.data;
colno++;
/*
***************
*** 2704,2709 ****
--- 2716,2726 ----
appendStringInfo(buf, " AS %s", quote_identifier(colname));
}
}
+
+ if (colno > 1 && ! PRETTY_INDENT(context))
+ appendStringInfoChar(buf,'\n');
+
+ pfree(sep_indent.data);
}
static void
***************
*** 4595,4602 ****
CaseExpr *caseexpr = (CaseExpr *) node;
ListCell *temp;
! appendContextKeyword(context, "CASE",
! 0, PRETTYINDENT_VAR, 0);
if (caseexpr->arg)
{
appendStringInfoChar(buf, ' ');
--- 4612,4620 ----
CaseExpr *caseexpr = (CaseExpr *) node;
ListCell *temp;
! appendStringInfoString(buf,"CASE");
! if (PRETTY_INDENT(context))
! context->indentLevel += PRETTYINDENT_VAR;
if (caseexpr->arg)
{
appendStringInfoChar(buf, ' ');
Andrew Dunstan <andrew@dunslane.net> writes:
Tom Lane wrote:
It might be worth pointing out that what I don't want pg_dump doing
is suppressing "useless" parentheses. Adding whitespace ought to be
safe enough. So if anyone wanted to do the work of decoupling those
two effects of the prettyprint option, we could have "semi pretty
printed" output in pg_dump. Dunno if it's worth it.
The attached patch goes part of the way towards doing this, by adding
white space unconditionally to the target list of a viewdef.
That's not what I had in mind by "decoupling" the option's effects.
regards, tom lane
Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
Tom Lane wrote:
It might be worth pointing out that what I don't want pg_dump doing
is suppressing "useless" parentheses. Adding whitespace ought to be
safe enough. So if anyone wanted to do the work of decoupling those
two effects of the prettyprint option, we could have "semi pretty
printed" output in pg_dump. Dunno if it's worth it.The attached patch goes part of the way towards doing this, by adding
white space unconditionally to the target list of a viewdef.I'd rather do that than add another pg_get_viewdef variant or option.
That's not what I had in mind by "decoupling" the option's effects.
Well, regardless of that it does what I want, and with a fairly small
amount of code.
I can make it work only in the pretty print case, if that's your objection.
Like you I doubt that fully decoupling pretty printing parentheses and
whitespace is going to be worth the trouble.
cheers
andrew
Andrew Dunstan <andrew@dunslane.net> writes:
Tom Lane wrote:
That's not what I had in mind by "decoupling" the option's effects.
Well, regardless of that it does what I want, and with a fairly small
amount of code.
Well, yeah, because you are paying no mind to what anyone else might
want.
I can make it work only in the pretty print case, if that's your objection.
I think that's back where we started in this thread. I can live with
it, but I thought some other people were unhappy with the idea of many
lines of targetlist...
regards, tom lane
Tom Lane wrote:
Andrew Dunstan <andrew@dunslane.net> writes:
Tom Lane wrote:
That's not what I had in mind by "decoupling" the option's effects.
Well, regardless of that it does what I want, and with a fairly small
amount of code.Well, yeah, because you are paying no mind to what anyone else might
want.
Umm, no. I have listened but it's not clear what people want. I don't
hear too many people actually defending the status quo. Any viewdef with
any substantial number of columns is just about unreadable.
I can make it work only in the pretty print case, if that's your objection.
I think that's back where we started in this thread. I can live with
it, but I thought some other people were unhappy with the idea of many
lines of targetlist...
Well, some were yes, but nobody actually came up with anything better.
IIRC the code I tried (in response to what people said) that limited
newlines by only wrapping lines at some limit got more objections than
my original proposal. At one stage you said:
Why don't you have the flag just driving your original two-line addition?
The patch I sent today is more or less the equivalent of that two-line
addition, but removes some ugliness. It's a few more lines, but not
many. So that suggestion of mine above to make it work just in the
pretty print case was actually intended to be responsive to your
previous suggestion.
cheers
andrew
On Mon, Sep 21, 2009 at 02:26:05PM -0400, Andrew Dunstan wrote:
andrew=# select pg_get_viewdef('foo',true);
pg_get_viewdef
------------------------------
SELECT 'a'::text AS b,
( SELECT 1
FROM dual) AS x,
random() AS y,
CASE
WHEN true THEN 1
ELSE 0
END AS c,
1 AS d
FROM dual;
(1 row)
+1
-dg
--
David Gould daveg@sonic.net 510 536 1443 510 282 0869
If simplicity worked, the world would be overrun with insects.
Is this a TODO?
---------------------------------------------------------------------------
Josh Berkus wrote:
All,
I've grown to adore the new \ef function editor.
It doesn't seem like it would be that difficult to add a view editor as
\ev. While editors would also be good for other objects, I don't think
we can do \et or \er etc. because those objects don't support CREATE OR
REPLACE.Opinions? Other objects which could take \e?
--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +
On Tue, Feb 23, 2010 at 12:43 AM, Bruce Momjian <bruce@momjian.us> wrote:
Is this a TODO?
Sounds good to me.
It doesn't seem like it would be that difficult to add a view editor as
\ev.
...Robert
Robert Haas wrote:
On Tue, Feb 23, 2010 at 12:43 AM, Bruce Momjian <bruce@momjian.us> wrote:
Is this a TODO?
Sounds good to me.
I think it would be useful if we put line breaks in the column list it
gets, otherwise not so much for any view with more than a handful of
columns. That's where we came in on the pretty-printing of viewdefs ...
cheers
andrew
Andrew Dunstan wrote:
Robert Haas wrote:
On Tue, Feb 23, 2010 at 12:43 AM, Bruce Momjian <bruce@momjian.us> wrote:
Is this a TODO?
Sounds good to me.
I think it would be useful if we put line breaks in the column list it
gets, otherwise not so much for any view with more than a handful of
columns. That's where we came in on the pretty-printing of viewdefs ...
Added to TODO:
|Add ability to edit views with \ev
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
PG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do
+ If your life is a hard drive, Christ can be your backup. +