On Wed, Dec 2, 2015 at 12:33 AM, Pavel Stehule <pavel.steh...@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
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index e4f72a8..47e9da2 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,11 +90,10 @@ 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
@@ -102,25 +102,34 @@ PostgreSQL documentation
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:
+ 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:
+ Putting more than one command in the <option>-c</option> string often
+ has unexpected results. This is a result of the fact that the whole
+ string is sent to the server as a single query.
+ 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
@@ -185,9 +194,11 @@ EOF
<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>.
+ 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 +550,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 +3737,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 +3831,12 @@ PSQL_EDITOR_LINENUMBER_ARG='--line '
</para>
</listitem>
+ <listitem>
+ <para>
+ Before <productname>PostgreSQL</productname> 9.6, <option>-c</option>
+ implied <option>-X</option>; this is no longer the case.
+ </para>
+ </listitem>
</itemizedlist>
</refsect1>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 6e8c623..cf6876b 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);
}
}
@@ -2288,13 +2288,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)
{
@@ -2339,37 +2338,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 4e5021a..dbfd20f 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;
@@ -279,52 +291,91 @@ main(int argc, char *argv[])
pset.progname, options.logfilename, strerror(errno));
}
- /*
- * Now find something to do
- */
+ if (!options.no_psqlrc)
+ process_psqlrc(argv[0]);
/*
- * process file given by -f
+ * If any actions were given by caller, process them in the order in
+ * which they were specified.
*/
- if (options.action == ACT_FILE)
+ if (options.actions.head != NULL)
{
- if (!options.no_psqlrc)
- process_psqlrc(argv[0]);
+ PGresult *res;
+ SimpleActionListCell *cell;
- 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;
-
- if (pset.echo == PSQL_ECHO_ALL)
- puts(options.action_string);
+ successResult = EXIT_SUCCESS; /* silence compiler */
- scan_state = psql_scan_create();
- psql_scan_setup(scan_state,
- options.action_string,
- strlen(options.action_string));
-
- 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:
+ ;
}
/*
@@ -332,9 +383,6 @@ main(int argc, char *argv[])
*/
else
{
- if (!options.no_psqlrc)
- process_psqlrc(argv[0]);
-
connection_warnings(true);
if (!pset.quiet)
printf(_("Type \"help\" for help.\n\n"));
@@ -419,14 +467,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 +486,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 +501,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);
@@ -675,11 +724,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);
@@ -893,6 +942,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
