Re: [HACKERS] New psql input mode problems

On 1999-11-08, Keith Parks mentioned:

> Peter Eisentraut <e99re41(at)DoCS(dot)UU(dot)SE>
> >On Sun, 7 Nov 1999, Bruce Momjian wrote:
> >
> >> > This seems to be where it goes wrong. (mainloop.c)
> >> >
> >> > /* No more input. Time to quit, or \i done */
> >> > if (line == NULL || (!pset->cur_cmd_interactive && *line == '\0'))
> >
> >This line was there in the old source as well.
>
> Just checked and you're right..
>
> >
> >> >
> >> > When a blank line is encountered in the input
> >> >
> >> > line = gets_fromFile(source);
> >> >
> >> > returns an empty string ('\0') and terminates the processing.
> >
> >Same in the old one.
>
> I think somehow the old version was subtly different.
>
> The old version of psql read and processed the whole of
> each regression sql file. The new version stops processing
> as soon as it sees a blank line.
>
> I'm not sure if there's something different with the '\n'
> handling in the new version. If the '\n' is being stripped
> out that could make all the difference between "\n", which
> would not terminate and "" which would.

We have a winner!

The new gets_fromFile strips the trailing newline, so an empty line in a
file really comes in as an empty line. I don't see any reason why the
check was done as it was in the first place, so the correct line in
mainloop.c should be:

/* No more input. Time to quit, or \i done */
if (line == NULL)
{

as suggested.

>
> >
> >> >
> >> > with the if clause reduced to checking for line == NULL psql
> >> > does the work but fails badly due to the differences between
> >> > results and expected. (comments, QUERY:, echo processing)
> >
> >As I said, that part was not my idea. I'll look into that though.
>
> This is a difficult one, I never was any good at decisions.
>
> There are a number of key differences.
>
> Query text OLD echoed prefixed with "QUERY:" NEW echoed as read.

This behaviour is plagiarized from command shells.

> Formatting OLD slightly different to NEW. (alignment and '-'s)
>
>
> OLD:
>
> QUERY: SELECT 1 AS one;
> one
> ---
> 1
> (1 row)
>
>
> NEW:
>
>
> SELECT 1 AS one;
> one
> -----
> 1
> (1 row)
>
>
> >
> >>
> >> >
> >> > Is the intention to modify expected to agree with the new
> >> > results output, or fix psql to output in the expected format?
> >
> >How about using the old psql for regression testing?
>
> I like the new one better :-)

Life sucks ;)

>
> >
> >>
> >> Good question. We need to know if people like the current output
> >> format, or the old one better?
> >
> >I send in several examples. If no one comments, that's silent approval.
> >I am really hesitant to put in a compatibility format, but I might just do
> >that until new regression tests are out and you ask me to.
> >
>
> If it was not too difficult this would be one way to go.
>
> We really need the regression output maintainer (Tom?) to comment
> here. I'd hate to have to do the 1st compare between the new output
> and old expected by eye.

Here's an idea: Why don't the regression tests use a single-user postgres
backend? That way you have even more control over internals, you can run
it on an uninstalled database, and you don't rely on the particularities
of the output of some obscure front-end.

But I'm not going to tailor psql around the regression tests. That's the
wrong direction. Just run it once with the old one and then with the new
one and put those results in the current tree as a temporary solution.

-Peter

--
Peter Eisentraut Sernanders vaeg 10:115
peter_e(at)gmx(dot)net 75262 Uppsala
http://yi.org/peter-e/ Sweden