On 2014-08-31 12:06:31 +0900, Sawada Masahiko wrote:
> Thank you for review comment and improving the patch!
> I tested it.
> Your patch always increment line number even if there is no input line
> as follows.
>
> postgres[1]=#
> postgres[2]=# select
> postgres[3]-# ,
> postgres[4]-# from
> postgres[5]-# hoge;
> ERROR: syntax error at or near "," at character 8
> STATEMENT: select
> ,
> from
> hoge;
> ERROR: syntax error at or near ","
> LINE 2: ,
> ^
> Actually error syntax is in line 2 as postgres reported.
> But it is inconsistent.
Hm. Right. That's clearly wrong.
> Attached patch is resolve above behavior based on your version patch.
I've looked a bit further and found two more broken things.
1)
postgres[1]=# SELECT 1; SELECT 2
postgres[1]=#
Note the 1 in the second line. Obviously wrong.
The fix for this is easy: Don't count a newline if there isn't one. But
check for PSCAN_EOL. That also gets rid of inconsistent pset.stmt_lineno
initializations (sometimes to 0, sometimes to 1).
2)
postgres[1]=# SELECT 1,
postgres[2]-# 2,
postgres[3]-# 3;
┌──────────┬──────────┬──────────┐
│ ?column? │ ?column? │ ?column? │
├──────────┼──────────┼──────────┤
│ 1 │ 2 │ 3 │
└──────────┴──────────┴──────────┘
(1 row)
postgres[1]=# SELECT 1,
2,
3;
┌──────────┬──────────┬──────────┐
│ ?column? │ ?column? │ ?column? │
├──────────┼──────────┼──────────┤
│ 1 │ 2 │ 3 │
└──────────┴──────────┴──────────┘
(1 row)
postgres[3]=#
Obviously the three in the last line is wrong.
The fix is slightly nontrivial. It's wrong to look at 'line' when
determining the number of lines to add - it may already be
executed. The, it seems to me, correct thing is to look at the data
that's appended to the query buffer. Alternatively we could always count
all lines in the query buffer, but that'd be O(lines^2)...
I've done both in the appended patch.
I've now used up a perfectly good glass of wine for this, so this is it
for today ;)
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
