Обсуждение: proposal: multiple psql option -c

<div dir="ltr"><br />On Thu, Jul 16, 2015 at 4:42 PM, Pavel Stehule <<a
href="mailto:pavel.stehule@gmail.com">pavel.stehule@gmail.com</a>>wrote:<br />><br />> Hi<br />><br />>
canwe support multiple "-c" option?<br />><br />> Why? Because some statements like VACUUM cannot be used
togetherwith any other statements with single -c option. The current solution is using echo and pipe op, but it is a
complicationin some complex scripts - higher complication when you run psql via multiple sudo statement.<br />><br
/>>Example:<br />><br />> psql -c "select pg_stat_reset()" -c "vacuum full analyze" dbname<br />><br />>
oron all db<br />><br />> psql -At -c "select datname from pg_databases" postgres | \<br />> xargs -n 1 -P 3
psql-c "..." -c "..."<br />><br />> Ideas, notes, comments?<br />><br /><br />Why you want it if we already
havethe -f option that cover this use case?<br /><br />Regards,<br /><br />--<br />Fabrízio de Royes Mello<br
/>Consultoria/CoachingPostgreSQL<br />>> Timbira: <a
href="http://www.timbira.com.br">http://www.timbira.com.br</a><br/>>> Blog: <a
href="http://fabriziomello.github.io">http://fabriziomello.github.io</a><br/>>> Linkedin: <a
href="http://br.linkedin.com/in/fabriziomello">http://br.linkedin.com/in/fabriziomello</a><br/>>> Twitter: <a
href="http://twitter.com/fabriziomello">http://twitter.com/fabriziomello</a><br/>>> Github: <a
href="http://github.com/fabriziomello">http://github.com/fabriziomello</a></div>

>
> it is one possible solution too
>
> multiple -c option has advantage of simple evaluation of backslash
> statements .. -c "\l" -c "\dt" - but this advantage is not high important.

Or just properly understand the ; ?

-c "select * from foo; update bar set baz = 'bing'; vacuum bar;"

JD

>
> Pavel
>
>
>
>     Best Regards,
>     Dinesh
>     manojadinesh.blogspot.com <http://manojadinesh.blogspot.com>
>
>         Regards
>
>         Pavel
>
>
>


-- 
Command Prompt, Inc. - http://www.commandprompt.com/  503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Announcing "I'm offended" is basically telling the world you can't
control your own emotions, so everyone else should do it for you.

On Fri, Jul 17, 2015 at 12:36 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> Or just properly understand the ; ?
>>
>> -c "select * from foo; update bar set baz = 'bing'; vacuum bar;"
>
> there is a risk of compatibility issues - all statements runs under one
> transaction implicitly

So what?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

2015-07-23 17:52 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:

On Fri, Jul 17, 2015 at 12:36 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> Or just properly understand the ; ?
>>
>> -c "select * from foo; update bar set baz = 'bing'; vacuum bar;"
>
> there is a risk of compatibility issues - all statements runs under one
> transaction implicitly

So what?

[pavel@dhcppc2 ~]$ psql -c "insert into x values(txid_current()::text);insert into x values(txid_current()::text)" postgres
INSERT 0 1
the state string "INSERT 0 1" is buggy probably

On Sat, Jul 25, 2015 at 5:27 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> It will be nice side effect, but my primary problem was a impossibility to
> combine VACUUM and any other statement to one simple psql call.

Seems like you can do that easily enough:

[rhaas pgsql]$ (echo 'SELECT 1;'; echo 'VACUUM;'; echo 'SELECT 2;') | psql?column?
----------       1
(1 row)

VACUUM?column?
----------       2
(1 row)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Mon, Jul 27, 2015 at 2:37 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> 2015-07-27 20:32 GMT+02:00 Robert Haas <robertmhaas@gmail.com>:
>>
>> On Sat, Jul 25, 2015 at 5:27 AM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> > It will be nice side effect, but my primary problem was a impossibility
>> > to
>> > combine VACUUM and any other statement to one simple psql call.
>>
>> Seems like you can do that easily enough:
>>
>> [rhaas pgsql]$ (echo 'SELECT 1;'; echo 'VACUUM;'; echo 'SELECT 2;') | psql
>>  ?column?
>> ----------
>>         1
>> (1 row)
>>
>> VACUUM
>>  ?column?
>> ----------
>>         2
>> (1 row)
>>
>
> how I can do it with xargs?

I don't specifically what you're trying to do, but I bet it's not that hard.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 07/27/2015 02:53 PM, Pavel Stehule wrote:
>
>
>
>
> I am trying to run parallel execution
>
> psql -At -c "select datname from pg_database" postgres | xargs -n 1 -P 
> 3 psql -c "select current_database()"
>
>


I don't think it's going to be a hugely important feature, but I don't 
see a problem with creating a new option (-C seems fine) which would 
have the same effect as if the arguments were contatenated into a file 
which is then used with -f. IIRC -c has some special characteristics 
which means it's probably best not to try to extend it for this feature.

cheers

andrew

On Mon, Jul 27, 2015 at 2:53 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> I am trying to run parallel execution
>
> psql -At -c "select datname from pg_database" postgres | xargs -n 1 -P 3
> psql -c "select current_database()"

Put this in a shell script called run-psql:

#!/bin/bash

test $# = 0 && exit
for f in "${@:1:$(($#-1))}"; do   echo "$f" \;
done | psql "${@:$#}"

Then:

psql -At -c "select datname from pg_database" postgres | xargs -n 1 -P
3 ./run-psql "select current_database()" "vacuum" "select 1"

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 7/27/15 2:57 PM, Andrew Dunstan wrote:
>>
>> psql -At -c "select datname from pg_database" postgres | xargs -n 1 -P
>> 3 psql -c "select current_database()"
>>
>>
>
>
> I don't think it's going to be a hugely important feature, but I don't
> see a problem with creating a new option (-C seems fine) which would
> have the same effect as if the arguments were contatenated into a file
> which is then used with -f. IIRC -c has some special characteristics
> which means it's probably best not to try to extend it for this feature.

+1. I've occasionally wanted this as well.

I'd also vote for -C returning every state string as well.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Data in Trouble? Get it in Treble! http://BlueTreble.com

<div style="direction: ltr;font-family: Tahoma;color: #000000;font-size: 10pt;"><br /> ><br /> ><br />
>2015-07-285:24 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:<br /> ><br /> >    2015-07-27 21:57
GMT+02:00Andrew Dunstan <andrew@dunslane.net>:<br /> ><br /> >        On 07/27/2015 02:53 PM, Pavel Stehule
wrote:<br/> ><br /> >            I am trying to run parallel execution<br /> ><br /> >            psql -At
-c"select datname from pg_database" postgres | xargs -n 1 -P 3 psql -c "select current_database()"<br /> ><br />
><br/> ><br /> >        I don't think it's going to be a hugely important feature, but I don't see a problem
withcreating a new option (-C seems fine) which would have the same effect as if the arguments were contatenated into a
filewhich is then used with -f. IIRC -c has some special characteristics which means it's probably best not to try to
extendit for this feature.<br /> ><br /> ><br /> >    ok, I'll try to write patch.<br /> ><br /> ><br />
>Ihave a question. Can be -C option multiple?<br /><br /><br /> hello,<br /> Have you thought of how to support -1
alongwith -C ?<br /><br /> > handle the input as with -f<br />      that is, -1 -C would be equivalent to -c<br
/><br/> and<br /> psql -1 -C "sql_1; sql_2;" -C "sql_3; sql_4;"<br /><br /> => ?<br /><br /> BEGIN;         <br />
sql_1;        <br /> sql_2;         <br /> END;         <br /><br /> BEGIN;<br /> sql_3;<br /> sql_4;<br /> END;<br
/><br/> thoughts ?<br /><br /> The same logic could be added to -f<br /> although I see less advantages as with adding
-C<br/><br /> psql -1 -f "file1, file2" -f "file3, file4"<br /><br /> regards,<br /> Marc Mamin</div> 

