2012/10/15 Pavel Stehule <pavel.steh...@gmail.com>: > 2012/10/15 Shigeru HANADA <shigeru.han...@gmail.com>: >> Hi Pavel, >> >> First of all, I'm sorry that my previous review was rough. I looked >> your patch and existing code closely again. >> >> On 2012/10/15, at 12:57, Pavel Stehule <pavel.steh...@gmail.com> wrote: >>> 2012/10/14 Tom Lane <t...@sss.pgh.pa.us>: >>>> * ExecQueryUsingCursor's concept of how to process command results >>>> (why? surely we don't need \gset to use a cursor) >>> >>> There was two possibilities, but hardly non using cursor is better way >> >> +1 for supporting the case when FETCH_COUNT > 0, because user might set >> so mainly for other queries, and they would want to avoid changing >> FETCH_COUNT setting during every query followed by \gset. >> >>>> * the psql lexer (adding a whole bunch of stuff that probably doesn't >>>> belong there) >>> >>> ?? >> >> I think that Tom is talking about psql_scan_slash_vars(). It seems too >> specific to \gset command. How about to improve >> psql_scan_slash_options() so that it can handle comma-separated variable >> list? Although you might have tried it before. >> # Unused OT_VARLIST macro gave me the idea. > > yes, it is possible - I'll look on it at evening
a reuse of psql_scan_slash_options is not good idea - a interface of this function is out of my purposes - and I cannot to signalise error from this procedure. But I can minimize psql_scan_slash_var and I can move lot of code out of lexer file. > >> >>>> * the core psql settings construct (to store something that is in >>>> no way a persistent setting) >>>> >>> >>> ?? >> >> I thought that having \gset arguments in pset is reasonable, since \g >> uses pest.gfname to hold its one-shot setting. Or, we should refactor >> \g to fit with \gset? I might be missing Tom's point... >> >>>> Surely there is a less ugly and invasive way to do this. The fact >>>> that the reviewer keeps finding bizarre bugs like "another backslash >>>> command on the same line doesn't work" seems to me to be a good >>>> indication that this is touching things it shouldn't. >>> >>> - all these bugs are based on lexer construct. A little modification >>> of lexer is possible >> >> IMHO those issues come from the design rather than the implementation of >> lexer. AFAIK we don't have consensus about the case that both of \g and >> \gset are used for a query like this: >> >> postgres=# SELECT 1 \gset var \\ \g foo.txt >> >> This seems regal. Should we store "1" into var and write the result >> into foo.txt? Or, only either of them? It's just an idea and it >> requires new special character, but how about use \g command for file, >> pipe, and variable? In the case we choose '&' for variable prefix: >> >> postgres=# SELECT 'hello', 'wonderful', 'world!' \g &var1,,var2 >> >> Anyway, we've had no psql's meta command which processes query result >> other than \g. So, we might have more considerable issues about design. > > a current design is rigid - a small implementation can stop parsing > target list, when other backslash statement is detected > >> >> BTW, what the word "comma_expected" means? It's in the comment above >> psql_scan_slash_vars(). It might be a remaining of old implementation. > > This identifier is mistaken - etc this comment is wrong and related to > old implementation - sorry. A first design was replaced by state > machine described by VarListParserState > > > >> >> Regards, >> -- >> Shigeru HANADA >> shigeru.han...@gmail.com >> >> >> >> >> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
