Re: review: psql: edit function, show function commands patch - Mailing list pgsql-hackers

From Tom Lane
Subject Re: review: psql: edit function, show function commands patch
Date
Msg-id 25723.1281499092@sss.pgh.pa.us
Whole thread Raw
In response to Re: review: psql: edit function, show function commands patch  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: review: psql: edit function, show function commands patch  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes:
> I spent some time cleaning this up tonight.  I think that the \e and
> \ef portions are now ready to commit, but I am not quite happy with
> the \sf stuff yet, so I've broken that out into a separate patch,
> which is also attached.

> Barring objections, I'll commit the \e and \ef portions of this in the
> morning after one final read-through.

The \e patch definitely needs another read-through.  I noticed a number
of comments that were still pretty poor English, and one ---/* skip header lines */
--- that seems just plain wrong.  The actual intent of that next bit is
to increase lineno to account for header lines, which is not well
conveyed by "skip".

BTW, at least in the usage in that loop, get_functiondef_dollarquote_tag
seems grossly overdesigned.  It would be clearer, shorter, and faster if
you just had a strncmp test for "AS $function" there.  Also, the entire
thing is subject to misbehavior in the case of \e (as opposed to \ef),
which really cannot safely assert() that it's reading the output of
pg_get_functiondef().  My inclination is to pull that part out of
do_edit and put it into \ef-specific code.

Also, there seemed to be some gratuitous inconsistency in the handling
of tests on line number variables, eg some places lineno > 0 and others
lineno >= 1.
        regards, tom lane


pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: review: psql: edit function, show function commands patch
Next
From: Boxuan Zhai
Date:
Subject: Re: MERGE Specification