Hello
>
> My first review...
>
> Patch applied cleanly to master and make check was fine.
>
> I tested it out and it operates as advertised. There were a couple
> things that stood out to me though.
>
> 1) NULL values are not displayed properly after \pset null is run.
>
> postgres=# \pset null '(null)'
> Null display is "(null)".
> postgres=# select NULL \gset var1
> postgres=# \echo :var1
>
> postgres=# select NULL;
> ?column?
> ----------
> (null)
> (1 row)
>
> I know this doesn't come back from the server like this, but you
> should be able to pull this from the options and display
> appropriately. Not sure if it should be in variable display code, or
> when you store it into the variable.
ok
>
> 2) The error messages seemed kind of terse. Other error messages are
> capitalized and have punctuation.
ok
>
> 3) We don't find out about incorrect number of columns until after
> query returns. I know this is hard/impossible to fix, but it might be
> nice to print out the result normally if you can't store it in the
> variables.
a changing behave when error is occurred is not good, but I can show a
number of returned columns
>
> 3b) You throw an error on too many variables, but still store the data
> since you have fewer columns than variables. This makes sense, but you
> don't inform the user at all.
there is not a some like stack, so I cannot to return values to
previous state. It should be documented - after error, a related
variables has undefined values.
>
> On to the code:
>
> 1) Introduction of random newlines:
>
> *************** cleanup:
> *** 1254,1259 ****
> --- 1383,1389 ----
> PQclear(results);
> }
>
> +
> if (pset.timing)
> {
> INSTR_TIME_SET_CURRENT(after);
>
> I saw that in a couple places, but that was the most obvious.
>
> 2) TargetList - Why not use the built in linked list operations rather
> than creating your own? Are they not accessible to client binaries
> like this?
actually, there are no support for lists on client side now. So it is
reason. But I'll remove it, because I'll move parsing to command
implementation, and then I don't need a list support
>
> Overall I think this is a useful feature and I think you integrated it
> well within the existing infrastructure, ie combining concepts of \set
> and \g.
Thank you
Regards
Pavel
