Re: proposal: multiple psql option -c

On Tue, Nov 10, 2015 at 2:18 AM, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> Hi
>
> 2015-11-05 22:23 GMT+01:00 Robert Haas <robertmhaas(at)gmail(dot)com>:
>>
>> On Thu, Nov 5, 2015 at 3:53 PM, Catalin Iacob <iacobcatalin(at)gmail(dot)com>
>> wrote:
>> > On Thu, Nov 5, 2015 at 5:27 PM, Robert Haas <robertmhaas(at)gmail(dot)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