Re: review: psql: edit function, show function commands patch

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Jan Urbański <wulczer(at)wulczer(dot)org>, Postgres - Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: review: psql: edit function, show function commands patch
Date: 2010-08-11 03:58:12
Message-ID: 25723.1281499092@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Boxuan Zhai 2010-08-11 04:09:11 Re: MERGE Specification
Previous Message Robert Haas 2010-08-11 03:39:55 Re: review: psql: edit function, show function commands patch