On 17 July 2015 at 03:42, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hi
>
> can we support multiple "-c" option?
>
> Why? Because some statements like VACUUM cannot be used together with any
> other statements with single -c option. The current solution is using echo
> and pipe op, but it is a complication in some complex scripts - higher
> complication when you run psql via multiple sudo statement.
>
> Example:
>
> psql -c "select pg_stat_reset()" -c "vacuum full analyze" dbname

I don't see the point. Taking your 'sudo' issue into account, just:

sudo -u postgres psql <'__END__'
select pg_stat_reset();
vacuum full analyze;
__END__


or

echo -e 'select pg_stat_reset()\n vacuum full analyze;' | sudo -u postgres psql

or, of course, just run two commands.

There are plenty of existing ways to do this. Personally I find -c
awkward due to the need to worry about shell quoting and tend to
prefer a quoted here-document lots of the time anyway.

-- Craig Ringer                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services

<div style="direction: ltr;font-family: Tahoma;color: #000000;font-size: 10pt;"><br /> ><br /> ><br />
>2015-07-2810:43 GMT+02:00 Marc Mamin <M.Mamin@intershop.de>:<br /> ><br /> ><br /> >    ><br />
>   ><br /> >    >2015-07-28 5:24 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:<br /> >   
><br/> >    >    2015-07-27 21:57 GMT+02:00 Andrew Dunstan <andrew@dunslane.net>:<br /> >    ><br
/>>    >        On 07/27/2015 02:53 PM, Pavel Stehule wrote:<br /> >    ><br /> >    >            I
amtrying to run parallel execution<br /> >    ><br /> >    >            psql -At -c "select datname from
pg_database"postgres | xargs -n 1 -P 3 psql -c "select current_database()"<br /> >    ><br /> >    ><br />
>   ><br /> >    >        I don't think it's going to be a hugely important feature, but I don't see a
problemwith creating a new option (-C seems fine) which would have the same effect as if the arguments were
contatenatedinto a file which is then used with -f. IIRC -c has some special characteristics which means it's probably
bestnot to try to extend it for this feature.<br /> >    ><br /> >    ><br /> >    >    ok, I'll try
towrite patch.<br /> >    ><br /> >    ><br /> >    >I have a question. Can be -C option multiple?<br
/>><br /> ><br /> >    hello,<br /> >    Have you thought of how to support -1 along with -C ?<br />
><br/> >    > handle the input as with -f<br /> >         that is, -1 -C would be equivalent to -c<br />
><br/> >    and<br /> >    psql -1 -C "sql_1; sql_2;" -C "sql_3; sql_4;"<br /> ><br /> >    => ?<br
/>><br /> >    BEGIN;         <br /> >    sql_1;         <br /> >    sql_2;         <br /> >   
END;        <br /> ><br /> >    BEGIN;<br /> >    sql_3;<br /> >    sql_4;<br /> >    END;<br /> ><br
/>>    thoughts ?<br /> ><br /> ><br /> >"-1" option is global -, I expected so following steps are more
natural<br/> ><br /> >BEGIN<br /> >  sql_1;<br /> >  sql_2;<br /> >  sql_3;<br /> >  sql_4;<br />
>END;<br/><br /> This is then exactly the same as -c. <br /> If introducing multiple -C to better manage transaction
handling,<br /> why not enrich this new feature with the abilities to define batches of transactions ?<br /><br />
Marc</div>

On 07/28/2015 04:43 AM, Marc Mamin wrote:
>
> >
> >
> >2015-07-28 5:24 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
> >
> >    2015-07-27 21:57 GMT+02:00 Andrew Dunstan <andrew@dunslane.net>:
> >
> >        On 07/27/2015 02:53 PM, Pavel Stehule wrote:
> >
> >            I am trying to run parallel execution
> >
> >            psql -At -c "select datname from pg_database" postgres | 
> xargs -n 1 -P 3 psql -c "select current_database()"
> >
> >
> >
> >        I don't think it's going to be a hugely important feature, 
> but I don't see a problem with creating a new option (-C seems fine) 
> which would have the same effect as if the arguments were contatenated 
> into a file which is then used with -f. IIRC -c has some special 
> characteristics which means it's probably best not to try to extend it 
> for this feature.
> >
> >
> >    ok, I'll try to write patch.
> >
> >
> >I have a question. Can be -C option multiple?
>
>
> hello,
> Have you thought of how to support -1 along with -C ?
>
> > handle the input as with -f
>      that is, -1 -C would be equivalent to -c
>
> and
> psql -1 -C "sql_1; sql_2;" -C "sql_3; sql_4;"
>
> => ?
>
> BEGIN;
> sql_1;
> sql_2;
> END;
>
> BEGIN;
> sql_3;
> sql_4;
> END;
>
> thoughts ?
>
> The same logic could be added to -f
> although I see less advantages as with adding -C
>
> psql -1 -f "file1, file2" -f "file3, file4"
>


This is way too complex and baroque. -1 should be global. Multiple -C 
options should be concatenated. -f should not be touched.

cheers

andrew

On 07/28/2015 12:08 AM, Pavel Stehule wrote:
>
>
> 2015-07-28 5:24 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com
> <mailto:pavel.stehule@gmail.com>>:
>
>
>
>     2015-07-27 21:57 GMT+02:00 Andrew Dunstan <andrew@dunslane.net
>     <mailto:andrew@dunslane.net>>:
>
>
>         On 07/27/2015 02:53 PM, Pavel Stehule wrote:
>
>
>
>
>
>             I am trying to run parallel execution
>
>             psql -At -c "select datname from pg_database" postgres |
>             xargs -n 1 -P 3 psql -c "select current_database()"
>
>
>
>
>         I don't think it's going to be a hugely important feature, but
>         I don't see a problem with creating a new option (-C seems
>         fine) which would have the same effect as if the arguments
>         were contatenated into a file which is then used with -f. IIRC
>         -c has some special characteristics which means it's probably
>         best not to try to extend it for this feature.
>
>
>     ok, I'll try to write patch.
>
>
> I have a question. Can be -C option multiple?
>
> The SQL is without problem, but what about \x command?
>
> postgres=# \dt \dn select 10;
> No relations found.
> List of schemas
> ┌──────┬───────┐
> │ Name │ Owner │
> ╞══════╪═══════╡
> └──────┴───────┘
> (0 rows)
>
> \dn: extra argument "10;" ignored


I don't understand the question.

You should include one sql or psql command per -C option, ISTM. e.g.
    psql -C '\dt' -C '\dn' -C 'select 10;'


Isn't that what we're talking about with this whole proposal?

cheers

andrew

On 07/28/2015 11:52 AM, Pavel Stehule wrote:
>
>
> 2015-07-28 15:16 GMT+02:00 Andrew Dunstan <andrew@dunslane.net
> <mailto:andrew@dunslane.net>>:
>
>
>     On 07/28/2015 12:08 AM, Pavel Stehule wrote:
>
>
>
>         2015-07-28 5:24 GMT+02:00 Pavel Stehule
>         <pavel.stehule@gmail.com <mailto:pavel.stehule@gmail.com>
>         <mailto:pavel.stehule@gmail.com
>         <mailto:pavel.stehule@gmail.com>>>:
>
>
>
>             2015-07-27 21:57 GMT+02:00 Andrew Dunstan
>         <andrew@dunslane.net <mailto:andrew@dunslane.net>
>             <mailto:andrew@dunslane.net <mailto:andrew@dunslane.net>>>:
>
>
>                 On 07/27/2015 02:53 PM, Pavel Stehule wrote:
>
>
>
>
>
>                     I am trying to run parallel execution
>
>                     psql -At -c "select datname from pg_database"
>         postgres |
>                     xargs -n 1 -P 3 psql -c "select current_database()"
>
>
>
>
>                 I don't think it's going to be a hugely important
>         feature, but
>                 I don't see a problem with creating a new option (-C seems
>                 fine) which would have the same effect as if the arguments
>                 were contatenated into a file which is then used with
>         -f. IIRC
>                 -c has some special characteristics which means it's
>         probably
>                 best not to try to extend it for this feature.
>
>
>             ok, I'll try to write patch.
>
>
>         I have a question. Can be -C option multiple?
>
>         The SQL is without problem, but what about \x command?
>
>         postgres=# \dt \dn select 10;
>         No relations found.
>         List of schemas
>         ┌──────┬───────┐
>         │ Name │ Owner │
>         ╞══════╪═══════╡
>         └──────┴───────┘
>         (0 rows)
>
>         \dn: extra argument "10;" ignored
>
>
>
>     I don't understand the question.
>
>     You should include one sql or psql command per -C option, ISTM. e.g.
>
>         psql -C '\dt' -C '\dn' -C 'select 10;'
>
>
>     Isn't that what we're talking about with this whole proposal?
>
>
>
> I am searching some agreement, how to solve a current "-c" limits. I
> am 100% for >>> psql -C '\dt' -C '\dn' -C 'select 10;' <<<
>
>

I think you're probably best off leaving -c alone. If there are issues
to be solved for -c they should be handled separately from the feature
we agree on.

cheers

andrew

<div dir="ltr">other example related to using psql in pipeline<br /><br /><span
style="font-family:monospace,monospace">[pavel@localhostpsql]$ ./psql postgres -q -g "vacuum analyze pg_attribute" -g
"\echo:DBNAME"<br />postgres</span><br /><br /></div> 

On 8/26/15 8:15 AM, Pavel Stehule wrote:
> +      and then exit. This is useful in shell scripts. Start-up files
> +      (<filename>psqlrc</filename> and <filename>~/.psqlrc</filename>) are
> +      ignored with this option.

Sorry if this was discussed and I missed it, but I think this is a bad 
idea. There's already an option to control this. More important, there's 
no option to force the rc files to be used, so if -g disables them you'd 
be stuck with that.

I agree that the rc files are a danger when scripting, but if we want to 
do something about that then it needs to be consistent for ALL 
non-interactive use.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com

On 8/28/15 3:31 PM, David G. Johnston wrote:
> --psqlrc​
> ​; read the standard rc files​
> --no-psqlrc ; do not read the standard rc files
>
> It belongs in a separate patch, though.
>
> In this patch -g should disable the reading of the standard rc files.

Agreed; I didn't realize -c disabled psqlrc.

> Yet another option could be added that allows the user to point to a
> different set of rc files.  Its presence should not cause the
> include/exclude behavior to change.  That way you can setup a psql
> wrapper function or alias that uses a different ​rc file while still
> having control over whether it is included or excluded.  The problem
> here is exploding the logic in order to deal with both a system and a
> user rc file.

If we had a \i variation that didn't fail if the file wasn't readable 
you could use that to pull a system psqlrc in from your custom one.
-- 
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com

Pavel,

> with -1 option support

FWIW, I have tried to apply this patch against master (7f11724) and
there is a minor error, see below.

From patch:

patching file src/bin/psql/settings.h
Hunk #2 FAILED at 135.
1 out of 2 hunks FAILED -- saving rejects to file src/bin/psql/settings.h.rej

From settings.h.rej:

--- src/bin/psql/settings.h
+++ src/bin/psql/settings.h
@@ -135,6 +141,7 @@       const char *prompt2;       const char *prompt3;       PGVerbosity verbosity;          /*
currenterror verbosity level */
 
+       GroupCommand *group_commands;} PsqlSettings;
extern PsqlSettings pset;

-Adam

-- 
Adam Brightwell - adam.brightwell@crunchydatasolutions.com
Database Engineer - www.crunchydatasolutions.com

On Sat, Oct 31, 2015 at 2:50 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> fixed patch attached

The documentation included in this patch doesn't really make it clear
why -g is different from or better than -c.


-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Nov 3, 2015 at 10:16 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> The documentation included in this patch doesn't really make it clear
>> why -g is different from or better than -c.
>
> I wrote some text. But needs some work of native speaker.

It does.  It would be nice if some kind reviewer could help volunteer
to clean that up.

Upthread, it was suggested that this option be called -C rather than
-g, and personally I like that better.  I don't really think there's
anything "grouped" about the -g option; it's just an upgraded version
of -c that does what we probably should have had -C do from the
beginning, but now don't want to change out of a concern for
backward-compatibility.  I would propose to change not only the
user-visible option name but all of the internal things that call this
"group" or "grouped".  Maybe introduce ACT_COMMAND_LINE or similar
instead of ACT_GROUP_COMMANDS.

Whatever else we do here, -1 on having both _MainLoop and MainLoop as
function names.  That can't be anything but confusing.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Nov 5, 2015 at 5:27 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> I wrote some text. But needs some work of native speaker.
>
> It does.  It would be nice if some kind reviewer could help volunteer
> to clean that up.

I'll give it a go sometime next week.

On Thu, Nov 5, 2015 at 3:53 PM, Catalin Iacob <iacobcatalin@gmail.com> wrote:
> On Thu, Nov 5, 2015 at 5:27 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> I wrote some text. But needs some work of native speaker.
>>
>> It does.  It would be nice if some kind reviewer could help volunteer
>> to clean that up.
>
> I'll give it a go sometime next week.

Thanks, that would be great!

I recommend comparing the section on -c and the section on -C, and
probably updating the former as well as adjusting the wording of the
latter.  We don't want to repeat all the same details in both places,
but we hopefully want to give people a little clue that if they're
thinking about using -c, they may wish to instead consider -C.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Tue, Nov 10, 2015 at 2:18 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> Hi
>
> 2015-11-05 22:23 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
>>
>> On Thu, Nov 5, 2015 at 3:53 PM, Catalin Iacob <iacobcatalin@gmail.com>
>> wrote:
>> > On Thu, Nov 5, 2015 at 5:27 PM, Robert Haas <robertmhaas@gmail.com>
>> > wrote:
>> >>> I wrote some text. But needs some work of native speaker.
>> >>
>> >> It does.  It would be nice if some kind reviewer could help volunteer
>> >> to clean that up.
>> >
>> > I'll give it a go sometime next week.
>>
>> Thanks, that would be great!
>>
>> I recommend comparing the section on -c and the section on -C, and
>> probably updating the former as well as adjusting the wording of the
>> latter.  We don't want to repeat all the same details in both places,
>> but we hopefully want to give people a little clue that if they're
>> thinking about using -c, they may wish to instead consider -C.

Just catching up with this thread... Using a separate option looks
fine to me, and it's definitely better to leave -c alone due to its
transactional behavior. I guess that it is true that more than one
person got caught by the fact that -c was running all its stuff within
a single transaction, particularly when having queries that do not
like transaction blocks.

> -g was replaced by -C option and some other required changes.
>
> I have not idea about good long name. In this moment I used "multi-command".
> Can be changed freely.

Or --command-multi, or --multiple-commands, though I have a good
history here at choosing bad names.

> The name of this patch is same (although it doesn't use "group-command"
> internally anymore) due better orientation.

I have been looking this patch a bit, and here are some comments:
   /*    * process slash command if one was given to -c    */   else if (options.action == ACT_SINGLE_SLASH)
This comment needs to be updated.

+       else if (options.action == ACT_COMMAND_LINE)
+       {
+               pset.notty = true;
+
+               /* use singleline mode, doesn't need semicolon on the
end line */
+               SetVariableBool(pset.vars, "SINGLELINE");
Er, enforcing an option is not user-friendly.

+                       /* Is there some unprocessed multi command? */
"Check presence of unprocessed commands"

@@ -451,7 +491,6 @@ MainLoop(FILE *source)       return successResult;}      /* MainLoop() */

