review: psql: edit function, show function commands patch
in [PgSql]
|
From: Tom Lane on 1 Aug 2010 11:35 Robert Haas <robertmhaas(a)gmail.com> writes: > On Sun, Aug 1, 2010 at 10:47 AM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >> The need to count lines manually in function definitions is >> far less than it was back when that kluge was put in. > Why? That hack goes back to plpgsql's prehistory (it's there, though sans comment, in plpgsql's scan.l 1.1). We had none of the current support for identifying error locations by cursor position and/or quoting part of the source text back at you. Let me illustrate what happened with a simple syntax error in PG 7.0: play=> create function fool() returns int as ' play'> begin play'> fool play'> end' language 'plpgsql'; CREATE play=> select fool(); NOTICE: plpgsql: ERROR during compile of fool near line 2 ERROR: missing ; at end of SQL statement play=> So you *had* to count lines, and do it accurately too, to figure out even pretty simple syntax errors. Personally, rather than sweat about what the exact definition of line numbers is, I think we should be moving further in the direction of being able to regurgitate source text to identify error locations. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Robert Haas on 1 Aug 2010 13:28 On Sun, Aug 1, 2010 at 11:35 AM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas(a)gmail.com> writes: >> On Sun, Aug 1, 2010 at 10:47 AM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >>> The need to count lines manually in function definitions is >>> far less than it was back when that kluge was put in. > >> Why? > > That hack goes back to plpgsql's prehistory (it's there, though sans > comment, in plpgsql's scan.l 1.1). �We had none of the current support > for identifying error locations by cursor position and/or quoting part > of the source text back at you. �Let me illustrate what happened with > a simple syntax error in PG 7.0: > > play=> create function fool() returns int as ' > play'> begin > play'> � fool > play'> end' language 'plpgsql'; > CREATE > play=> select fool(); > NOTICE: �plpgsql: ERROR during compile of fool near line 2 > ERROR: �missing ; at end of SQL statement > play=> > > So you *had* to count lines, and do it accurately too, to figure out > even pretty simple syntax errors. > > Personally, rather than sweat about what the exact definition of line > numbers is, I think we should be moving further in the direction of > being able to regurgitate source text to identify error locations. I basically agree with that; but on the other hand, in a large PL/pgsql function, you may have very similar-looking text in multiple places. So line numbers are good, too: but then you weren't proposing to remove those, I assume, just to augment them with additional information. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Tom Lane on 1 Aug 2010 14:43 Robert Haas <robertmhaas(a)gmail.com> writes: > On Sun, Aug 1, 2010 at 11:35 AM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >> Personally, rather than sweat about what the exact definition of line >> numbers is, I think we should be moving further in the direction of >> being able to regurgitate source text to identify error locations. > I basically agree with that; but on the other hand, in a large > PL/pgsql function, you may have very similar-looking text in multiple > places. So line numbers are good, too: but then you weren't proposing > to remove those, I assume, just to augment them with additional > information. Right. I'm just suggesting that source text is better than line numbers for exact position identification, so I don't see a lot of value in preserving historical behaviors that change line numbers by one count one way or another. If some of the PLs used zero-based instead of one-based line numbers, we'd be looking to standardize that not cater to their individual idiosyncrasies. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Pavel Stehule on 1 Aug 2010 16:53 Hello I am sending a modified patch - changes: a) remove special row number handling of plpgsql (first patch) b) more robust algorithm for header rows identification Regards Pavel Stehule 2010/8/1 Robert Haas <robertmhaas(a)gmail.com>: > On Sun, Aug 1, 2010 at 10:47 AM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote: >> Pavel Stehule <pavel.stehule(a)gmail.com> writes: >>> so my plan >> >>> a) fix problem with ambiguous $function* like you proposed >>> b) fix problem with "first row excepting" - I can activate a detection >>> only for plpgsql language - I can identify LANGUAGE before. >> >> Ick. Â We should absolutely NOT have a client-side special case for plpgsql. >> >> Personally I'd be fine with dropping the special case from the plpgsql >> parser --- I don't believe that that behavior was ever discussed, much >> less documented, and I doubt that many people rely on it or even know >> it exists. > > +1. > >> The need to count lines manually in function definitions is >> far less than it was back when that kluge was put in. > > Why? > >> If anyone can make a convincing case that it's a good idea to ignore >> leading newlines, we should reimplement the behavior in such a way that >> it applies across the board to all PLs (ie, make CREATE FUNCTION strip >> a leading newline before storing the text). Â However, then you'd have >> issues about whether or when to put back the newline, so I'm not really >> in favor of that route. > > Ditto. > > As a procedural note, if we decide to go this route, this should be > split into two patches - one that removes the line-numbering kludge, > and a second for the psql changes. > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise Postgres Company >
From: Robert Haas on 1 Aug 2010 23:48
On Sun, Aug 1, 2010 at 4:53 PM, Pavel Stehule <pavel.stehule(a)gmail.com> wrote: > a) remove special row number handling of plpgsql (first patch) Committed. > b) more robust algorithm for header rows identification Have not gotten to this one yet. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |