On Tue, Dec 1, 2015 at 11:46 AM, Pavel Stehule <pavel.steh...@gmail.com> wrote:
> 2015-11-30 15:17 GMT+01:00 Michael Paquier <michael.paqu...@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
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 5899bb4..2928c92 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -38,9 +38,10 @@ PostgreSQL documentation
<productname>PostgreSQL</productname>. It enables you to type in
queries interactively, issue them to
<productname>PostgreSQL</productname>, and see the query results.
- Alternatively, input can be from a file. In addition, it provides a
- number of meta-commands and various shell-like features to
- facilitate writing scripts and automating a wide variety of tasks.
+ Alternatively, input can be from a file or from command line
+ arguments. In addition, it provides a number of meta-commands and various
+ shell-like features to facilitate writing scripts and automating a wide
+ variety of tasks.
</para>
</refsect1>
@@ -89,38 +90,45 @@ PostgreSQL documentation
<term><option>--command=<replaceable class="parameter">command</replaceable></></term>
<listitem>
<para>
- Specifies that <application>psql</application> is to execute one
- command string, <replaceable class="parameter">command</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.
+ Specifies that <application>psql</application> is to execute the given
+ command string, <replaceable class="parameter">command</replaceable>.
+ This option can be repeated and combined in any order with
+ the <option>-f</option> option.
</para>
<para>
- <replaceable class="parameter">command</replaceable> must be either
- a command string that is completely parsable by the server (i.e.,
- it contains no <application>psql</application>-specific features),
- or a single backslash command. Thus you cannot mix
- <acronym>SQL</acronym> and <application>psql</application>
- meta-commands with this option. To achieve that, you could
- pipe the string into <application>psql</application>, for example:
- <literal>echo '\x \\ SELECT * FROM foo;' | psql</literal>.
+ <replaceable class="parameter">command</replaceable> must be either a
+ command string that is completely parsable by the server (i.e., it
+ contains no <application>psql</application>-specific features), or a
+ single backslash command. Thus you cannot mix
+ <acronym>SQL</acronym> and <application>psql</application> meta-commands
+ with this option. To achieve that, you could use
+ repeated <option>-c</option> options or pipe the string
+ into <application>psql</application>, for example:
+ <literal>psql -c '\x' -c 'SELECT * FROM foo;'</literal> or
+ <literal>echo '\x \\ SELECT * FROM foo;' | psql</literal>
(<literal>\\</> is the separator meta-command.)
</para>
<para>
- If the command string contains multiple SQL commands, they are
- processed in a single transaction, unless there are explicit
- <command>BEGIN</>/<command>COMMIT</> commands included in the
- string to divide it into multiple transactions. This is
- different from the behavior when the same string is fed to
- <application>psql</application>'s standard input. Also, only
- the result of the last SQL command is returned.
+ Each command string passed to <option>-c</option> is sent to the server
+ as a single query. Because of this, the server executes it as a single
+ transaction, even if a command string contains
+ multiple <acronym>SQL</acronym> commands, unless there are
+ explicit <command>BEGIN</>/<command>COMMIT</> commands included in the
+ string to divide it into multiple transactions. Also, the server only
+ returns the result of the last <acronym>SQL</acronym> command to the
+ client. This is different from the behavior when the same string with
+ multiple <acronym>SQL</acronym> commands is fed
+ to <application>psql</application>'s standard input because
+ then <application>psql</application> sends each <acronym>SQL</acronym>
+ command separately.
</para>
<para>
- Because of these legacy behaviors, putting more than one command in
- the <option>-c</option> string often has unexpected results. It's
- better to feed multiple commands to <application>psql</application>'s
- standard input, either using <application>echo</application> as
- illustrated above, or via a shell here-document, for example:
+ Because of the execution as a single query, putting more than one
+ command in the <option>-c</option> string often has unexpected results.
+ It's better to use repeated <option>-c</option> commands or feed
+ multiple commands to <application>psql</application>'s standard input,
+ either using <application>echo</application> as illustrated above, or
+ via a shell here-document, for example:
<programlisting>
psql <<EOF
\x
@@ -183,11 +191,13 @@ EOF
<term><option>--file=<replaceable class="parameter">filename</replaceable></></term>
<listitem>
<para>
- Use the file <replaceable class="parameter">filename</replaceable>
- as the source of commands instead of reading commands interactively.
- After the file is processed, <application>psql</application>
- terminates. This is in many ways equivalent to the meta-command
- <command>\i</command>.
+ Use the file <replaceable class="parameter">filename</replaceable> as
+ the source of commands instead of reading commands interactively. This
+ option can be repeated and combined in any order with
+ the <option>-c</option> option. After the commands in
+ every <option>-c</option> command string and <option>-f</option> file
+ are processed, <application>psql</application> terminates. This option
+ is in many ways equivalent to the meta-command <command>\i</command>.
</para>
<para>
@@ -539,20 +549,21 @@ EOF
<term><option>--single-transaction</option></term>
<listitem>
<para>
- When <application>psql</application> executes a script, adding
- this option wraps <command>BEGIN</>/<command>COMMIT</> around the
- script to execute it as a single transaction. This ensures that
- either all the commands complete successfully, or no changes are
- applied.
+ When <application>psql</application> executes commands from a script
+ and/or a <option>-c</option> option, adding this option
+ wraps <command>BEGIN</>/<command>COMMIT</> around all of those
+ commands as a whole to execute them as a single transaction. This
+ ensures that either all the commands complete successfully, or no
+ changes are applied.
</para>
<para>
- If the script itself uses <command>BEGIN</>, <command>COMMIT</>,
+ If the commands themselves
+ contain <command>BEGIN</>, <command>COMMIT</>,
or <command>ROLLBACK</>, this option will not have the desired
- effects.
- Also, if the script contains any command that cannot be executed
- inside a transaction block, specifying this option will cause that
- command (and hence the whole transaction) to fail.
+ effects. Also, if an individual command cannot be executed inside a
+ transaction block, specifying this option will cause the whole
+ transaction to fail.
</para>
</listitem>
</varlistentry>
@@ -3725,7 +3736,7 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
<term><filename>psqlrc</filename> and <filename>~/.psqlrc</filename></term>
<listitem>
<para>
- Unless it is passed an <option>-X</option> or <option>-c</option> option,
+ Unless it is passed an <option>-X</option> option,
<application>psql</application> attempts to read and execute commands
from the system-wide startup file (<filename>psqlrc</filename>) and then
the user's personal startup file (<filename>~/.psqlrc</filename>), after
@@ -3819,6 +3830,13 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
</para>
</listitem>
+ <listitem>
+ <para>
+ Before <productname>PostgreSQL</productname> 9.6, <option>-c</option>
+ used to imply <option>-X</option> and therefore it wouldn't
+ read <filename>psqlrc</filename> files, that is no longer the case.
+ </para>
+ </listitem>
</itemizedlist>
</refsect1>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 438a4ec..d642803 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -916,7 +916,7 @@ exec_command(const char *cmd,
include_relative = (strcmp(cmd, "ir") == 0
|| strcmp(cmd, "include_relative") == 0);
expand_tilde(&fname);
- success = (process_file(fname, false, include_relative) == EXIT_SUCCESS);
+ success = (process_file(fname, include_relative) == EXIT_SUCCESS);
free(fname);
}
}
@@ -2284,13 +2284,12 @@ do_edit(const char *filename_arg, PQExpBuffer query_buf,
* the file from where the currently processed file (if any) is located.
*/
int
-process_file(char *filename, bool single_txn, bool use_relative_path)
+process_file(char *filename, bool use_relative_path)
{
FILE *fd;
int result;
char *oldfilename;
char relpath[MAXPGPATH];
- PGresult *res;
if (!filename)
{
@@ -2335,37 +2334,8 @@ process_file(char *filename, bool single_txn, bool use_relative_path)
oldfilename = pset.inputfile;
pset.inputfile = filename;
- if (single_txn)
- {
- if ((res = PSQLexec("BEGIN")) == NULL)
- {
- if (pset.on_error_stop)
- {
- result = EXIT_USER;
- goto error;
- }
- }
- else
- PQclear(res);
- }
-
result = MainLoop(fd);
- if (single_txn)
- {
- if ((res = PSQLexec("COMMIT")) == NULL)
- {
- if (pset.on_error_stop)
- {
- result = EXIT_USER;
- goto error;
- }
- }
- else
- PQclear(res);
- }
-
-error:
if (fd != stdin)
fclose(fd);
diff --git a/src/bin/psql/command.h b/src/bin/psql/command.h
index 54385e8..c817600 100644
--- a/src/bin/psql/command.h
+++ b/src/bin/psql/command.h
@@ -27,7 +27,7 @@ typedef enum _backslashResult
extern backslashResult HandleSlashCmds(PsqlScanState scan_state,
PQExpBuffer query_buf);
-extern int process_file(char *filename, bool single_txn, bool use_relative_path);
+extern int process_file(char *filename, bool use_relative_path);
extern bool do_pset(const char *param,
const char *value,
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 7aa997d..5869eef 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -50,13 +50,24 @@ PsqlSettings pset;
*/
enum _actions
{
- ACT_NOTHING = 0,
- ACT_SINGLE_SLASH,
- ACT_LIST_DB,
ACT_SINGLE_QUERY,
+ ACT_SINGLE_SLASH,
ACT_FILE
};
+typedef struct SimpleActionListCell
+{
+ struct SimpleActionListCell *next;
+ int action;
+ char val[FLEXIBLE_ARRAY_MEMBER];
+} SimpleActionListCell;
+
+typedef struct SimpleActionList
+{
+ SimpleActionListCell *head;
+ SimpleActionListCell *tail;
+} SimpleActionList;
+
struct adhoc_opts
{
char *dbname;
@@ -64,11 +75,11 @@ struct adhoc_opts
char *port;
char *username;
char *logfilename;
- enum _actions action;
- char *action_string;
bool no_readline;
bool no_psqlrc;
bool single_txn;
+ bool list_dbs;
+ SimpleActionList actions;
};
static void parse_psql_options(int argc, char *argv[],
@@ -76,6 +87,8 @@ static void parse_psql_options(int argc, char *argv[],
static void process_psqlrc(char *argv0);
static void process_psqlrc_file(char *filename);
static void showVersion(void);
+static void simple_action_list_append(SimpleActionList *list,
+ int action, const char *val);
static void EstablishVariableSpace(void);
#define NOPAGER 0
@@ -159,6 +172,9 @@ main(int argc, char *argv[])
SetVariable(pset.vars, "PROMPT2", DEFAULT_PROMPT2);
SetVariable(pset.vars, "PROMPT3", DEFAULT_PROMPT3);
+ options.actions.head = NULL;
+ options.actions.tail = NULL;
+
parse_psql_options(argc, argv, &options);
/*
@@ -166,14 +182,11 @@ main(int argc, char *argv[])
* as if the user had specified "-f -". This lets single-transaction mode
* work in this case.
*/
- if (options.action == ACT_NOTHING && pset.notty)
- {
- options.action = ACT_FILE;
- options.action_string = NULL;
- }
+ if (options.actions.head == NULL && pset.notty)
+ simple_action_list_append(&options.actions, ACT_FILE, "-");
/* Bail out if -1 was specified but will be ignored. */
- if (options.single_txn && options.action != ACT_FILE && options.action == ACT_NOTHING)
+ if (options.single_txn && options.actions.head == NULL)
{
fprintf(stderr, _("%s: -1 can only be used in non-interactive mode\n"), pset.progname);
exit(EXIT_FAILURE);
@@ -217,8 +230,7 @@ main(int argc, char *argv[])
keywords[3] = "password";
values[3] = password;
keywords[4] = "dbname";
- values[4] = (options.action == ACT_LIST_DB &&
- options.dbname == NULL) ?
+ values[4] = (options.list_dbs && options.dbname == NULL) ?
"postgres" : options.dbname;
keywords[5] = "fallback_application_name";
values[5] = pset.progname;
@@ -259,7 +271,7 @@ main(int argc, char *argv[])
SyncVariables();
- if (options.action == ACT_LIST_DB)
+ if (options.list_dbs)
{
int success;
@@ -280,51 +292,90 @@ main(int argc, char *argv[])
}
/*
- * Now find something to do
- */
-
- /*
- * process file given by -f
+ * Now find something to do, process any actions given by caller,
+ * respecting their order of reception.
*/
- if (options.action == ACT_FILE)
+ if (options.actions.head != NULL)
{
- if (!options.no_psqlrc)
- process_psqlrc(argv[0]);
-
- successResult = process_file(options.action_string, options.single_txn, false);
- }
-
- /*
- * process slash command if one was given to -c
- */
- else if (options.action == ACT_SINGLE_SLASH)
- {
- PsqlScanState scan_state;
+ PGresult *res;
+ SimpleActionListCell *cell;
- if (pset.echo == PSQL_ECHO_ALL)
- puts(options.action_string);
+ successResult = EXIT_SUCCESS; /* be compiler quiete */
- scan_state = psql_scan_create();
- psql_scan_setup(scan_state,
- options.action_string,
- strlen(options.action_string));
+ if (!options.no_psqlrc)
+ process_psqlrc(argv[0]);
- successResult = HandleSlashCmds(scan_state, NULL) != PSQL_CMD_ERROR
- ? EXIT_SUCCESS : EXIT_FAILURE;
+ if (options.single_txn)
+ {
+ if ((res = PSQLexec("BEGIN")) == NULL)
+ {
+ if (pset.on_error_stop)
+ {
+ successResult = EXIT_USER;
+ goto error;
+ }
+ }
+ else
+ PQclear(res);
+ }
- psql_scan_destroy(scan_state);
- }
+ for (cell = options.actions.head; cell; cell = cell->next)
+ {
+ if (cell->action == ACT_SINGLE_QUERY)
+ {
+ if (pset.echo == PSQL_ECHO_ALL)
+ puts(cell->val);
+
+ successResult = SendQuery(cell->val)
+ ? EXIT_SUCCESS : EXIT_FAILURE;
+ }
+ else if (cell->action == ACT_SINGLE_SLASH)
+ {
+ PsqlScanState scan_state;
+
+ if (pset.echo == PSQL_ECHO_ALL)
+ puts(cell->val);
+
+ scan_state = psql_scan_create();
+ psql_scan_setup(scan_state,
+ cell->val,
+ strlen(cell->val));
+
+ successResult = HandleSlashCmds(scan_state, NULL) != PSQL_CMD_ERROR
+ ? EXIT_SUCCESS : EXIT_FAILURE;
+
+ psql_scan_destroy(scan_state);
+ }
+ else if (cell->action == ACT_FILE)
+ {
+ successResult = process_file(cell->val, false);
+ }
+ else
+ {
+ /* should never come here */
+ Assert(0);
+ }
+
+ if (successResult != EXIT_SUCCESS && pset.on_error_stop)
+ break;
+ }
- /*
- * If the query given to -c was a normal one, send it
- */
- else if (options.action == ACT_SINGLE_QUERY)
- {
- if (pset.echo == PSQL_ECHO_ALL)
- puts(options.action_string);
+ if (options.single_txn)
+ {
+ if ((res = PSQLexec("COMMIT")) == NULL)
+ {
+ if (pset.on_error_stop)
+ {
+ successResult = EXIT_USER;
+ goto error;
+ }
+ }
+ else
+ PQclear(res);
+ }
- successResult = SendQuery(options.action_string)
- ? EXIT_SUCCESS : EXIT_FAILURE;
+error:
+ ;
}
/*
@@ -419,14 +470,14 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options)
SetVariable(pset.vars, "ECHO", "errors");
break;
case 'c':
- options->action_string = pg_strdup(optarg);
if (optarg[0] == '\\')
- {
- options->action = ACT_SINGLE_SLASH;
- options->action_string++;
- }
+ simple_action_list_append(&options->actions,
+ ACT_SINGLE_SLASH,
+ pstrdup(optarg + 1));
else
- options->action = ACT_SINGLE_QUERY;
+ simple_action_list_append(&options->actions,
+ ACT_SINGLE_QUERY,
+ pstrdup(optarg));
break;
case 'd':
options->dbname = pg_strdup(optarg);
@@ -438,8 +489,9 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options)
SetVariableBool(pset.vars, "ECHO_HIDDEN");
break;
case 'f':
- options->action = ACT_FILE;
- options->action_string = pg_strdup(optarg);
+ simple_action_list_append(&options->actions,
+ ACT_FILE,
+ pg_strdup(optarg));
break;
case 'F':
pset.popt.topt.fieldSep.separator = pg_strdup(optarg);
@@ -452,7 +504,7 @@ parse_psql_options(int argc, char *argv[], struct adhoc_opts * options)
pset.popt.topt.format = PRINT_HTML;
break;
case 'l':
- options->action = ACT_LIST_DB;
+ options->list_dbs = true;
break;
case 'L':
options->logfilename = pg_strdup(optarg);
@@ -674,11 +726,11 @@ process_psqlrc_file(char *filename)
/* check for minor version first, then major, then no version */
if (access(psqlrc_minor, R_OK) == 0)
- (void) process_file(psqlrc_minor, false, false);
+ (void) process_file(psqlrc_minor, false);
else if (access(psqlrc_major, R_OK) == 0)
- (void) process_file(psqlrc_major, false, false);
+ (void) process_file(psqlrc_major, false);
else if (access(filename, R_OK) == 0)
- (void) process_file(filename, false, false);
+ (void) process_file(filename, false);
free(psqlrc_minor);
free(psqlrc_major);
@@ -892,6 +944,29 @@ show_context_hook(const char *newval)
}
+/*
+ * Support for list of actions. SimpleStringList cannot be used due possible
+ * combination different actions with the requirement to save the order.
+ */
+static void
+simple_action_list_append(SimpleActionList *list, int action, const char *val)
+{
+ SimpleActionListCell *cell;
+
+ cell = (SimpleActionListCell *)
+ pg_malloc(offsetof(SimpleActionListCell, val) + strlen(val) + 1);
+
+ cell->next = NULL;
+ cell->action = action;
+ strcpy(cell->val, val);
+
+ if (list->tail)
+ list->tail->next = cell;
+ else
+ list->head = cell;
+ list->tail = cell;
+}
+
static void
EstablishVariableSpace(void)
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