-/*
This is unnecessary diff noise.

+               fprintf(stderr, _("%s: options -c/--command and
-C/--multi_command cannot be used together\n"),
+                               pset.progname);
I would rather formulate that as "cannot use --opt1 and --opt2 together".

+      <term><option>-C <replaceable
class="parameter">command(s)</replaceable></></term>
Don't think you need the "(s)" here.

+      <para>
+      Specifies that <application>psql</application> is to execute one or
+      more command strings, <replaceable
class="parameter">commands</replaceable>,
+      and then exit. This is useful in shell scripts. Start-up files
+      (<filename>psqlrc</filename> and <filename>~/.psqlrc</filename>) are
+      ignored with this option.
+      </para>
This is a copy-paste of the same paragraph for option -c.

It seems to me that the documentation should specify that when -C is
used with -1 each individual series of commands is executed within a
transaction block. As far as I understood this command:
psql -1 -c 'SELECT 1;SELECT 2' -c 'SELECT 3;SELECT4'
is equivalent to that:
BEGIN:
SELECT 1;
SELECT 2;
COMMIT;
BEGIN:
SELECT 3;
SELECT 4;
COMMIT;

s/commads/commands/, and the documentation needs a good brush up:
- The first paragraph is a copy of what is used for -c
- Specifying multiple times -C concatenates those series of commands
into mutiple subsets running in their own transaction.
- Documentation should clearly mention what the interactions with -1.
Regards,
-- 
Michael

On Thu, Nov 12, 2015 at 9:35 AM, David G. Johnston
<david.g.johnston@gmail.com> wrote:
> On Wed, Nov 11, 2015 at 7:01 AM, Michael Paquier <michael.paquier@gmail.com>
> wrote:
>>
>> It seems to me that the documentation should specify that when -C is
>> used with -1 each individual series of commands is executed within a
>> transaction block.
>
> In summary:
>
> Default (Not Single + Auto-Commit): One Transactions per parsed statement in
> all -Cs [<neither option specified>]
> Single + Auto-Commit: One Transaction per -C [--single-transaction] {same as
> --no-auto-commit]
> Not Single + Not Auto-Commit: One Transaction per -C [--no-auto-commit]
> {same as --single-transaction}
> Single + Not Auto-Commit: One Transaction covering all -Cs [--no-auto-commit
> --single-transaction]
> Explanation:

I am assuming you refer to this command in your analysis: "#1
Statement #1" being the first statement of the first -C, "#1 Statement
#2" the second statement in the first -C switch, and "Statement Only"
the statement of of a second -C switch. Or simply that:
psql -C 'Statement1,Statement2' -C 'Statement'

> The transactional behavior of -C
> can, with defaults, be described thusly:
> BEGIN:
> -C #1 Statement #1
> COMMIT;
> BEGIN;
> -C #1 Statement #2
> COMMIT;
> BEGIN;
> -C #2 Statement Only
> COMMIT;

Yep, that's what it does by going though MainLoop().

> Basically the explicit representation of Auto-Commit "on" Mode
>
> I don't understand how -c implements the promise of:
> """
> If the command string contains multiple SQL commands, they are processed in
> a single transaction, unless there are explicit BEGIN/COMMIT commands
> included in the string to divide it into multiple transactions.
> """

-c simply processes everything it has within libpq in one shot. This
code path does not care about --single-transaction, and it relies on a
backend check to be sure that nothing not allowed within a transaction
block runs when multiple queries are passed though it.

> But my gut (and Pavel) says that this is "legacy behavior" that should not
> be carried over to -C.  I would suggest going further and disallowing
> transaction control statements within -C commands.
>
> Now, in the presence of "--single-transaction" we would convert the
> transactional behavior from that shown above to:
>
> BEGIN;
> -C #1 Statement #1
> -C #1 Statement #2
> COMMIT; -- auto-committed;
> BEGIN;
> -C #2
> COMMIT;

Correct.

> Additionally, if the variable AUTOCOMMIT is "off" then the implicit script
> should look like:
>
> BEGIN;
> -C #1 Statement #1
> -C #2 Statement #2
> -C #2
> COMMIT;
> So a "true" single transaction requires setting AUTOCOMMIT to off otherwise
> you only get each -C singly.

Yeah, that's what -c does actually by relying on the backend to check
incompatibilities regarding stuff that should not run in transaction
blocks.

> I would suggest adding an action "--no-auto-commit" option to complete the
> existence of the "--single-transaction" option.  While the variable method
> works it doesn't feel as clean now that we are adding this option that (can)
> make direct use of it.
>
> Specifying only --no-auto-commit results in:
> BEGIN;
> -C #1 Statement #1
> -C #1 Statement #2
> COMMIT;
> BEGIN;
> -C #2
> COMMIT;
> Which is redundant with specifying only "--single-transaction".  Each -C
> still commits otherwise you would just use the default.

Don't you mean that actually:
BEGIN;
-C #1 Statement #1
-C #1 Statement #2
-C #2
COMMIT;
By the way there is no much point to this option. It seems to me that
it is already possible to do it with --set AUTOCOMMIT=off.
-- 
Michael

So I promised I'd try to document this. I had a look at the proposed
semantics of -C and I think in the patch they're too complicated which
makes explaining them hard.

My assumptions about behaviour without this patch, from reading the
docs and some experimenting, correct me if I'm wrong:

1. psql normally splits its input by ; let's call each piece of the
split a statement

2. for every statement resulting after 1, if it's a \ command it's
interpreted internally, else a query with it is sent to the server,
the result is displayed

3. 1. and 2. happen when the input comes from a file (-f) or from stdin

4. autocommit off changes behaviour in that it sends a BEGIN before
any of the statements after the split in 1 (except for \ commands,
BEGIN or things like VACUUM which don't work within transactions)

5. --single-transaction changes behaviour in that it puts a BEGIN
before the whole input (not around each statement) and a COMMIT after

6. all of the above DON'T apply for -c which very different things: it
doesn't split and instead it sends everything, in one query to the
backend. The backend can execute such a thing (it splits itself by ;)
except in some cases like SELECT + VACUUM. Since the single query is
effectively a single transaction for the backend -c ignores
--single-transaction and autocommit off. Even more, when executing
such a multiple statement the backend only returns results for the
last statement of the query.

From the above it seems -c is a different thing altogether while other
behaviour allows 1 input with multiple commands, multiple results and
works the same on stdin and a file.

So my proposal is: allow a *single* argument for -C and treat its
content *exactly* like the input from stdin or from a file.

This answers all the questions about interactions with
--single-transaction and autocommit naturally: it behaves exactly like
stdin and -f behave today. And having a single parameter is similar to
having a single file or single stdin. Having multiple -C is also
confusing since it seems the statements in one -C are grouped somehow
and the ones in the next -C are another group so this starts feeling
like there's maybe a transaction per -C group etc.

Am I missing something or is it that simple?

On 11/13/2015 03:54 PM, Catalin Iacob wrote:
>
> So my proposal is: allow a *single* argument for -C and treat its
> content *exactly* like the input from stdin or from a file.


That seems to me to get rid of the main motivation for this change, 
which is to allow multiple such arguments, which together would as as if 
they were all written to a file which was then invoked like -f file.

If we can only have a single such argument then the change is of 
comparatively little value.

cheers

andrew

On Sun, Nov 15, 2015 at 1:27 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
> That seems to me to get rid of the main motivation for this change, which is
> to allow multiple such arguments, which together would as as if they were
> all written to a file which was then invoked like -f file.

It seems to me the motivation is not "multiple command line arguments"
but sending multiple statements to the backend in one psql invocation
without needing bash specific here docs or a temporary file for -f.
Most combinations of such multiple statements can already be sent via
-c which sends them in one query, the backend executes them in one
transaction but the backend rejects some combinations like SELECT +
VACUUM.

I think the proposal was mislead by the apparent similarity with -c
and said "if -c can't do SELECT + VACUUM let's do a sort of repeated
-c and call that -C SELECT -C VACUUM". But why not do the same with -C
"SELECT 1; VACUUM" which works just like having a file with that
content works today but handier for scripts? Doesn't this solve the
exact need in this thread?

I'm arguing that sending multiple statements and executing each in one
transaction (the current proposed semantics of -C) is not like -c and
doesn't need to be "repeated -c" it's exactly like reading stdin or
file passed to -f and solves the original problem.But maybe I'm
missing something.

On 11/15/2015 08:50 AM, Catalin Iacob wrote:
> On Sun, Nov 15, 2015 at 1:27 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>> That seems to me to get rid of the main motivation for this change, which is
>> to allow multiple such arguments, which together would as as if they were
>> all written to a file which was then invoked like -f file.
> It seems to me the motivation is not "multiple command line arguments"
> but sending multiple statements to the backend in one psql invocation
> without needing bash specific here docs or a temporary file for -f.
> Most combinations of such multiple statements can already be sent via
> -c which sends them in one query, the backend executes them in one
> transaction but the backend rejects some combinations like SELECT +
> VACUUM.
>
> I think the proposal was mislead by the apparent similarity with -c
> and said "if -c can't do SELECT + VACUUM let's do a sort of repeated
> -c and call that -C SELECT -C VACUUM". But why not do the same with -C
> "SELECT 1; VACUUM" which works just like having a file with that
> content works today but handier for scripts? Doesn't this solve the
> exact need in this thread?
>
> I'm arguing that sending multiple statements and executing each in one
> transaction (the current proposed semantics of -C) is not like -c and
> doesn't need to be "repeated -c" it's exactly like reading stdin or
> file passed to -f and solves the original problem.But maybe I'm
> missing something.
>



I suggest you review the original thread on this subject before a line 
was ever written. "multiple" (see subject line on this whole thread) is 
clearly what is being asked for. Making people turn that into a single 
argument is not what was envisaged. See for example Pavel's original 
example involving use of xargs where that's clearly not at all easy.

cheers

andrew

On 11/15/15 9:53 AM, Andrew Dunstan wrote:
> I suggest you review the original thread on this subject before a line
> was ever written. "multiple" (see subject line on this whole thread) is
> clearly what is being asked for. Making people turn that into a single
> argument is not what was envisaged. See for example Pavel's original
> example involving use of xargs where that's clearly not at all easy.

I can see (small) value in having a new option that is like -c but
interprets the string as a fully-featured script like -f.  (Small
because the same behavior can already be had with here strings in bash.)

The behavior should be exactly like -f, including all the behavior with
single-transaction and single-step modes or whatever.

But then I will point out that we currently don't handle multiple -f
options.

On 11/15/2015 08:24 PM, Peter Eisentraut wrote:
> On 11/15/15 9:53 AM, Andrew Dunstan wrote:
>> I suggest you review the original thread on this subject before a line
>> was ever written. "multiple" (see subject line on this whole thread) is
>> clearly what is being asked for. Making people turn that into a single
>> argument is not what was envisaged. See for example Pavel's original
>> example involving use of xargs where that's clearly not at all easy.
> I can see (small) value in having a new option that is like -c but
> interprets the string as a fully-featured script like -f.  (Small
> because the same behavior can already be had with here strings in bash.)
>
> The behavior should be exactly like -f, including all the behavior with
> single-transaction and single-step modes or whatever.
>
> But then I will point out that we currently don't handle multiple -f
> options.
>
>


If we can only have one I would say the value is vanishingly small.

As to -f, I don't see why we shouldn't allow multiple such options, only 
that nobody has bothered to do it.

cheers

andrew

On Sun, Nov 15, 2015 at 3:53 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
> I suggest you review the original thread on this subject before a line was
> ever written. "multiple" (see subject line on this whole thread) is clearly
> what is being asked for. Making people turn that into a single argument is
> not what was envisaged. See for example Pavel's original example involving
> use of xargs where that's clearly not at all easy.

I couldn't see why it would matter to have multiple -C, but xargs
having -n which consumes more than 1 stdin item is indeed an use case.
When reading the thread I didn't notice it since I didn't know what -n
does.

But multiple -C is confusing since it suggests the groupings matter.

To me at least, it feels weird that -C "SELECT 1; SELECT 2;" -C
"SELECT 3;" is the same as -C "SELECT 1; SELECT 2; SELECT 3" and lots
of other combinations. It feels like the split in groups must mean
something, otherwise why would you support/use multiple groups?

Upthread at least somebody thought each -C group would/should be a
transaction and I can see this confusion coming up again and again,
enough to question whether this patch is an improvement over the
current situation. And if a single -C is too small of an improvement,
maybe it means the whole idea should be dropped. I think the same of
multiple -f as well: to me they're more confusing than helpful for the
same reason.

On 11/16/2015 11:16 AM, Catalin Iacob wrote:
> On Sun, Nov 15, 2015 at 3:53 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>> I suggest you review the original thread on this subject before a line was
>> ever written. "multiple" (see subject line on this whole thread) is clearly
>> what is being asked for. Making people turn that into a single argument is
>> not what was envisaged. See for example Pavel's original example involving
>> use of xargs where that's clearly not at all easy.
> I couldn't see why it would matter to have multiple -C, but xargs
> having -n which consumes more than 1 stdin item is indeed an use case.
> When reading the thread I didn't notice it since I didn't know what -n
> does.
>
> But multiple -C is confusing since it suggests the groupings matter.
>
> To me at least, it feels weird that -C "SELECT 1; SELECT 2;" -C
> "SELECT 3;" is the same as -C "SELECT 1; SELECT 2; SELECT 3" and lots
> of other combinations. It feels like the split in groups must mean
> something, otherwise why would you support/use multiple groups?
>
> Upthread at least somebody thought each -C group would/should be a
> transaction and I can see this confusion coming up again and again,
> enough to question whether this patch is an improvement over the
> current situation. And if a single -C is too small of an improvement,
> maybe it means the whole idea should be dropped. I think the same of
> multiple -f as well: to me they're more confusing than helpful for the
> same reason.
>


I honestly don't see what's so confusing about it, and if there is any 
confusion then surely we could make sure what's happening is well 
documented.

cheers

andrew

On Mon, Nov 16, 2015 at 6:05 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
> I honestly don't see what's so confusing about it, and if there is any
> confusion then surely we could make sure what's happening is well
> documented.

+1.  I'm actually kind of wondering if we should just back up and
change the way -c works instead, and allow it to be specified more
than once.  The current behavior is essentially a crock that has only
backward compatibility to recommend it, and not having two
confusingly-similar options is of some non-trivial value.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Nov 16, 2015 at 6:05 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>> I honestly don't see what's so confusing about it, and if there is any
>> confusion then surely we could make sure what's happening is well
>> documented.

> +1.  I'm actually kind of wondering if we should just back up and
> change the way -c works instead, and allow it to be specified more
> than once.  The current behavior is essentially a crock that has only
> backward compatibility to recommend it, and not having two
> confusingly-similar options is of some non-trivial value.

Well, it's not *entirely* true that it has only backwards compatibility
to recommend it: without -c in its current form, there would be no way
to test multiple-commands-in-one-PQexec cases without hacking up some
custom test infrastructure.  That's not a very strong reason maybe, but
it's a reason.  And backwards compatibility is usually a strong argument
around here anyway.

I've not been following this thread in any detail, but have we considered
the approach of allowing multiple -c and saying that each -c is a separate
PQexec (or backslash command)?  So the semantics of one -c wouldn't change,
but commands submitted through multiple -c switches would behave
relatively unsurprisingly, and we wouldn't need two kinds of switch.

Another issue here is how -1 ought to interact with multiple -c.
        regards, tom lane

On Tue, Nov 17, 2015 at 2:25 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Mon, Nov 16, 2015 at 6:05 PM, Andrew Dunstan <andrew@dunslane.net> wrote:
>>> I honestly don't see what's so confusing about it, and if there is any
>>> confusion then surely we could make sure what's happening is well
>>> documented.
>
>> +1.  I'm actually kind of wondering if we should just back up and
>> change the way -c works instead, and allow it to be specified more
>> than once.  The current behavior is essentially a crock that has only
>> backward compatibility to recommend it, and not having two
>> confusingly-similar options is of some non-trivial value.
>
> Well, it's not *entirely* true that it has only backwards compatibility
> to recommend it: without -c in its current form, there would be no way
> to test multiple-commands-in-one-PQexec cases without hacking up some
> custom test infrastructure.  That's not a very strong reason maybe, but
> it's a reason.

True.  We could have a --no-split-commands option for that case, perhaps.

> And backwards compatibility is usually a strong argument
> around here anyway.

Yeah, but not - at least in my book - at the expense of being stuck
with a confusing interface forever.

> I've not been following this thread in any detail, but have we considered
> the approach of allowing multiple -c and saying that each -c is a separate
> PQexec (or backslash command)?  So the semantics of one -c wouldn't change,
> but commands submitted through multiple -c switches would behave
> relatively unsurprisingly, and we wouldn't need two kinds of switch.
>
> Another issue here is how -1 ought to interact with multiple -c.

On a code level, I think the issue here is that ACT_SINGLE_QUERY
bypasses a lot of stuff that happens in the ACT_FILE case: directly in
main, there's process_psqlrc(); inside process_file(), there's the
single_txn handling; then inside MainLoop, there's splitting of
commands and cancel handling and various other stuff.  In my
imagination, it's like this because originally psql wasn't nearly as
complicated as it is now, and as features got added in various places,
-c gradually diverged.  I don't know whether that's really what
happened, but it seems to me that it would be good to bring those
things back together.

A few years ago there was a proposal to not only allow multiple -c
options, but to allow -c and -f to be intermingled.  This seems really
rather useful; I'd like to be able to type psql -c do_this_first -f
script.sql and have that work.  But of course it's kind of hard to
figure out how that should behave given the various differences
between -c and -f.  I think in the long run we'll be better off
rationalizing the interface; I really doubt how many people, even on
this mailing list, can even enumerate all the differences between -c
and -f.  If it's too complicated for hackers to remember, it's
probably not very good for users either.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> A few years ago there was a proposal to not only allow multiple -c
> options, but to allow -c and -f to be intermingled.  This seems really
> rather useful; I'd like to be able to type psql -c do_this_first -f
> script.sql and have that work.  But of course it's kind of hard to
> figure out how that should behave given the various differences
> between -c and -f.

Hm.  That's actually a good reason for changing -c, I guess.  (Or else
introducing a -C that is compatible with -f, but I agree that that
seems a bit ugly.)

If we made -c handle its input just like it had come from a file,
then what would we need to explain to people that wanted the old
behavior?  I guess we'd need to tell them to use --no-psqlrc and
--single-transaction at least, and "-v ON_ERROR_STOP=1".  And
even then it wouldn't replicate the behavior of discarding all
but the last command output.  (Maybe nobody out there is relying
on that, but I wouldn't bet on it.)  And that's all still assuming
that they don't care about multi-command-per-PQexec in itself, but
only the easily user-visible differences.

So that is kind of looking like a mess, and 90% of the mess is from
not wanting to use a PQexec per -c switch, which if you ask me is
not very much of the value-add here.  AFAICS the only thing that's
really in conflict with -f is the implied --no-psqlrc.  How about
this design:

1. -c no longer implies --no-psqlrc.  That's a backwards incompatibility,
but very easy to explain and very easy to work around.

2. You can have multiple -c and/or -f.  Each -c is processed in
the traditional way, ie, either it's a single backslash command
or it's sent in a single PQexec.  That doesn't seem to me to have
much impact on the behavior of adjacent -c or -f.

3. If you combine -1 with -c and/or -f, you get one BEGIN inserted
at the beginning and one COMMIT at the end.  Nothing else changes.

As long as you put only one SQL command per -c, I don't think that
this definition has any real surprises.  And we can discourage
people from putting more than one, saying that that will invoke
legacy behaviors you probably don't want.
        regards, tom lane

On 11/17/2015 04:13 PM, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> A few years ago there was a proposal to not only allow multiple -c
>> options, but to allow -c and -f to be intermingled.  This seems really
>> rather useful; I'd like to be able to type psql -c do_this_first -f
>> script.sql and have that work.  But of course it's kind of hard to
>> figure out how that should behave given the various differences
>> between -c and -f.
> Hm.  That's actually a good reason for changing -c, I guess.  (Or else
> introducing a -C that is compatible with -f, but I agree that that
> seems a bit ugly.)
>
> If we made -c handle its input just like it had come from a file,
> then what would we need to explain to people that wanted the old
> behavior?  I guess we'd need to tell them to use --no-psqlrc and
> --single-transaction at least, and "-v ON_ERROR_STOP=1".  And
> even then it wouldn't replicate the behavior of discarding all
> but the last command output.  (Maybe nobody out there is relying
> on that, but I wouldn't bet on it.)  And that's all still assuming
> that they don't care about multi-command-per-PQexec in itself, but
> only the easily user-visible differences.
>
> So that is kind of looking like a mess, and 90% of the mess is from
> not wanting to use a PQexec per -c switch, which if you ask me is
> not very much of the value-add here.  AFAICS the only thing that's
> really in conflict with -f is the implied --no-psqlrc.  How about
> this design:
>
> 1. -c no longer implies --no-psqlrc.  That's a backwards incompatibility,
> but very easy to explain and very easy to work around.
>
> 2. You can have multiple -c and/or -f.  Each -c is processed in
> the traditional way, ie, either it's a single backslash command
> or it's sent in a single PQexec.  That doesn't seem to me to have
> much impact on the behavior of adjacent -c or -f.
>
> 3. If you combine -1 with -c and/or -f, you get one BEGIN inserted
> at the beginning and one COMMIT at the end.  Nothing else changes.
>
> As long as you put only one SQL command per -c, I don't think that
> this definition has any real surprises.  And we can discourage
> people from putting more than one, saying that that will invoke
> legacy behaviors you probably don't want.
>
>             


WFM. The only reason I originally suggested -C was to avoid 
compatibility issues. If we're prepared to take that on then I agree 
it's better to do what you suggest.

I assume that we won't have any great difficulty in handling 
intermingled -c and -f options in the correct order. Essentially I think 
we'll have to have a unified list of such arguments.

cheers

andrew

On Tue, Nov 17, 2015 at 10:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 1. -c no longer implies --no-psqlrc.  That's a backwards incompatibility,
> but very easy to explain and very easy to work around.
>
> 2. You can have multiple -c and/or -f.  Each -c is processed in
> the traditional way, ie, either it's a single backslash command
> or it's sent in a single PQexec.  That doesn't seem to me to have
> much impact on the behavior of adjacent -c or -f.
>
> 3. If you combine -1 with -c and/or -f, you get one BEGIN inserted
> at the beginning and one COMMIT at the end.  Nothing else changes.

And -v AUTOCOMMIT=off should do the same as now for -c: issue a BEGIN
before each single PQexec with the content of each -c.

I like it, it avoids what I didn't like about -C semantics since the
grouping now means something (single PQexec per group) and you even
see the effects of the grouping in the result (only last result of
group is returned). If you don't like that grouping (probably most
people won't) the solution is intuitive: split the group to multiple
-c.

Another incompatibility is that -1 is now silently ignored by -c so
for example psql -1 -c VACUUM now works and it won't work with the new
semantics. But this seems like a good thing because it reflects that
VACUUM doesn't work in a transaction so I don't think it should stop
this proposal from going ahead.

I'll try to write the documentation patch for these semantics sometime
next week.

Pavel Stehule <pavel.stehule@gmail.com> writes:
>> 1. -c no longer implies --no-psqlrc.  That's a backwards incompatibility,
>> but very easy to explain and very easy to work around.

> This can be very surprising change. Can we disable it temporary by some
> environment variable? like NOPSQLRC ?

Why would that be better than "add -X to the psql call"?

I think generally the idea here is to have fewer warts, not more.
        regards, tom lane

On Wed, Nov 18, 2015 at 3:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Pavel Stehule <pavel.stehule@gmail.com> writes:
>>> 1. -c no longer implies --no-psqlrc.  That's a backwards incompatibility,
>>> but very easy to explain and very easy to work around.
>
>> This can be very surprising change. Can we disable it temporary by some
>> environment variable? like NOPSQLRC ?
>
> Why would that be better than "add -X to the psql call"?
>
> I think generally the idea here is to have fewer warts, not more.

Amen to that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Wed, Nov 18, 2015 at 5:49 PM, Catalin Iacob <iacobcatalin@gmail.com> wrote:
> On Tue, Nov 17, 2015 at 10:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 1. -c no longer implies --no-psqlrc.  That's a backwards incompatibility,
>> but very easy to explain and very easy to work around.
>>
>> 2. You can have multiple -c and/or -f.  Each -c is processed in
>> the traditional way, ie, either it's a single backslash command
>> or it's sent in a single PQexec.  That doesn't seem to me to have
>> much impact on the behavior of adjacent -c or -f.
>>
>> 3. If you combine -1 with -c and/or -f, you get one BEGIN inserted
>> at the beginning and one COMMIT at the end.  Nothing else changes.

> I'll try to write the documentation patch for these semantics sometime
> next week.

Attached is my attempt at a documentation patch, feedback welcome. I'm
assuming Pavel will pick up the implementation, if not I could also
try it.

On Thu, Nov 26, 2015 at 4:21 AM, Pavel Stehule wrote:
> Attached patch per Tom Lane proposal.
>
> * multiple -c -f options are supported, the order of options is respected
> * the statements for one -c options are executed in transactions
> * Iacob's doc patch merged
enum _actions{       ACT_NOTHING = 0,
-       ACT_SINGLE_SLASH,       ACT_LIST_DB,
-       ACT_SINGLE_QUERY,
-       ACT_FILE
+       ACT_FILE_STDIN};

Removing some items from the list of potential actions and creating a
new sublist listing action types is a bit weird. Why not grouping them
together and allow for example -l as well in the list of things that
is considered as a repeatable action? It seems to me that we could
simplify the code this way, and instead of ACT_NOTHING we could check
if the list of actions is empty or not.
-- 
Michael

On Tue, Dec 1, 2015 at 11:46 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> 2015-11-30 15:17 GMT+01:00 Michael Paquier <michael.paquier@gmail.com>:
>> Removing some items from the list of potential actions and creating a
>> new sublist listing action types is a bit weird. Why not grouping them
>> together and allow for example -l as well in the list of things that
>> is considered as a repeatable action? It seems to me that we could
>> simplify the code this way, and instead of ACT_NOTHING we could check
>> if the list of actions is empty or not.
>
>
> fixed

Thanks for the patch. I have to admit that adding ACT_LIST_DB in the
list of actions was not actually a good idea. It makes the patch
uglier.

Except that, I just had an in-depth look at this patch, and there are
a couple of things that looked strange:
- ACT_LIST_DB would live better if removed from the list of actions
and be used as a separate, independent option. My previous suggestion
was unadapted. Sorry.
- There is not much meaning to have simple_action_list_append and all
its structures in common.c and common.h. Its use is limited in
startup.c (this code is basically a duplicate of dumputils.c still
things are rather different, justifying the duplication) and
centralized around parse_psql_options.
- use_stdin is not necessary. It is sufficient to rely on actions.head
== NULL instead.
- The documentation is pretty clear. That's nice.
Attached is a patch implementing those suggestions. This simplifies
the code without changing its usefulness. If you are fine with those
changes I will switch this patch as ready for committer.
Regards,
--
Michael

On Tue, Dec 1, 2015 at 1:53 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Attached is a patch implementing those suggestions. This simplifies
> the code without changing its usefulness. If you are fine with those
> changes I will switch this patch as ready for committer.

I tested the v07 patch (so not Michael's version) a few days ago but
didn't send this email earlier.

I combined various -c and -f with --echo-all, --single-transaction,
\set ON_ERROR_STOP=1, separate -c "VACUUM", "SELECT + VACUUM" in a
single and in two -c, inserting -f - somewhere in the middle of the
other -c and -f. They all behave as I would expect.

One maybe slightly surprising behaviour is that -f - can be specified
multiple times and only the first one has an effect since the others
act on an exhausted stdin. But I don't think forbidding multiple -f -
is better.

As for the code (these still apply to Michael's latest patch):

1. the be compiler quiete comment is not good English, /* silence the
compiler */ would be better or remove it completely

2. shouldn't atyp in SimpleActionListCell be of type enum _atypes?
Otherwise why an enum if it's casted to int when actually used? If
it's an enum the repeated ifs on cell->atyp should be a switch, either
with a default Assert(0) or no default which makes gcc give a warning
if an enum value is ever added without having a corresponding case.

On Wed, Dec 2, 2015 at 2:56 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> 2015-12-01 17:52 GMT+01:00 Catalin Iacob <iacobcatalin@gmail.com>:
>> One maybe slightly surprising behaviour is that -f - can be specified
>> multiple times and only the first one has an effect since the others
>> act on an exhausted stdin. But I don't think forbidding multiple -f -
>> is better.

I don't see any good reason to forbid it actually. This simplifies the
code and it's not like this would break psql.

>> As for the code (these still apply to Michael's latest patch):
>>
>> 1. the be compiler quiete comment is not good English, /* silence the
>> compiler */ would be better or remove it completely

Fixed. Indeed I didn't notice that.

>> 2. shouldn't atyp in SimpleActionListCell be of type enum _atypes?
>> Otherwise why an enum if it's casted to int when actually used? If
>> it's an enum the repeated ifs on cell->atyp should be a switch, either
>> with a default Assert(0) or no default which makes gcc give a warning
>> if an enum value is ever added without having a corresponding case.
> It is maybe different topic - the psql uses enums and ints very freely. So I
> wrote code consistent with current code.

Yeah, I don't think that's a big issue either to be honest. The code
is kept consistent a maximum with what is there previously.

Patch is switched to ready for committer.
--
Michael

On Wed, Dec 2, 2015 at 12:33 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> Yeah, I don't think that's a big issue either to be honest. The code
>> is kept consistent a maximum with what is there previously.
>>
>> Patch is switched to ready for committer.
>
> perfect
>
> Thank you very much to all

I did some edits on this patch and was all set to commit it when I ran
the regression tests and discovered that this breaks 130 out of the
160 regression tests. Allow me to suggest that before submitting a
patch, or marking it ready for commiter, you test that 'make check'
passes.

The problem seems to be the result of this code:

+        if (options.actions.head == NULL && pset.notty)
+                simple_action_list_append(&options.actions, ACT_FILE, "-");

The problem with this is that process_file() gets called with "-"
where it previously got called with NULL, which changes the way error
reports are printed. This would be trivial to fix were it not for the
fact that SimpleActionListCell uses char val[FLEXIBLE_ARRAY_MEMBER]
rather than char *val.  I think you should change it so that it does
the latter, and then change the above line to pass NULL for the third
argument.  I think that will fix it, but it's more work than I want to
do on somebody else's patch, so I'm attaching my edited version here
for further work.

For the most part, the cleanups in this version are just cosmetic: I
fixed some whitespace damage, and reverted some needless changes to
the psql references page that were whitespace-only adjustments.  In a
few places, I tweaked documentation or comment language.  I also
hoisted the psqlrc handling out of an if statement where it was the
same in both branches.  Other than that, this version is, I believe,
the same as Pavel's last version.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Fri, Dec 4, 2015 at 3:47 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> For the most part, the cleanups in this version are just cosmetic: I
> fixed some whitespace damage, and reverted some needless changes to
> the psql references page that were whitespace-only adjustments.  In a
> few places, I tweaked documentation or comment language.

Sorry for the docs whitespace-only changes, I did that.

I realized before the submission I made the diff bigger than it needed
to be, but that's because I used M-q in Emacs to break the lines I did
change and that reformatted the whole paragraph including some
unchanged lines. Breaking all the lines by hand would be quite a job,
and any time you go back and tweak the wording or so you need to do it
again. So I just used M-q and sent the result of that.
Do you happen to know of a better way to do this? I do load
src/tools/editors/emacs.samples in my ~/.emacs but it seems the width
my Emacs chooses doesn't match the one already in the file.

The doc tweaks are good, they make the text more clear. I'm happy
that's all you found to improve: writing good docs is hard and the
Postgres docs are already good so it's not easy to change them, I had
the feeling I'll only make them worse and spent quite some time trying
not to do that.

On Fri, Dec 4, 2015 at 11:47 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Dec 2, 2015 at 12:33 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>> Yeah, I don't think that's a big issue either to be honest. The code
>>> is kept consistent a maximum with what is there previously.
>>>
>>> Patch is switched to ready for committer.
>>
>> perfect
>>
>> Thank you very much to all
>
> I did some edits on this patch and was all set to commit it when I ran
> the regression tests and discovered that this breaks 130 out of the
> 160 regression tests. Allow me to suggest that before submitting a
> patch, or marking it ready for commiter, you test that 'make check'
> passes.

Mea culpa. I thought I did a check-world run... But well...

> For the most part, the cleanups in this version are just cosmetic: I
> fixed some whitespace damage, and reverted some needless changes to
> the psql references page that were whitespace-only adjustments.  In a
> few places, I tweaked documentation or comment language.  I also
> hoisted the psqlrc handling out of an if statement where it was the
> same in both branches.  Other than that, this version is, I believe,
> the same as Pavel's last version.

Thanks, I looked at that again and problem is fixed as attached.
--
Michael

On Sun, Dec 6, 2015 at 10:56 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> Thanks, I looked at that again and problem is fixed as attached.

Er, not exactly... poll_query_until in PostgresNode.pm is using psql
-c without the --no-psqlrc switch so this patch causes the regression
tests of pg_rewind to fail. Fixed as attached.
--
Michael

On Fri, Dec 4, 2015 at 11:08 AM, Catalin Iacob <iacobcatalin@gmail.com> wrote:
> On Fri, Dec 4, 2015 at 3:47 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> For the most part, the cleanups in this version are just cosmetic: I
>> fixed some whitespace damage, and reverted some needless changes to
>> the psql references page that were whitespace-only adjustments.  In a
>> few places, I tweaked documentation or comment language.
>
> Sorry for the docs whitespace-only changes, I did that.
>
> I realized before the submission I made the diff bigger than it needed
> to be, but that's because I used M-q in Emacs to break the lines I did
> change and that reformatted the whole paragraph including some
> unchanged lines. Breaking all the lines by hand would be quite a job,
> and any time you go back and tweak the wording or so you need to do it
> again. So I just used M-q and sent the result of that.
> Do you happen to know of a better way to do this?

No.  I always redo the indentation by hand and then look at the diff
afterwards to see if there's anything I can strip out.

I also use vim rather than emacs, except when my hand is steady enough
for the magnetized needle approach.[1]

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[1] https://xkcd.com/378/

On Tue, Dec 08, 2015 at 01:51:57PM -0500, Robert Haas wrote:
> On Fri, Dec 4, 2015 at 11:08 AM, Catalin Iacob <iacobcatalin@gmail.com> wrote:
> > On Fri, Dec 4, 2015 at 3:47 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> >> For the most part, the cleanups in this version are just cosmetic: I
> >> fixed some whitespace damage, and reverted some needless changes to
> >> the psql references page that were whitespace-only adjustments.  In a
> >> few places, I tweaked documentation or comment language.
> >
> > Sorry for the docs whitespace-only changes, I did that.
> >
> > I realized before the submission I made the diff bigger than it needed
> > to be, but that's because I used M-q in Emacs to break the lines I did
> > change and that reformatted the whole paragraph including some
> > unchanged lines. Breaking all the lines by hand would be quite a job,
> > and any time you go back and tweak the wording or so you need to do it
> > again. So I just used M-q and sent the result of that.
> > Do you happen to know of a better way to do this?
> 
> No.  I always redo the indentation by hand and then look at the diff
> afterwards to see if there's anything I can strip out.

There's also an excellent git check-whitepace thing Peter Eisentraut
put together:

http://peter.eisentraut.org/blog/2014/11/04/checking-whitespace-with-git/

> I also use vim rather than emacs, except when my hand is steady enough
> for the magnetized needle approach.[1]

I figured you for more the butterfly type.

Cheers,
David.
-- 
David Fetter <david@fetter.org> http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter      XMPP: david.fetter@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

On Sun, Dec 6, 2015 at 9:27 AM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Sun, Dec 6, 2015 at 10:56 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
>> Thanks, I looked at that again and problem is fixed as attached.
>
> Er, not exactly... poll_query_until in PostgresNode.pm is using psql
> -c without the --no-psqlrc switch so this patch causes the regression
> tests of pg_rewind to fail. Fixed as attached.

Committed.  Go, team.

This has been a long time coming.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Wed, Dec 9, 2015 at 5:08 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> 2015-12-08 20:09 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
>> On Sun, Dec 6, 2015 at 9:27 AM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>> > On Sun, Dec 6, 2015 at 10:56 PM, Michael Paquier
>> > <michael.paquier@gmail.com> wrote:
>> >> Thanks, I looked at that again and problem is fixed as attached.
>> >
>> > Er, not exactly... poll_query_until in PostgresNode.pm is using psql
>> > -c without the --no-psqlrc switch so this patch causes the regression
>> > tests of pg_rewind to fail. Fixed as attached.
>>
>> Committed.  Go, team.
>>
>> This has been a long time coming.
>
>
> great, thank you very much you and all

Thanks! This is now marked as committed in the CF app...
-- 
Michael

On Wed, Dec 9, 2015 at 2:07 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> should be noted, recorded somewhere so this introduce possible compatibility
> issue - due default processing .psqlrc.

That's written in the commit log, so I guess that's fine.
-- 
Michael

On Wed, Dec 9, 2015 at 12:15 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>> On Wed, Dec 9, 2015 at 2:07 PM, Pavel Stehule <pavel.stehule@gmail.com>
>> wrote:
>> > should be noted, recorded somewhere so this introduce possible
>> > compatibility
>> > issue - due default processing .psqlrc.
>>
>> That's written in the commit log, so I guess that's fine.
>
> ook

Bruce uses the commit log to prepare the release notes, so I guess
he'll make mention of this.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Dec 10, 2015 at 11:10:55AM -0500, Robert Haas wrote:
> On Wed, Dec 9, 2015 at 12:15 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> >> On Wed, Dec 9, 2015 at 2:07 PM, Pavel Stehule <pavel.stehule@gmail.com>
> >> wrote:
> >> > should be noted, recorded somewhere so this introduce possible
> >> > compatibility
> >> > issue - due default processing .psqlrc.
> >>
> >> That's written in the commit log, so I guess that's fine.
> >
> > ook
> 
> Bruce uses the commit log to prepare the release notes, so I guess
> he'll make mention of this.

Yes, I will certainly see it.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Roman grave inscription                             +

On 12/29/15 10:38 AM, Bruce Momjian wrote:
> On Thu, Dec 10, 2015 at 11:10:55AM -0500, Robert Haas wrote:
>> On Wed, Dec 9, 2015 at 12:15 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>>>> On Wed, Dec 9, 2015 at 2:07 PM, Pavel Stehule <pavel.stehule@gmail.com>
>>>> wrote:
>>>>> should be noted, recorded somewhere so this introduce possible
>>>>> compatibility
>>>>> issue - due default processing .psqlrc.
>>>>
>>>> That's written in the commit log, so I guess that's fine.
>>>
>>> ook
>>
>> Bruce uses the commit log to prepare the release notes, so I guess
>> he'll make mention of this.
> 
> Yes, I will certainly see it.

I generally use the master branch psql for normal work, and this change
has caused massive breakage for me.  It's straightforward to fix, but in
some cases the breakage is silent, for example if you do
something=$(psql -c ...) and the .psqlrc processing causes additional
output.  I'm not sure what to make of it yet, but I want to mention it,
because I fear there will be heartache.

On Thu, Feb 4, 2016 at 8:35 PM, Peter Eisentraut <peter_e@gmx.net> wrote:
> I generally use the master branch psql for normal work, and this change
> has caused massive breakage for me.  It's straightforward to fix, but in
> some cases the breakage is silent, for example if you do
> something=$(psql -c ...) and the .psqlrc processing causes additional
> output.  I'm not sure what to make of it yet, but I want to mention it,
> because I fear there will be heartache.

I think this is a good thing to be concerned about, but I'm not sure
what to make of it either.  We could of course decide that -c implies
--no-psqlrc after all, for backward compatibility reasons.  That would
be sort of strange because then psql -c A -f B will imply --no-psqlrc
but psql -f B will not.  And that doesn't seem great either.  I'm
inclined toward thinking we should just accept that some people will
need to update their scripts, but if that turns out to make enough
people unhappy then I may regret thinking that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company