proposal: psql \setfileref
Hi
I propose a new type of psql variables - file references. The content of
file reference is specified by referenced file. It allows simple inserting
large data without necessity of manual escaping or using LO api.
When I wrote the patch, I used parametrized queries for these data instead
escaped strings - the code is not much bigger, and the error messages are
much more friendly if query is not bloated by bigger content. The text mode
is used only - when escaping is not required, then content is implicitly
transformed to bytea. By default the content of file is bytea. When use
requires escaping, then he enforces text escaping - because it has sense
only for text type.
postgres=# \setfileref a ~/test2.xml
postgres=# \setfileref b ~/avatar.gif
postgres=# insert into test values(convert_from(:a, 'latin2')::xml, :b); --
xml is passed as bytea
postgres=# insert into test values(:'a', :b); -- xml is passed via unknown
text value
The content of file reference variables is not persistent in memory.
Comments, notes?
Regards
Pavel
Attachments:
psql-setfileref-initial.patchtext/x-patch; charset=US-ASCII; name=psql-setfileref-initial.patchDownload
commit 077c71b1f8ae24ccf2f3723e1e4ca5bf05bca0d3
Author: Pavel Stehule <pavel.stehule@gooddata.com>
Date: Wed Aug 31 17:15:33 2016 +0200
initial
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 4aaf657..3150510 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1355,6 +1355,32 @@ exec_command(const char *cmd,
free(envval);
}
+ /* \setfileref - set variable by reference on file */
+ else if (strcmp(cmd, "setfileref") == 0)
+ {
+ char *name = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+ char *ref = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+ success = false;
+
+ if (!name || !ref)
+ {
+ psql_error("\\%s: missing required argument\n", cmd);
+ success = false;
+ }
+ else
+ {
+ if (!SetFileRef(pset.vars, name, ref))
+ {
+ psql_error("\\%s: error while setting variable\n", cmd);
+ success = false;
+ }
+ }
+ }
+
/* \sf -- show a function's source code */
else if (strcmp(cmd, "sf") == 0 || strcmp(cmd, "sf+") == 0)
{
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 7399950..3c1db17 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -32,7 +32,6 @@ static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
static bool command_no_begin(const char *query);
static bool is_select_command(const char *query);
-
/*
* openQueryOutputFile --- attempt to open a query output file
*
@@ -108,6 +107,123 @@ setQFout(const char *fname)
return true;
}
+void
+psql_reset_query_params(void)
+{
+ int i;
+
+ for (i = 0; i < pset.nparams; i++)
+ if (pset.params[i] != NULL)
+ {
+ PQfreemem(pset.params[i]);
+ pset.params[i] = NULL;
+ }
+
+ pset.nparams = 0;
+}
+
+/*
+ * Load a content of the file_ref related file to query params buffer.
+ * When escaping is requested, then the text content is expected.
+ * Without escaping the bytea content is expected and related bytea
+ * escaping is processed.
+ */
+#define BYTEAOID 17
+#define UNKNOWNOID 0
+
+static char *
+get_file_ref_content(const char *value, bool escape, bool as_ident)
+{
+ PQExpBufferData buffer;
+ FILE *fd = NULL;
+ char *fname;
+ char *escaped_value;
+ char line[1024];
+ size_t size;
+
+ fname = pstrdup(value);
+
+ expand_tilde(&fname);
+ if (!fname)
+ {
+ psql_error("missing valid path to file\n");
+ return NULL;
+ }
+
+ canonicalize_path(fname);
+
+ fd = fopen(fname, PG_BINARY_R);
+ if (!fd)
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ /* can append another parameter */
+ if (pset.nparams >= MAX_BINARY_PARAMS)
+ {
+ psql_error("too much binary parameters");
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ if (!pset.db)
+ {
+ psql_error("cannot escape without active connection\n");
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ initPQExpBuffer(&buffer);
+
+ while ((size = fread(line, 1, sizeof(line), fd)) > 0)
+ appendBinaryPQExpBuffer(&buffer, line, size);
+
+ if (ferror(fd))
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+ return NULL;
+ }
+
+ if (escape)
+ {
+ if (as_ident)
+ escaped_value =
+ PQescapeIdentifier(pset.db, buffer.data, buffer.len);
+ else
+ escaped_value =
+ PQescapeLiteral(pset.db, buffer.data, buffer.len);
+ pset.paramTypes[pset.nparams] = UNKNOWNOID;
+ }
+ else
+ {
+ escaped_value = (char *)
+ PQescapeByteaConn(pset.db,
+ (const unsigned char *) buffer.data, buffer.len, &size);
+ pset.paramTypes[pset.nparams] = BYTEAOID;
+ }
+
+ /* fname, buffer is not necessary longer */
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+
+ if (escaped_value == NULL)
+ {
+ const char *error = PQerrorMessage(pset.db);
+
+ psql_error("%s", error);
+ return NULL;
+ }
+
+ pset.params[pset.nparams] = escaped_value;
+
+ snprintf(line, sizeof(line) - 1, "$%d", ++pset.nparams);
+
+ return pstrdup(line);
+}
/*
* Variable-fetching callback for flex lexer
@@ -124,11 +240,15 @@ psql_get_variable(const char *varname, bool escape, bool as_ident)
{
char *result;
const char *value;
+ bool is_file_ref;
- value = GetVariable(pset.vars, varname);
+ value = (char *) GetVariableOrFileRef(pset.vars, varname, &is_file_ref);
if (!value)
return NULL;
+ if (is_file_ref)
+ return get_file_ref_content(value, escape, as_ident);
+
if (escape)
{
char *escaped_value;
@@ -1235,7 +1355,16 @@ SendQuery(const char *query)
if (pset.timing)
INSTR_TIME_SET_CURRENT(before);
- results = PQexec(pset.db, query);
+ if (pset.nparams > 0)
+ results = PQexecParams(pset.db, query,
+ pset.nparams,
+ pset.paramTypes,
+ (const char * const *) pset.params,
+ NULL,
+ NULL,
+ 0);
+ else
+ results = PQexec(pset.db, query);
/* these operations are included in the timing result: */
ResetCancelConn();
@@ -1380,7 +1509,6 @@ sendquery_cleanup:
return OK;
}
-
/*
* ExecQueryUsingCursor: run a SELECT-like query using a cursor
*
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index bdcb58f..4f46b9c 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -38,6 +38,8 @@ extern int PSQLexecWatch(const char *query, const printQueryOpt *opt);
extern bool SendQuery(const char *query);
+void psql_reset_query_params(void);
+
extern bool is_superuser(void);
extern bool standard_strings(void);
extern const char *session_username(void);
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 37dfa4d..23fd6d3 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -23,7 +23,6 @@ const PsqlScanCallbacks psqlscan_callbacks = {
psql_error
};
-
/*
* Main processing loop for reading lines of input
* and sending them to the backend.
@@ -403,6 +402,9 @@ MainLoop(FILE *source)
psql_scan_finish(scan_state);
free(line);
+ /* reset binary parameters */
+ psql_reset_query_params();
+
if (slashCmdStatus == PSQL_CMD_TERMINATE)
{
successResult = EXIT_SUCCESS;
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 8cfe9d2..2874704 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -77,6 +77,8 @@ enum trivalue
TRI_YES
};
+#define MAX_BINARY_PARAMS 32
+
typedef struct _psqlSettings
{
PGconn *db; /* connection to backend */
@@ -135,6 +137,9 @@ typedef struct _psqlSettings
const char *prompt3;
PGVerbosity verbosity; /* current error verbosity level */
PGContextVisibility show_context; /* current context display level */
+ int nparams;
+ Oid paramTypes[MAX_BINARY_PARAMS];
+ char *params[MAX_BINARY_PARAMS];
} PsqlSettings;
extern PsqlSettings pset;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 111593c..1ed40aa 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -106,6 +106,7 @@ main(int argc, char *argv[])
char *password = NULL;
char *password_prompt = NULL;
bool new_pass;
+ int i;
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("psql"));
@@ -139,6 +140,10 @@ main(int argc, char *argv[])
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
+ pset.nparams = 0;
+ for (i = 0; i < MAX_BINARY_PARAMS; i++)
+ pset.params[i] = NULL;
+
/* We rely on unmentioned fields of pset.popt to start out 0/false/NULL */
pset.popt.topt.format = PRINT_ALIGNED;
pset.popt.topt.border = 1;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 1345e4e..97460ee 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1292,7 +1292,7 @@ psql_completion(const char *text, int start, int end)
"\\f", "\\g", "\\gexec", "\\gset", "\\h", "\\help", "\\H", "\\i", "\\ir", "\\l",
"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\\qecho", "\\r",
- "\\s", "\\set", "\\setenv", "\\sf", "\\sv", "\\t", "\\T",
+ "\\s", "\\set", "\\setenv", "\\setfileref", "\\sf", "\\sv", "\\t", "\\T",
"\\timing", "\\unset", "\\x", "\\w", "\\watch", "\\z", "\\!", NULL
};
@@ -3107,6 +3107,12 @@ psql_completion(const char *text, int start, int end)
matches = completion_matches(text, complete_from_files);
}
+ else if (Matches2("\\setfileref", MatchAny))
+ {
+ completion_charp = "\\";
+ matches = completion_matches(text, complete_from_files);
+ }
+
/*
* Finally, we look through the list of "things", such as TABLE, INDEX and
* check if that was the previous word. If so, execute the query to get a
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index f43f418..1c12719 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -71,6 +71,31 @@ GetVariable(VariableSpace space, const char *name)
if (strcmp(current->name, name) == 0)
{
/* this is correct answer when value is NULL, too */
+
+ return current->value;
+ }
+ }
+
+ return NULL;
+}
+
+const char *
+GetVariableOrFileRef(VariableSpace space, const char *name, bool *is_file_ref)
+{
+ struct _variable *current;
+
+ if (!space)
+ return NULL;
+
+ for (current = space->next; current; current = current->next)
+ {
+ if (strcmp(current->name, name) == 0)
+ {
+ /* this is correct answer when value is NULL, too */
+
+ if (is_file_ref)
+ *is_file_ref = current->is_file_ref;
+
return current->value;
}
}
@@ -178,7 +203,7 @@ PrintVariables(VariableSpace space)
for (ptr = space->next; ptr; ptr = ptr->next)
{
if (ptr->value)
- printf("%s = '%s'\n", ptr->name, ptr->value);
+ printf("%s = %s'%s'\n", ptr->name, ptr->is_file_ref ? "^" : "", ptr->value);
if (cancel_pressed)
break;
}
@@ -218,6 +243,47 @@ SetVariable(VariableSpace space, const char *name, const char *value)
/* not present, make new entry */
current = pg_malloc(sizeof *current);
current->name = pg_strdup(name);
+ current->is_file_ref = false;
+ current->value = pg_strdup(value);
+ current->assign_hook = NULL;
+ current->next = NULL;
+ previous->next = current;
+ return true;
+}
+
+bool
+SetFileRef(VariableSpace space, const char *name, const char *value)
+{
+ struct _variable *current,
+ *previous;
+
+ if (!space)
+ return false;
+
+ if (!valid_variable_name(name))
+ return false;
+
+ if (!value)
+ return DeleteVariable(space, name);
+
+ for (previous = space, current = space->next;
+ current;
+ previous = current, current = current->next)
+ {
+ if (strcmp(current->name, name) == 0)
+ {
+ /* found entry, so update */
+ if (current->value)
+ free(current->value);
+ current->value = pg_strdup(value);
+ return true;
+ }
+ }
+
+ /* not present, make new entry */
+ current = pg_malloc(sizeof *current);
+ current->is_file_ref = true;
+ current->name = pg_strdup(name);
current->value = pg_strdup(value);
current->assign_hook = NULL;
current->next = NULL;
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index d7a05a1..8b24441 100644
--- a/src/bin/psql/variables.h
+++ b/src/bin/psql/variables.h
@@ -26,6 +26,7 @@ struct _variable
{
char *name;
char *value;
+ bool is_file_ref;
VariableAssignHook assign_hook;
struct _variable *next;
};
@@ -34,6 +35,7 @@ typedef struct _variable *VariableSpace;
VariableSpace CreateVariableSpace(void);
const char *GetVariable(VariableSpace space, const char *name);
+const char *GetVariableOrFileRef(VariableSpace space, const char *name, bool *is_file_ref);
bool ParseVariableBool(const char *value, const char *name);
int ParseVariableNum(const char *val,
@@ -49,6 +51,7 @@ int GetVariableNum(VariableSpace space,
void PrintVariables(VariableSpace space);
bool SetVariable(VariableSpace space, const char *name, const char *value);
+bool SetFileRef(VariableSpace space, const char *name, const char *value);
bool SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook);
bool SetVariableBool(VariableSpace space, const char *name);
bool DeleteVariable(VariableSpace space, const char *name);
On Wed, Aug 31, 2016 at 11:32 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
Hi
I propose a new type of psql variables - file references. The content of
file reference is specified by referenced file. It allows simple inserting
large data without necessity of manual escaping or using LO api.When I wrote the patch, I used parametrized queries for these data instead
escaped strings - the code is not much bigger, and the error messages are
much more friendly if query is not bloated by bigger content. The text mode
is used only - when escaping is not required, then content is implicitly
transformed to bytea. By default the content of file is bytea. When use
requires escaping, then he enforces text escaping - because it has sense
only for text type.postgres=# \setfileref a ~/test2.xml
postgres=# \setfileref b ~/avatar.gif
postgres=# insert into test values(convert_from(:a, 'latin2')::xml, :b);
-- xml is passed as bytea
postgres=# insert into test values(:'a', :b); -- xml is passed via unknown
text valueThe content of file reference variables is not persistent in memory.
Comments, notes?
Regards
Pavel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Clearly jumping ahead on this one, but if the fileref is essentially a pipe
to "cat /path/to/file.name", is there anything stopping us from setting
pipes?
My interest is primarily in ways that COPY could use this.
2016-08-31 18:24 GMT+02:00 Corey Huinker <corey.huinker@gmail.com>:
On Wed, Aug 31, 2016 at 11:32 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:Hi
I propose a new type of psql variables - file references. The content of
file reference is specified by referenced file. It allows simple inserting
large data without necessity of manual escaping or using LO api.When I wrote the patch, I used parametrized queries for these data
instead escaped strings - the code is not much bigger, and the error
messages are much more friendly if query is not bloated by bigger content.
The text mode is used only - when escaping is not required, then content is
implicitly transformed to bytea. By default the content of file is bytea.
When use requires escaping, then he enforces text escaping - because it has
sense only for text type.postgres=# \setfileref a ~/test2.xml
postgres=# \setfileref b ~/avatar.gif
postgres=# insert into test values(convert_from(:a, 'latin2')::xml, :b);
-- xml is passed as bytea
postgres=# insert into test values(:'a', :b); -- xml is passed via
unknown text valueThe content of file reference variables is not persistent in memory.
Comments, notes?
Regards
Pavel
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackersClearly jumping ahead on this one, but if the fileref is essentially a
pipe to "cat /path/to/file.name", is there anything stopping us from
setting pipes?
My interest is primarily in ways that COPY could use this.
I don't see a reason why it should not be possible - the current code can't
do it, but with 20 lines more, it should be possible
There is one disadvantage against copy - the content should be fully loaded
to memory, but any other functionality should be same.
Regards
Pavel
Hi Pavel,
I just tried to apply your patch psql-setfileref-initial.patch (using git apply) to the newest revision of postgres at the time (da6c4f6ca88) and it failed to patch startup.c. Thinking that the patch was for some previous revision, I then checked out d062245b5, which was from 2016-08-31, and tried applying the patch from there. I had the same problem:
$ git apply psql-setfileref-initial.patch
error: patch failed: src/bin/psql/startup.c:106
error: src/bin/psql/startup.c: patch does not apply
However, when I applied the changes to startup.c manually and removed them from the patch, the rest of the patch applied without a problem. I don't know if I may have done something wrong in trying to apply the patch, but you may want to double check if you need to regenerate your patch from the latest revision so it will apply smoothly for reviewers.
In the meantime, I haven't had a chance to try out the fileref feature yet but I'll give it a go when I get a chance!
Thanks!
Ryan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-09-26 14:57 GMT+02:00 Ryan Murphy <ryanfmurphy@gmail.com>:
Hi Pavel,
I just tried to apply your patch psql-setfileref-initial.patch (using git
apply) to the newest revision of postgres at the time (da6c4f6ca88) and it
failed to patch startup.c. Thinking that the patch was for some previous
revision, I then checked out d062245b5, which was from 2016-08-31, and
tried applying the patch from there. I had the same problem:$ git apply psql-setfileref-initial.patch
error: patch failed: src/bin/psql/startup.c:106
error: src/bin/psql/startup.c: patch does not applyHowever, when I applied the changes to startup.c manually and removed them
from the patch, the rest of the patch applied without a problem. I don't
know if I may have done something wrong in trying to apply the patch, but
you may want to double check if you need to regenerate your patch from the
latest revision so it will apply smoothly for reviewers.
Thank you for info. I'll do it immediately.
Regards
Pavel
Show quoted text
In the meantime, I haven't had a chance to try out the fileref feature yet
but I'll give it a go when I get a chance!Thanks!
Ryan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi
2016-09-26 14:57 GMT+02:00 Ryan Murphy <ryanfmurphy@gmail.com>:
Hi Pavel,
I just tried to apply your patch psql-setfileref-initial.patch (using git
apply) to the newest revision of postgres at the time (da6c4f6ca88) and it
failed to patch startup.c. Thinking that the patch was for some previous
revision, I then checked out d062245b5, which was from 2016-08-31, and
tried applying the patch from there. I had the same problem:$ git apply psql-setfileref-initial.patch
error: patch failed: src/bin/psql/startup.c:106
error: src/bin/psql/startup.c: patch does not applyHowever, when I applied the changes to startup.c manually and removed them
from the patch, the rest of the patch applied without a problem. I don't
know if I may have done something wrong in trying to apply the patch, but
you may want to double check if you need to regenerate your patch from the
latest revision so it will apply smoothly for reviewers.
please, can you check attached patch? It worked in my laptop.
Regards
Pavel
Show quoted text
In the meantime, I haven't had a chance to try out the fileref feature yet
but I'll give it a go when I get a chance!Thanks!
Ryan
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
psql-setfileref-2016-09-26.patchtext/x-patch; charset=US-ASCII; name=psql-setfileref-2016-09-26.patchDownload
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a9a2fdb..af38ff9 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1363,6 +1363,32 @@ exec_command(const char *cmd,
free(envval);
}
+ /* \setfileref - set variable by reference on file */
+ else if (strcmp(cmd, "setfileref") == 0)
+ {
+ char *name = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+ char *ref = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+ success = false;
+
+ if (!name || !ref)
+ {
+ psql_error("\\%s: missing required argument\n", cmd);
+ success = false;
+ }
+ else
+ {
+ if (!SetFileRef(pset.vars, name, ref))
+ {
+ psql_error("\\%s: error while setting variable\n", cmd);
+ success = false;
+ }
+ }
+ }
+
/* \sf -- show a function's source code */
else if (strcmp(cmd, "sf") == 0 || strcmp(cmd, "sf+") == 0)
{
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a7789df..b160228 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -25,6 +25,7 @@
#include "settings.h"
#include "command.h"
#include "copy.h"
+#include "catalog/pg_type.h"
#include "crosstabview.h"
#include "fe_utils/mbprint.h"
@@ -33,7 +34,6 @@ static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
static bool command_no_begin(const char *query);
static bool is_select_command(const char *query);
-
/*
* openQueryOutputFile --- attempt to open a query output file
*
@@ -109,6 +109,120 @@ setQFout(const char *fname)
return true;
}
+void
+psql_reset_query_params(void)
+{
+ int i;
+
+ for (i = 0; i < pset.nparams; i++)
+ if (pset.params[i] != NULL)
+ {
+ PQfreemem(pset.params[i]);
+ pset.params[i] = NULL;
+ }
+
+ pset.nparams = 0;
+}
+
+/*
+ * Load a content of the file_ref related file to query params buffer.
+ * When escaping is requested, then the text content is expected.
+ * Without escaping the bytea content is expected and related bytea
+ * escaping is processed.
+ */
+static char *
+get_file_ref_content(const char *value, bool escape, bool as_ident)
+{
+ PQExpBufferData buffer;
+ FILE *fd = NULL;
+ char *fname;
+ char *escaped_value;
+ char line[1024];
+ size_t size;
+
+ fname = pstrdup(value);
+
+ expand_tilde(&fname);
+ if (!fname)
+ {
+ psql_error("missing valid path to file\n");
+ return NULL;
+ }
+
+ canonicalize_path(fname);
+
+ fd = fopen(fname, PG_BINARY_R);
+ if (!fd)
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ /* can append another parameter */
+ if (pset.nparams >= MAX_BINARY_PARAMS)
+ {
+ psql_error("too much binary parameters");
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ if (!pset.db)
+ {
+ psql_error("cannot escape without active connection\n");
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ initPQExpBuffer(&buffer);
+
+ while ((size = fread(line, 1, sizeof(line), fd)) > 0)
+ appendBinaryPQExpBuffer(&buffer, line, size);
+
+ if (ferror(fd))
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+ return NULL;
+ }
+
+ if (escape)
+ {
+ if (as_ident)
+ escaped_value =
+ PQescapeIdentifier(pset.db, buffer.data, buffer.len);
+ else
+ escaped_value =
+ PQescapeLiteral(pset.db, buffer.data, buffer.len);
+ pset.paramTypes[pset.nparams] = UNKNOWNOID;
+ }
+ else
+ {
+ escaped_value = (char *)
+ PQescapeByteaConn(pset.db,
+ (const unsigned char *) buffer.data, buffer.len, &size);
+ pset.paramTypes[pset.nparams] = BYTEAOID;
+ }
+
+ /* fname, buffer is not necessary longer */
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+
+ if (escaped_value == NULL)
+ {
+ const char *error = PQerrorMessage(pset.db);
+
+ psql_error("%s", error);
+ return NULL;
+ }
+
+ pset.params[pset.nparams] = escaped_value;
+
+ snprintf(line, sizeof(line) - 1, "$%d", ++pset.nparams);
+
+ return pstrdup(line);
+}
/*
* Variable-fetching callback for flex lexer
@@ -125,11 +239,15 @@ psql_get_variable(const char *varname, bool escape, bool as_ident)
{
char *result;
const char *value;
+ bool is_file_ref;
- value = GetVariable(pset.vars, varname);
+ value = (char *) GetVariableOrFileRef(pset.vars, varname, &is_file_ref);
if (!value)
return NULL;
+ if (is_file_ref)
+ return get_file_ref_content(value, escape, as_ident);
+
if (escape)
{
char *escaped_value;
@@ -1287,7 +1405,16 @@ SendQuery(const char *query)
if (pset.timing)
INSTR_TIME_SET_CURRENT(before);
- results = PQexec(pset.db, query);
+ if (pset.nparams > 0)
+ results = PQexecParams(pset.db, query,
+ pset.nparams,
+ pset.paramTypes,
+ (const char * const *) pset.params,
+ NULL,
+ NULL,
+ 0);
+ else
+ results = PQexec(pset.db, query);
/* these operations are included in the timing result: */
ResetCancelConn();
@@ -1432,7 +1559,6 @@ sendquery_cleanup:
return OK;
}
-
/*
* ExecQueryUsingCursor: run a SELECT-like query using a cursor
*
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index bdcb58f..4f46b9c 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -38,6 +38,8 @@ extern int PSQLexecWatch(const char *query, const printQueryOpt *opt);
extern bool SendQuery(const char *query);
+void psql_reset_query_params(void);
+
extern bool is_superuser(void);
extern bool standard_strings(void);
extern const char *session_username(void);
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 37dfa4d..23fd6d3 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -23,7 +23,6 @@ const PsqlScanCallbacks psqlscan_callbacks = {
psql_error
};
-
/*
* Main processing loop for reading lines of input
* and sending them to the backend.
@@ -403,6 +402,9 @@ MainLoop(FILE *source)
psql_scan_finish(scan_state);
free(line);
+ /* reset binary parameters */
+ psql_reset_query_params();
+
if (slashCmdStatus == PSQL_CMD_TERMINATE)
{
successResult = EXIT_SUCCESS;
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 8cfe9d2..2874704 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -77,6 +77,8 @@ enum trivalue
TRI_YES
};
+#define MAX_BINARY_PARAMS 32
+
typedef struct _psqlSettings
{
PGconn *db; /* connection to backend */
@@ -135,6 +137,9 @@ typedef struct _psqlSettings
const char *prompt3;
PGVerbosity verbosity; /* current error verbosity level */
PGContextVisibility show_context; /* current context display level */
+ int nparams;
+ Oid paramTypes[MAX_BINARY_PARAMS];
+ char *params[MAX_BINARY_PARAMS];
} PsqlSettings;
extern PsqlSettings pset;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 7ce05fb..05715a6 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -107,6 +107,7 @@ main(int argc, char *argv[])
char password[100];
char *password_prompt = NULL;
bool new_pass;
+ int i;
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("psql"));
@@ -140,6 +141,10 @@ main(int argc, char *argv[])
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
+ pset.nparams = 0;
+ for (i = 0; i < MAX_BINARY_PARAMS; i++)
+ pset.params[i] = NULL;
+
/* We rely on unmentioned fields of pset.popt to start out 0/false/NULL */
pset.popt.topt.format = PRINT_ALIGNED;
pset.popt.topt.border = 1;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 50a45eb..ff4bf73 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1294,7 +1294,7 @@ psql_completion(const char *text, int start, int end)
"\\f", "\\g", "\\gexec", "\\gset", "\\h", "\\help", "\\H", "\\i", "\\ir", "\\l",
"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\\qecho", "\\r",
- "\\s", "\\set", "\\setenv", "\\sf", "\\sv", "\\t", "\\T",
+ "\\s", "\\set", "\\setenv", "\\setfileref", "\\sf", "\\sv", "\\t", "\\T",
"\\timing", "\\unset", "\\x", "\\w", "\\watch", "\\z", "\\!", NULL
};
@@ -3128,6 +3128,12 @@ psql_completion(const char *text, int start, int end)
matches = completion_matches(text, complete_from_files);
}
+ else if (Matches2("\\setfileref", MatchAny))
+ {
+ completion_charp = "\\";
+ matches = completion_matches(text, complete_from_files);
+ }
+
/*
* Finally, we look through the list of "things", such as TABLE, INDEX and
* check if that was the previous word. If so, execute the query to get a
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index f43f418..1c12719 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -71,6 +71,31 @@ GetVariable(VariableSpace space, const char *name)
if (strcmp(current->name, name) == 0)
{
/* this is correct answer when value is NULL, too */
+
+ return current->value;
+ }
+ }
+
+ return NULL;
+}
+
+const char *
+GetVariableOrFileRef(VariableSpace space, const char *name, bool *is_file_ref)
+{
+ struct _variable *current;
+
+ if (!space)
+ return NULL;
+
+ for (current = space->next; current; current = current->next)
+ {
+ if (strcmp(current->name, name) == 0)
+ {
+ /* this is correct answer when value is NULL, too */
+
+ if (is_file_ref)
+ *is_file_ref = current->is_file_ref;
+
return current->value;
}
}
@@ -178,7 +203,7 @@ PrintVariables(VariableSpace space)
for (ptr = space->next; ptr; ptr = ptr->next)
{
if (ptr->value)
- printf("%s = '%s'\n", ptr->name, ptr->value);
+ printf("%s = %s'%s'\n", ptr->name, ptr->is_file_ref ? "^" : "", ptr->value);
if (cancel_pressed)
break;
}
@@ -218,6 +243,47 @@ SetVariable(VariableSpace space, const char *name, const char *value)
/* not present, make new entry */
current = pg_malloc(sizeof *current);
current->name = pg_strdup(name);
+ current->is_file_ref = false;
+ current->value = pg_strdup(value);
+ current->assign_hook = NULL;
+ current->next = NULL;
+ previous->next = current;
+ return true;
+}
+
+bool
+SetFileRef(VariableSpace space, const char *name, const char *value)
+{
+ struct _variable *current,
+ *previous;
+
+ if (!space)
+ return false;
+
+ if (!valid_variable_name(name))
+ return false;
+
+ if (!value)
+ return DeleteVariable(space, name);
+
+ for (previous = space, current = space->next;
+ current;
+ previous = current, current = current->next)
+ {
+ if (strcmp(current->name, name) == 0)
+ {
+ /* found entry, so update */
+ if (current->value)
+ free(current->value);
+ current->value = pg_strdup(value);
+ return true;
+ }
+ }
+
+ /* not present, make new entry */
+ current = pg_malloc(sizeof *current);
+ current->is_file_ref = true;
+ current->name = pg_strdup(name);
current->value = pg_strdup(value);
current->assign_hook = NULL;
current->next = NULL;
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index d7a05a1..8b24441 100644
--- a/src/bin/psql/variables.h
+++ b/src/bin/psql/variables.h
@@ -26,6 +26,7 @@ struct _variable
{
char *name;
char *value;
+ bool is_file_ref;
VariableAssignHook assign_hook;
struct _variable *next;
};
@@ -34,6 +35,7 @@ typedef struct _variable *VariableSpace;
VariableSpace CreateVariableSpace(void);
const char *GetVariable(VariableSpace space, const char *name);
+const char *GetVariableOrFileRef(VariableSpace space, const char *name, bool *is_file_ref);
bool ParseVariableBool(const char *value, const char *name);
int ParseVariableNum(const char *val,
@@ -49,6 +51,7 @@ int GetVariableNum(VariableSpace space,
void PrintVariables(VariableSpace space);
bool SetVariable(VariableSpace space, const char *name, const char *value);
+bool SetFileRef(VariableSpace space, const char *name, const char *value);
bool SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook);
bool SetVariableBool(VariableSpace space, const char *name);
bool DeleteVariable(VariableSpace space, const char *name);
please, can you check attached patch? It worked in my laptop.
Regards
Pavel
Yes, that one applied for me without any problems.
Thanks,
Ryan
2016-09-26 21:47 GMT+02:00 Ryan Murphy <ryanfmurphy@gmail.com>:
please, can you check attached patch? It worked in my laptop.
Regards
Pavel
Yes, that one applied for me without any problems.
Great,
Thank you
Pavel
Show quoted text
Thanks,
Ryan
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, failed
Spec compliant: tested, failed
Documentation: not tested
Contents & Purpose
==================
This patch adds a new type of psql variables: file references, that can be
set using command \setfileref. The value of the named variable is the path
to the referenced file. It allows simple inserting of large data without
necessity of manual escaping or using LO api. Use:
postgres=# create table test (col1 bytea);
postgres=# \setfileref a ~/avatar.gif
postgres=# insert into test values(:a);
Content of file is returned as bytea, the text output can be used when
escaping is not required, in this case use convert_from() to obtain a
text output.
postgres=# create table test (col1 text);
postgres=# \setfileref a ~/README.txt
postgres=# insert into test values(convert_from(:a, 'utf8'));
The content of file reference variables is not persistent in memory.
List of file referenced variable can be listed using \set
postgres=# \set
...
b = ^'~/README.txt'
Empty file outputs an empty bytea (\x).
Initial Run
===========
The patch applies cleanly to HEAD and doesn't break anything, at least the
regression tests all pass successfully.
Feedback on testing
===============
I see several problems.
1) Error reading referenced file returns the system error and a syntax error
on variable:
postgres=# \setfileref a /etc/sudoers
postgres=# insert into test values (:a);
/etc/sudoers: Permission denied
ERROR: syntax error at or near ":"
LINE 1: insert into t1 values (:a);
2) Trying to load a file with size upper than the 1GB limit reports the following
error (size 2254110720 bytes):
postgres=# \setfileref b /home/VMs/ISOs/sol-10-u11-ga-x86-dvd.iso
postgres=# insert into test values (:b);
INSERT 0 1
postgres=# ANALYZE test;
postgres=# SELECT relname, relkind, relpages, pg_size_pretty(relpages::bigint*8192) FROM pg_class WHERE relname='test';
relname | relkind | relpages | pg_size_pretty
---------+---------+----------+----------------
test | r | 1 | 8192 bytes
(1 row)
postgres=# select * from test;
col1
------
\x
(1 row)
This should not insert an empty bytea but might raise an error instead.
Trying to load smaller file but with bytea escaping total size upper than
the 1GB limit (size 666894336 bytes) reports:
postgres=# \setfileref a /var/ISOs/CentOS-7-x86_64-Minimal-1503-01.iso
postgres=# insert into t1 values (1, :a, 'tr');
ERROR: out of memory
DETAIL: Cannot enlarge string buffer containing 0 bytes by 1333788688 more bytes.
Time: 1428.657 ms (00:01.429)
This raise an error as bytea escaping increase content size which is the behavior expected.
3) The path doesn't not allow the use of pipe to system command, which is a secure
behavior, but it is quite easy to perform a DOS by using special files like:
postgres=# \setfileref b /dev/random
postgres=# insert into t1 (:b);.
this will never end until we kill the psql session. I think we must also prevent
non regular files to be referenced using S_ISREG.
I think all these three errors can be caught and prevented at variable declaration using some tests on files like:
is readable?
is a regular file?
is small size enough?
to report an error directly at variable file reference setting.
4) An other problem is that like this this patch will allow anyone to upload into a
column the content of any system file that can be read by postgres system user
and then allow non system user to read its content. For example, connected as
a basic PostgreSQL only user:
testdb=> select current_user;
current_user
--------------
user1
(1 row)
testdb=> \setfileref b /etc/passwd
testdb=> insert into test values (:b);
INSERT 0 1
then to read the file:
testdb=> select convert_from(col1, 'utf8') from t1;
Maybe the referenced files must be limited to some part of the filesystem
or the feature limited to superuser only.
5) There is no documentation at all on this feature. Here a proposal
for inclusion in doc/src/sgml/ref/psql-ref.sgml
\setfileref name value
Sets the internal variable name as a reference to the file path
set as value. To unset a variable, use the \unset command.
File references are shown as file path prefixed with the ^ character
when using the \set command alone.
Valid variable names can contain characters, digits, and underscores.
See the section Variables below for details. Variable names are
case-sensitive.
More detail here about file size, file privilege, etc following
what will be decided.
6) I would also like to see \setfileref display all file references variables
defined instead of message "missing required argument". Just like \set and
\pset alone are working. \set can still show the file references variables, that's
not a problem for me.
The new status of this patch is: Waiting on Author
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Mon, Oct 3, 2016 at 3:54 PM, Gilles Darold <gilles@darold.net> wrote:
4) An other problem is that like this this patch will allow anyone to upload into a
column the content of any system file that can be read by postgres system user
and then allow non system user to read its content.
I thought this was a client-side feature, so that it would let a
client upload any file that the client can read, but not things that
can only be read by the postgres system user.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Le 03/10/2016 à 23:03, Robert Haas a écrit :
On Mon, Oct 3, 2016 at 3:54 PM, Gilles Darold <gilles@darold.net> wrote:
4) An other problem is that like this this patch will allow anyone to upload into a
column the content of any system file that can be read by postgres system user
and then allow non system user to read its content.I thought this was a client-side feature, so that it would let a
client upload any file that the client can read, but not things that
can only be read by the postgres system user.
Yes that's right, sorry for the noise, forget this fourth report.
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Le 03/10/2016 à 23:23, Gilles Darold a écrit :
Le 03/10/2016 à 23:03, Robert Haas a écrit :
On Mon, Oct 3, 2016 at 3:54 PM, Gilles Darold <gilles@darold.net> wrote:
4) An other problem is that like this this patch will allow anyone to upload into a
column the content of any system file that can be read by postgres system user
and then allow non system user to read its content.I thought this was a client-side feature, so that it would let a
client upload any file that the client can read, but not things that
can only be read by the postgres system user.Yes that's right, sorry for the noise, forget this fourth report.
After some more though there is still a security issue here. For a
PostgreSQL user who also have login acces to the server, it is possible
to read any file that the postgres system user can read, especially a
.pgpass or a recovery.conf containing password.
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-10-04 9:18 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com>:
Le 03/10/2016 à 23:23, Gilles Darold a écrit :
Le 03/10/2016 à 23:03, Robert Haas a écrit :
On Mon, Oct 3, 2016 at 3:54 PM, Gilles Darold <gilles@darold.net>
wrote:
4) An other problem is that like this this patch will allow anyone to
upload into a
column the content of any system file that can be read by postgres
system user
and then allow non system user to read its content.
I thought this was a client-side feature, so that it would let a
client upload any file that the client can read, but not things that
can only be read by the postgres system user.Yes that's right, sorry for the noise, forget this fourth report.
After some more though there is still a security issue here. For a
PostgreSQL user who also have login acces to the server, it is possible
to read any file that the postgres system user can read, especially a
.pgpass or a recovery.conf containing password.
This patch doesn't introduce any new server side functionality, so if there
is some vulnerability, then it is exists now too.
Regards
Pavel
Show quoted text
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Le 04/10/2016 à 17:29, Pavel Stehule a écrit :
2016-10-04 9:18 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com
<mailto:gilles.darold@dalibo.com>>:Le 03/10/2016 à 23:23, Gilles Darold a écrit :
Le 03/10/2016 à 23:03, Robert Haas a écrit :
On Mon, Oct 3, 2016 at 3:54 PM, Gilles Darold
<gilles@darold.net <mailto:gilles@darold.net>> wrote:
4) An other problem is that like this this patch will allow
anyone to upload into a
column the content of any system file that can be read by
postgres system user
and then allow non system user to read its content.
I thought this was a client-side feature, so that it would let a
client upload any file that the client can read, but not thingsthat
can only be read by the postgres system user.
Yes that's right, sorry for the noise, forget this fourth report.
After some more though there is still a security issue here. For a
PostgreSQL user who also have login acces to the server, it is
possible
to read any file that the postgres system user can read, especially a
.pgpass or a recovery.conf containing password.This patch doesn't introduce any new server side functionality, so if
there is some vulnerability, then it is exists now too.
It doesn't exists, that was my system user which have extended
privilege. You can definitively forget the fouth point.
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
hi
2016-10-04 9:18 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com>:
Le 03/10/2016 à 23:23, Gilles Darold a écrit :
Le 03/10/2016 à 23:03, Robert Haas a écrit :
On Mon, Oct 3, 2016 at 3:54 PM, Gilles Darold <gilles@darold.net>
wrote:
4) An other problem is that like this this patch will allow anyone to
upload into a
column the content of any system file that can be read by postgres
system user
and then allow non system user to read its content.
I thought this was a client-side feature, so that it would let a
client upload any file that the client can read, but not things that
can only be read by the postgres system user.Yes that's right, sorry for the noise, forget this fourth report.
After some more though there is still a security issue here. For a
PostgreSQL user who also have login acces to the server, it is possible
to read any file that the postgres system user can read, especially a
.pgpass or a recovery.conf containing password.
here is new update - some mentioned issues are fixed + regress tests and
docs
regards
Pavel
Show quoted text
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachments:
psql-setfileref-2016-10-09.patchtext/x-patch; charset=US-ASCII; name=psql-setfileref-2016-10-09.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 4806e77..9fb9cd9 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2714,6 +2714,24 @@ testdb=> <userinput>\setenv LESS -imx4F</userinput>
</varlistentry>
<varlistentry>
+ <term><literal>\setfileref <replaceable class="parameter">name</replaceable> [ <replaceable class="parameter">value</replaceable> ]</literal></term>
+
+ <listitem>
+ <para>
+ Sets the internal variable name as a reference to the file path
+ set as value. To unset a variable, use the <command>\unset</command> command.
+
+ File references are shown as file path prefixed with the ^ character
+ when using the <command>\set</command> command alone.
+
+ Valid variable names can contain characters, digits, and underscores.
+ See the section Variables below for details. Variable names are
+ case-sensitive.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>\sf[+] <replaceable class="parameter">function_description</> </literal></term>
<listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a9a2fdb..daa40b2 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1363,6 +1363,41 @@ exec_command(const char *cmd,
free(envval);
}
+ /* \setfileref - set variable by reference on file */
+ else if (strcmp(cmd, "setfileref") == 0)
+ {
+ char *name = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+ char *ref = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+ success = false;
+
+ if (!name && !ref)
+ {
+ /* list all variables */
+ PrintSetFileRefVariables(pset.vars);
+ success = true;
+ }
+ else
+ {
+ if (!name || !ref)
+ {
+ psql_error("\\%s: missing required argument\n", cmd);
+ success = false;
+ }
+ else
+ {
+ if (!SetFileRef(pset.vars, name, ref))
+ {
+ psql_error("\\%s: error while setting variable\n", cmd);
+ success = false;
+ }
+ }
+ }
+ }
+
/* \sf -- show a function's source code */
else if (strcmp(cmd, "sf") == 0 || strcmp(cmd, "sf+") == 0)
{
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a7789df..64fa5a2 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -12,6 +12,8 @@
#include <limits.h>
#include <math.h>
#include <signal.h>
+#include <sys/stat.h>
+
#ifndef WIN32
#include <unistd.h> /* for write() */
#else
@@ -25,6 +27,7 @@
#include "settings.h"
#include "command.h"
#include "copy.h"
+#include "catalog/pg_type.h"
#include "crosstabview.h"
#include "fe_utils/mbprint.h"
@@ -33,7 +36,6 @@ static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
static bool command_no_begin(const char *query);
static bool is_select_command(const char *query);
-
/*
* openQueryOutputFile --- attempt to open a query output file
*
@@ -109,6 +111,143 @@ setQFout(const char *fname)
return true;
}
+void
+psql_reset_query_params(void)
+{
+ int i;
+
+ for (i = 0; i < pset.nparams; i++)
+ if (pset.params[i] != NULL)
+ {
+ PQfreemem(pset.params[i]);
+ pset.params[i] = NULL;
+ }
+
+ pset.nparams = 0;
+}
+
+/*
+ * Load a content of the file_ref related file to query params buffer.
+ * When escaping is requested, then the text content is expected.
+ * Without escaping the bytea content is expected and related bytea
+ * escaping is processed.
+ */
+static char *
+get_file_ref_content(const char *value, bool escape, bool as_ident)
+{
+ PQExpBufferData buffer;
+ FILE *fd = NULL;
+ char *fname;
+ char *escaped_value;
+ char line[1024];
+ size_t size;
+ struct stat fst;
+
+ fname = pstrdup(value);
+
+ expand_tilde(&fname);
+ if (!fname)
+ {
+ psql_error("missing valid path to file\n");
+ return NULL;
+ }
+
+ canonicalize_path(fname);
+
+ fd = fopen(fname, PG_BINARY_R);
+ if (!fd)
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ /* ensure, max size of file content < 1GB */
+ if (fstat(fileno(fd), &fst) == -1)
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ if (fst.st_size > ((int64) 1024) * 1024 * 1024)
+ {
+ psql_error("file %s is too big (bigger than 1GB)\n", fname);
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ /* can append another parameter */
+ if (pset.nparams >= MAX_BINARY_PARAMS)
+ {
+ psql_error("too much binary parameters");
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ if (!pset.db)
+ {
+ psql_error("cannot escape without active connection\n");
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ initPQExpBuffer(&buffer);
+
+ while ((size = fread(line, 1, sizeof(line), fd)) > 0)
+ appendBinaryPQExpBuffer(&buffer, line, size);
+
+ if (ferror(fd))
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+ return NULL;
+ }
+
+ if (escape)
+ {
+ if (as_ident)
+ {
+ /* Identifiers should not be passed as query parameters */
+ psql_error("A file reference cannot be used as a identifier");
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+ return NULL;
+ }
+ else
+ {
+ /* escaping is not necessary for text parameters */
+ escaped_value = pstrdup(buffer.data);
+ pset.paramTypes[pset.nparams] = TEXTOID;
+ }
+ }
+ else
+ {
+ escaped_value = (char *)
+ PQescapeByteaConn(pset.db,
+ (const unsigned char *) buffer.data, buffer.len, &size);
+ pset.paramTypes[pset.nparams] = BYTEAOID;
+ }
+
+ /* fname, buffer is not necessary longer */
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+
+ if (escaped_value == NULL)
+ {
+ const char *error = PQerrorMessage(pset.db);
+
+ psql_error("%s", error);
+ return NULL;
+ }
+
+ pset.params[pset.nparams] = escaped_value;
+
+ snprintf(line, sizeof(line) - 1, "$%d", ++pset.nparams);
+
+ return pstrdup(line);
+}
/*
* Variable-fetching callback for flex lexer
@@ -125,11 +264,15 @@ psql_get_variable(const char *varname, bool escape, bool as_ident)
{
char *result;
const char *value;
+ bool is_file_ref;
- value = GetVariable(pset.vars, varname);
+ value = (char *) GetVariableOrFileRef(pset.vars, varname, &is_file_ref);
if (!value)
return NULL;
+ if (is_file_ref)
+ return get_file_ref_content(value, escape, as_ident);
+
if (escape)
{
char *escaped_value;
@@ -1287,7 +1430,16 @@ SendQuery(const char *query)
if (pset.timing)
INSTR_TIME_SET_CURRENT(before);
- results = PQexec(pset.db, query);
+ if (pset.nparams > 0)
+ results = PQexecParams(pset.db, query,
+ pset.nparams,
+ pset.paramTypes,
+ (const char * const *) pset.params,
+ NULL,
+ NULL,
+ 0);
+ else
+ results = PQexec(pset.db, query);
/* these operations are included in the timing result: */
ResetCancelConn();
@@ -1432,7 +1584,6 @@ sendquery_cleanup:
return OK;
}
-
/*
* ExecQueryUsingCursor: run a SELECT-like query using a cursor
*
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index bdcb58f..4f46b9c 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -38,6 +38,8 @@ extern int PSQLexecWatch(const char *query, const printQueryOpt *opt);
extern bool SendQuery(const char *query);
+void psql_reset_query_params(void);
+
extern bool is_superuser(void);
extern bool standard_strings(void);
extern const char *session_username(void);
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 37dfa4d..23fd6d3 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -23,7 +23,6 @@ const PsqlScanCallbacks psqlscan_callbacks = {
psql_error
};
-
/*
* Main processing loop for reading lines of input
* and sending them to the backend.
@@ -403,6 +402,9 @@ MainLoop(FILE *source)
psql_scan_finish(scan_state);
free(line);
+ /* reset binary parameters */
+ psql_reset_query_params();
+
if (slashCmdStatus == PSQL_CMD_TERMINATE)
{
successResult = EXIT_SUCCESS;
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 8cfe9d2..2874704 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -77,6 +77,8 @@ enum trivalue
TRI_YES
};
+#define MAX_BINARY_PARAMS 32
+
typedef struct _psqlSettings
{
PGconn *db; /* connection to backend */
@@ -135,6 +137,9 @@ typedef struct _psqlSettings
const char *prompt3;
PGVerbosity verbosity; /* current error verbosity level */
PGContextVisibility show_context; /* current context display level */
+ int nparams;
+ Oid paramTypes[MAX_BINARY_PARAMS];
+ char *params[MAX_BINARY_PARAMS];
} PsqlSettings;
extern PsqlSettings pset;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 7ce05fb..05715a6 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -107,6 +107,7 @@ main(int argc, char *argv[])
char password[100];
char *password_prompt = NULL;
bool new_pass;
+ int i;
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("psql"));
@@ -140,6 +141,10 @@ main(int argc, char *argv[])
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
+ pset.nparams = 0;
+ for (i = 0; i < MAX_BINARY_PARAMS; i++)
+ pset.params[i] = NULL;
+
/* We rely on unmentioned fields of pset.popt to start out 0/false/NULL */
pset.popt.topt.format = PRINT_ALIGNED;
pset.popt.topt.border = 1;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 50a45eb..ff4bf73 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1294,7 +1294,7 @@ psql_completion(const char *text, int start, int end)
"\\f", "\\g", "\\gexec", "\\gset", "\\h", "\\help", "\\H", "\\i", "\\ir", "\\l",
"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\\qecho", "\\r",
- "\\s", "\\set", "\\setenv", "\\sf", "\\sv", "\\t", "\\T",
+ "\\s", "\\set", "\\setenv", "\\setfileref", "\\sf", "\\sv", "\\t", "\\T",
"\\timing", "\\unset", "\\x", "\\w", "\\watch", "\\z", "\\!", NULL
};
@@ -3128,6 +3128,12 @@ psql_completion(const char *text, int start, int end)
matches = completion_matches(text, complete_from_files);
}
+ else if (Matches2("\\setfileref", MatchAny))
+ {
+ completion_charp = "\\";
+ matches = completion_matches(text, complete_from_files);
+ }
+
/*
* Finally, we look through the list of "things", such as TABLE, INDEX and
* check if that was the previous word. If so, execute the query to get a
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index f43f418..74b3f7b 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -71,6 +71,31 @@ GetVariable(VariableSpace space, const char *name)
if (strcmp(current->name, name) == 0)
{
/* this is correct answer when value is NULL, too */
+
+ return current->value;
+ }
+ }
+
+ return NULL;
+}
+
+const char *
+GetVariableOrFileRef(VariableSpace space, const char *name, bool *is_file_ref)
+{
+ struct _variable *current;
+
+ if (!space)
+ return NULL;
+
+ for (current = space->next; current; current = current->next)
+ {
+ if (strcmp(current->name, name) == 0)
+ {
+ /* this is correct answer when value is NULL, too */
+
+ if (is_file_ref)
+ *is_file_ref = current->is_file_ref;
+
return current->value;
}
}
@@ -178,7 +203,7 @@ PrintVariables(VariableSpace space)
for (ptr = space->next; ptr; ptr = ptr->next)
{
if (ptr->value)
- printf("%s = '%s'\n", ptr->name, ptr->value);
+ printf("%s = %s'%s'\n", ptr->name, ptr->is_file_ref ? "^" : "", ptr->value);
if (cancel_pressed)
break;
}
@@ -218,6 +243,64 @@ SetVariable(VariableSpace space, const char *name, const char *value)
/* not present, make new entry */
current = pg_malloc(sizeof *current);
current->name = pg_strdup(name);
+ current->is_file_ref = false;
+ current->value = pg_strdup(value);
+ current->assign_hook = NULL;
+ current->next = NULL;
+ previous->next = current;
+ return true;
+}
+
+void
+PrintSetFileRefVariables(VariableSpace space)
+{
+ struct _variable *ptr;
+
+ if (!space)
+ return;
+
+ for (ptr = space->next; ptr; ptr = ptr->next)
+ {
+ if (ptr->is_file_ref && ptr->value)
+ printf("%s = '%s'\n", ptr->name, ptr->value);
+ if (cancel_pressed)
+ break;
+ }
+}
+
+bool
+SetFileRef(VariableSpace space, const char *name, const char *value)
+{
+ struct _variable *current,
+ *previous;
+
+ if (!space)
+ return false;
+
+ if (!valid_variable_name(name))
+ return false;
+
+ if (!value)
+ return DeleteVariable(space, name);
+
+ for (previous = space, current = space->next;
+ current;
+ previous = current, current = current->next)
+ {
+ if (strcmp(current->name, name) == 0)
+ {
+ /* found entry, so update */
+ if (current->value)
+ free(current->value);
+ current->value = pg_strdup(value);
+ return true;
+ }
+ }
+
+ /* not present, make new entry */
+ current = pg_malloc(sizeof *current);
+ current->is_file_ref = true;
+ current->name = pg_strdup(name);
current->value = pg_strdup(value);
current->assign_hook = NULL;
current->next = NULL;
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index d7a05a1..bad4e37 100644
--- a/src/bin/psql/variables.h
+++ b/src/bin/psql/variables.h
@@ -26,6 +26,7 @@ struct _variable
{
char *name;
char *value;
+ bool is_file_ref;
VariableAssignHook assign_hook;
struct _variable *next;
};
@@ -34,6 +35,7 @@ typedef struct _variable *VariableSpace;
VariableSpace CreateVariableSpace(void);
const char *GetVariable(VariableSpace space, const char *name);
+const char *GetVariableOrFileRef(VariableSpace space, const char *name, bool *is_file_ref);
bool ParseVariableBool(const char *value, const char *name);
int ParseVariableNum(const char *val,
@@ -47,8 +49,11 @@ int GetVariableNum(VariableSpace space,
bool allowtrail);
void PrintVariables(VariableSpace space);
+void PrintSetFileRefVariables(VariableSpace space);
+
bool SetVariable(VariableSpace space, const char *name, const char *value);
+bool SetFileRef(VariableSpace space, const char *name, const char *value);
bool SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook);
bool SetVariableBool(VariableSpace space, const char *name);
bool DeleteVariable(VariableSpace space, const char *name);
diff --git a/src/test/regress/input/misc.source b/src/test/regress/input/misc.source
index dd2d1b2..1ca29fb 100644
--- a/src/test/regress/input/misc.source
+++ b/src/test/regress/input/misc.source
@@ -263,6 +263,29 @@ select i, length(t), octet_length(t), oldstyle_length(i,t) from oldstyle_test;
drop table oldstyle_test;
--
+-- psql refvariables
+--
+
+CREATE TABLE test_setref(a text, b bytea);
+
+-- use two different ways for import data - result should be same
+\lo_import @abs_builddir@/data/hash.data
+\set lo_oid :LASTOID
+INSERT INTO test_setref (b) VALUES(lo_get(:lo_oid));
+\lo_unlink :lo_oid
+SELECT md5(b) FROM test_setref;
+TRUNCATE test_setref;
+
+\setfileref testdata @abs_builddir@/data/hash.data
+INSERT INTO test_setref VALUES(convert_from(:testdata, current_setting('server_encoding')), :testdata);
+SELECT md5(a), md5(b) FROM test_setref;
+TRUNCATE test_setref;
+INSERT INTO test_setref(a) VALUES(:'testdata');
+SELECT md5(a) FROM test_setref;
+
+DROP TABLE test_setref;
+
+--
-- functional joins
--
diff --git a/src/test/regress/output/misc.source b/src/test/regress/output/misc.source
index 574ef0d..a26b461 100644
--- a/src/test/regress/output/misc.source
+++ b/src/test/regress/output/misc.source
@@ -700,6 +700,39 @@ select i, length(t), octet_length(t), oldstyle_length(i,t) from oldstyle_test;
drop table oldstyle_test;
--
+-- psql refvariables
+--
+CREATE TABLE test_setref(a text, b bytea);
+-- use two different ways for import data - result should be same
+\lo_import @abs_builddir@/data/hash.data
+\set lo_oid :LASTOID
+INSERT INTO test_setref (b) VALUES(lo_get(:lo_oid));
+\lo_unlink :lo_oid
+SELECT md5(b) FROM test_setref;
+ md5
+----------------------------------
+ e446fe6ea5a347e69670633412c7f8cb
+(1 row)
+
+TRUNCATE test_setref;
+\setfileref testdata @abs_builddir@/data/hash.data
+INSERT INTO test_setref VALUES(convert_from(:testdata, current_setting('server_encoding')), :testdata);
+SELECT md5(a), md5(b) FROM test_setref;
+ md5 | md5
+----------------------------------+----------------------------------
+ e446fe6ea5a347e69670633412c7f8cb | e446fe6ea5a347e69670633412c7f8cb
+(1 row)
+
+TRUNCATE test_setref;
+INSERT INTO test_setref(a) VALUES(:'testdata');
+SELECT md5(a) FROM test_setref;
+ md5
+----------------------------------
+ e446fe6ea5a347e69670633412c7f8cb
+(1 row)
+
+DROP TABLE test_setref;
+--
-- functional joins
--
--
here is new update - some mentioned issues are fixed + regress tests and
docs
This might tie into some work I'm doing. Is there any way these filerefs
could be used as the inline portion of a COPY command?
i.e. like this:
COPY my_table FROM STDIN
:file_ref
\.
2016-10-09 17:27 GMT+02:00 Corey Huinker <corey.huinker@gmail.com>:
here is new update - some mentioned issues are fixed + regress tests and
docs
This might tie into some work I'm doing. Is there any way these filerefs
could be used as the inline portion of a COPY command?
i.e. like this:COPY my_table FROM STDIN
:file_ref
\.
I understand, but I am not sure how difficult implementation it is. This
part (COPY input) doesn't support parametrization - and parametrization can
have a negative performance impact.
Regards
Pavel
On Sun, Oct 9, 2016 at 12:12 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
2016-10-09 17:27 GMT+02:00 Corey Huinker <corey.huinker@gmail.com>:
here is new update - some mentioned issues are fixed + regress tests and
docs
This might tie into some work I'm doing. Is there any way these filerefs
could be used as the inline portion of a COPY command?
i.e. like this:COPY my_table FROM STDIN
:file_ref
\.I understand, but I am not sure how difficult implementation it is. This
part (COPY input) doesn't support parametrization - and parametrization can
have a negative performance impact.Regards
I may not understand your response. I was thinking we'd want :file_ref to
simply 'cat' the file (or program) in place. The fact that the output was
consumed by COPY was coincidental. Does that chance your response?
2016-10-09 19:14 GMT+02:00 Corey Huinker <corey.huinker@gmail.com>:
On Sun, Oct 9, 2016 at 12:12 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:2016-10-09 17:27 GMT+02:00 Corey Huinker <corey.huinker@gmail.com>:
here is new update - some mentioned issues are fixed + regress tests and
docs
This might tie into some work I'm doing. Is there any way these filerefs
could be used as the inline portion of a COPY command?
i.e. like this:COPY my_table FROM STDIN
:file_ref
\.I understand, but I am not sure how difficult implementation it is. This
part (COPY input) doesn't support parametrization - and parametrization can
have a negative performance impact.Regards
I may not understand your response. I was thinking we'd want :file_ref to
simply 'cat' the file (or program) in place. The fact that the output was
consumed by COPY was coincidental. Does that chance your response?
look to code - some parts allows psql session variables, some not - I can
use variables in statements - not in data block.
More, there is ambiguity - :xxx should not be part of SQL statement (and
then psql variables are possible), but :xxxx should be part of data.
Regards
Pavel
Show quoted text
look to code - some parts allows psql session variables, some not - I can
use variables in statements - not in data block.More, there is ambiguity - :xxx should not be part of SQL statement (and
then psql variables are possible), but :xxxx should be part of data.Regards
Pavel
Makes sense, thanks for clearing it up.
2016-10-09 19:27 GMT+02:00 Corey Huinker <corey.huinker@gmail.com>:
look to code - some parts allows psql session variables, some not - I can
use variables in statements - not in data block.
More, there is ambiguity - :xxx should not be part of SQL statement (and
then psql variables are possible), but :xxxx should be part of data.Regards
Pavel
Makes sense, thanks for clearing it up.
ok
Regards
Pavel
Show quoted text
On Sun, Oct 9, 2016 at 06:12:16PM +0200, Pavel Stehule wrote:
2016-10-09 17:27 GMT+02:00 Corey Huinker <corey.huinker@gmail.com>:
here is new update - some mentioned issues are fixed + regress
tests and docs�
This might tie into some work I'm doing. Is there any way these filerefs
could be used as the inline portion of a COPY command?
i.e. like this:COPY my_table FROM STDIN
:file_ref
\.I understand, but I am not sure how difficult implementation it is. This part
(COPY input) doesn't support parametrization - and parametrization can have a
negative performance impact.
And it would need to be \:file_ref in COPY so real data doesn't trigger
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. +
+ Ancient Roman grave inscription +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 10/9/16 9:54 PM, Bruce Momjian wrote:
I understand, but I am not sure how difficult implementation it is. This part
(COPY input) doesn't support parametrization - and parametrization can have a
negative performance impact.And it would need to be \:file_ref in COPY so real data doesn't trigger
it.
ISTM it'd be much saner to just restrict file ref's to only working with
psql's \COPY, and not server-side COPY.
--
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
855-TREBLE2 (855-873-2532) mobile: 512-569-9461
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Sun, Oct 9, 2016 at 11:26 PM, Jim Nasby <Jim.Nasby@bluetreble.com> wrote:
On 10/9/16 9:54 PM, Bruce Momjian wrote:
I understand, but I am not sure how difficult implementation it is. This
part
(COPY input) doesn't support parametrization - and parametrization can
have a
negative performance impact.
And it would need to be \:file_ref in COPY so real data doesn't trigger
it.ISTM it'd be much saner to just restrict file ref's to only working with
psql's \COPY, and not server-side COPY.
Which is a great discussion for the thread on "COPY as a set returning
function". I didn't mean to hijack this thread with a misguided tie-in.
2016-10-10 5:26 GMT+02:00 Jim Nasby <Jim.Nasby@bluetreble.com>:
On 10/9/16 9:54 PM, Bruce Momjian wrote:
I understand, but I am not sure how difficult implementation it is. This
part
(COPY input) doesn't support parametrization - and parametrization can
have a
negative performance impact.
And it would need to be \:file_ref in COPY so real data doesn't trigger
it.ISTM it'd be much saner to just restrict file ref's to only working with
psql's \COPY, and not server-side COPY.
What should be a benefit, use case for file ref for client side \COPY ?
Regards
Pavel
Show quoted text
--
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
855-TREBLE2 (855-873-2532) mobile: 512-569-9461
Le 09/10/2016 � 11:48, Pavel Stehule a �crit :
hi
2016-10-04 9:18 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com
<mailto:gilles.darold@dalibo.com>>:Le 03/10/2016 � 23:23, Gilles Darold a �crit :
Le 03/10/2016 � 23:03, Robert Haas a �crit :
On Mon, Oct 3, 2016 at 3:54 PM, Gilles Darold
<gilles@darold.net <mailto:gilles@darold.net>> wrote:
4) An other problem is that like this this patch will allow
anyone to upload into a
column the content of any system file that can be read by
postgres system user
and then allow non system user to read its content.
I thought this was a client-side feature, so that it would let a
client upload any file that the client can read, but not thingsthat
can only be read by the postgres system user.
Yes that's right, sorry for the noise, forget this fourth report.
After some more though there is still a security issue here. For a
PostgreSQL user who also have login acces to the server, it is
possible
to read any file that the postgres system user can read, especially a
.pgpass or a recovery.conf containing password.here is new update - some mentioned issues are fixed + regress tests
and docs
Looks very good for me minus the two following points:
1) I think \setfileref must return the same syntax than \set command
postgres=# \setfileref a testfile.txt
postgres=# \setfileref
a = 'testfile.txt'
postgres=# \setfileref
...
a = ^'testfile.txt'
I think it would be better to prefixed the variable value with the ^ too
like in the \set report even if we know by using this command that
reported variables are file references.
2) You still allow special file to be used, I understand that this is
from the user responsibility but I think it could be a wise precaution.
postgres=# \setfileref b /dev/random
postgres=# insert into test (:b);
Process need to be killed using SIGTERM.
However if this last point is not critical and should be handle by the
user, I think this patch is ready to be reviewed by a committer after
fixing the first point.
Regards,
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
Gilles Darold wrote:
postgres=# \setfileref b /dev/random
postgres=# insert into test (:b);Process need to be killed using SIGTERM.
This can be fixed by setting sigint_interrupt_enabled to true
before operating on the file. Then ctrl-C would be able to cancel
the command.
See comment in common.c, above the declaration of
sigint_interrupt_enabled:
/*
[....]
* SIGINT is supposed to abort all long-running psql operations, not only
* database queries. In most places, this is accomplished by checking
* cancel_pressed during long-running loops. However, that won't work when
* blocked on user input (in readline() or fgets()). In those places, we
* set sigint_interrupt_enabled TRUE while blocked, instructing the signal
* catcher to longjmp through sigint_interrupt_jmp. We assume readline and
* fgets are coded to handle possible interruption. (XXX currently this does
* not work on win32, so control-C is less useful there)
*/
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Le 10/10/2016 ᅵ 14:38, Daniel Verite a ᅵcrit :
Gilles Darold wrote:
postgres=# \setfileref b /dev/random
postgres=# insert into test (:b);Process need to be killed using SIGTERM.
This can be fixed by setting sigint_interrupt_enabled to true
before operating on the file. Then ctrl-C would be able to cancel
the command.
If we do not prevent the user to be able to load special files that
would be useful yes.
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-10-10 15:17 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com>:
Le 10/10/2016 à 14:38, Daniel Verite a écrit :
Gilles Darold wrote:
postgres=# \setfileref b /dev/random
postgres=# insert into test (:b);Process need to be killed using SIGTERM.
This can be fixed by setting sigint_interrupt_enabled to true
before operating on the file. Then ctrl-C would be able to cancel
the command.If we do not prevent the user to be able to load special files that
would be useful yes.
I don't think so special files has some sense, just I had not time fix this
issue. I will do it early - I hope.
Regards
Pavel
Show quoted text
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
2016-10-10 19:50 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
2016-10-10 15:17 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com>:
Le 10/10/2016 à 14:38, Daniel Verite a écrit :
Gilles Darold wrote:
postgres=# \setfileref b /dev/random
postgres=# insert into test (:b);Process need to be killed using SIGTERM.
This can be fixed by setting sigint_interrupt_enabled to true
before operating on the file. Then ctrl-C would be able to cancel
the command.If we do not prevent the user to be able to load special files that
would be useful yes.I don't think so special files has some sense, just I had not time fix
this issue. I will do it early - I hope.
fresh patch - only regular files are allowed
Regards
Pavel
Show quoted text
Regards
Pavel
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
Attachments:
psql-setfileref-2016-10-11.patchtext/x-patch; charset=US-ASCII; name=psql-setfileref-2016-10-11.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 4806e77..9fb9cd9 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2714,6 +2714,24 @@ testdb=> <userinput>\setenv LESS -imx4F</userinput>
</varlistentry>
<varlistentry>
+ <term><literal>\setfileref <replaceable class="parameter">name</replaceable> [ <replaceable class="parameter">value</replaceable> ]</literal></term>
+
+ <listitem>
+ <para>
+ Sets the internal variable name as a reference to the file path
+ set as value. To unset a variable, use the <command>\unset</command> command.
+
+ File references are shown as file path prefixed with the ^ character
+ when using the <command>\set</command> command alone.
+
+ Valid variable names can contain characters, digits, and underscores.
+ See the section Variables below for details. Variable names are
+ case-sensitive.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
<term><literal>\sf[+] <replaceable class="parameter">function_description</> </literal></term>
<listitem>
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index a9a2fdb..daa40b2 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -1363,6 +1363,41 @@ exec_command(const char *cmd,
free(envval);
}
+ /* \setfileref - set variable by reference on file */
+ else if (strcmp(cmd, "setfileref") == 0)
+ {
+ char *name = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+ char *ref = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+
+ success = false;
+
+ if (!name && !ref)
+ {
+ /* list all variables */
+ PrintSetFileRefVariables(pset.vars);
+ success = true;
+ }
+ else
+ {
+ if (!name || !ref)
+ {
+ psql_error("\\%s: missing required argument\n", cmd);
+ success = false;
+ }
+ else
+ {
+ if (!SetFileRef(pset.vars, name, ref))
+ {
+ psql_error("\\%s: error while setting variable\n", cmd);
+ success = false;
+ }
+ }
+ }
+ }
+
/* \sf -- show a function's source code */
else if (strcmp(cmd, "sf") == 0 || strcmp(cmd, "sf+") == 0)
{
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a7789df..58b0065 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -12,6 +12,8 @@
#include <limits.h>
#include <math.h>
#include <signal.h>
+#include <sys/stat.h>
+
#ifndef WIN32
#include <unistd.h> /* for write() */
#else
@@ -25,6 +27,7 @@
#include "settings.h"
#include "command.h"
#include "copy.h"
+#include "catalog/pg_type.h"
#include "crosstabview.h"
#include "fe_utils/mbprint.h"
@@ -33,7 +36,6 @@ static bool ExecQueryUsingCursor(const char *query, double *elapsed_msec);
static bool command_no_begin(const char *query);
static bool is_select_command(const char *query);
-
/*
* openQueryOutputFile --- attempt to open a query output file
*
@@ -109,6 +111,150 @@ setQFout(const char *fname)
return true;
}
+void
+psql_reset_query_params(void)
+{
+ int i;
+
+ for (i = 0; i < pset.nparams; i++)
+ if (pset.params[i] != NULL)
+ {
+ PQfreemem(pset.params[i]);
+ pset.params[i] = NULL;
+ }
+
+ pset.nparams = 0;
+}
+
+/*
+ * Load a content of the file_ref related file to query params buffer.
+ * When escaping is requested, then the text content is expected.
+ * Without escaping the bytea content is expected and related bytea
+ * escaping is processed.
+ */
+static char *
+get_file_ref_content(const char *value, bool escape, bool as_ident)
+{
+ PQExpBufferData buffer;
+ FILE *fd = NULL;
+ char *fname;
+ char *escaped_value;
+ char line[1024];
+ size_t size;
+ struct stat fst;
+
+ fname = pstrdup(value);
+
+ expand_tilde(&fname);
+ if (!fname)
+ {
+ psql_error("missing valid path to file\n");
+ return NULL;
+ }
+
+ canonicalize_path(fname);
+
+ fd = fopen(fname, PG_BINARY_R);
+ if (!fd)
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ /* ensure, max size of file content < 1GB */
+ if (fstat(fileno(fd), &fst) == -1)
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ if (!S_ISREG(fst.st_mode))
+ {
+ psql_error("referenced file of file ref variable is not regular file\n");
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ if (fst.st_size > ((int64) 1024) * 1024 * 1024)
+ {
+ psql_error("file %s is too big (bigger than 1GB)\n", fname);
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ /* can append another parameter */
+ if (pset.nparams >= MAX_BINARY_PARAMS)
+ {
+ psql_error("too much binary parameters");
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ if (!pset.db)
+ {
+ psql_error("cannot escape without active connection\n");
+ PQfreemem(fname);
+ return NULL;
+ }
+
+ initPQExpBuffer(&buffer);
+
+ while ((size = fread(line, 1, sizeof(line), fd)) > 0)
+ appendBinaryPQExpBuffer(&buffer, line, size);
+
+ if (ferror(fd))
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+ return NULL;
+ }
+
+ if (escape)
+ {
+ if (as_ident)
+ {
+ /* Identifiers should not be passed as query parameters */
+ psql_error("A file reference cannot be used as a identifier");
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+ return NULL;
+ }
+ else
+ {
+ /* escaping is not necessary for text parameters */
+ escaped_value = pstrdup(buffer.data);
+ pset.paramTypes[pset.nparams] = TEXTOID;
+ }
+ }
+ else
+ {
+ escaped_value = (char *)
+ PQescapeByteaConn(pset.db,
+ (const unsigned char *) buffer.data, buffer.len, &size);
+ pset.paramTypes[pset.nparams] = BYTEAOID;
+ }
+
+ /* fname, buffer is not necessary longer */
+ PQfreemem(fname);
+ termPQExpBuffer(&buffer);
+
+ if (escaped_value == NULL)
+ {
+ const char *error = PQerrorMessage(pset.db);
+
+ psql_error("%s", error);
+ return NULL;
+ }
+
+ pset.params[pset.nparams] = escaped_value;
+
+ snprintf(line, sizeof(line) - 1, "$%d", ++pset.nparams);
+
+ return pstrdup(line);
+}
/*
* Variable-fetching callback for flex lexer
@@ -125,11 +271,15 @@ psql_get_variable(const char *varname, bool escape, bool as_ident)
{
char *result;
const char *value;
+ bool is_file_ref;
- value = GetVariable(pset.vars, varname);
+ value = (char *) GetVariableOrFileRef(pset.vars, varname, &is_file_ref);
if (!value)
return NULL;
+ if (is_file_ref)
+ return get_file_ref_content(value, escape, as_ident);
+
if (escape)
{
char *escaped_value;
@@ -1287,7 +1437,16 @@ SendQuery(const char *query)
if (pset.timing)
INSTR_TIME_SET_CURRENT(before);
- results = PQexec(pset.db, query);
+ if (pset.nparams > 0)
+ results = PQexecParams(pset.db, query,
+ pset.nparams,
+ pset.paramTypes,
+ (const char * const *) pset.params,
+ NULL,
+ NULL,
+ 0);
+ else
+ results = PQexec(pset.db, query);
/* these operations are included in the timing result: */
ResetCancelConn();
@@ -1432,7 +1591,6 @@ sendquery_cleanup:
return OK;
}
-
/*
* ExecQueryUsingCursor: run a SELECT-like query using a cursor
*
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index bdcb58f..4f46b9c 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -38,6 +38,8 @@ extern int PSQLexecWatch(const char *query, const printQueryOpt *opt);
extern bool SendQuery(const char *query);
+void psql_reset_query_params(void);
+
extern bool is_superuser(void);
extern bool standard_strings(void);
extern const char *session_username(void);
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 37dfa4d..23fd6d3 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -23,7 +23,6 @@ const PsqlScanCallbacks psqlscan_callbacks = {
psql_error
};
-
/*
* Main processing loop for reading lines of input
* and sending them to the backend.
@@ -403,6 +402,9 @@ MainLoop(FILE *source)
psql_scan_finish(scan_state);
free(line);
+ /* reset binary parameters */
+ psql_reset_query_params();
+
if (slashCmdStatus == PSQL_CMD_TERMINATE)
{
successResult = EXIT_SUCCESS;
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 8cfe9d2..2874704 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -77,6 +77,8 @@ enum trivalue
TRI_YES
};
+#define MAX_BINARY_PARAMS 32
+
typedef struct _psqlSettings
{
PGconn *db; /* connection to backend */
@@ -135,6 +137,9 @@ typedef struct _psqlSettings
const char *prompt3;
PGVerbosity verbosity; /* current error verbosity level */
PGContextVisibility show_context; /* current context display level */
+ int nparams;
+ Oid paramTypes[MAX_BINARY_PARAMS];
+ char *params[MAX_BINARY_PARAMS];
} PsqlSettings;
extern PsqlSettings pset;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 7ce05fb..05715a6 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -107,6 +107,7 @@ main(int argc, char *argv[])
char password[100];
char *password_prompt = NULL;
bool new_pass;
+ int i;
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("psql"));
@@ -140,6 +141,10 @@ main(int argc, char *argv[])
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
+ pset.nparams = 0;
+ for (i = 0; i < MAX_BINARY_PARAMS; i++)
+ pset.params[i] = NULL;
+
/* We rely on unmentioned fields of pset.popt to start out 0/false/NULL */
pset.popt.topt.format = PRINT_ALIGNED;
pset.popt.topt.border = 1;
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index 50a45eb..ff4bf73 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1294,7 +1294,7 @@ psql_completion(const char *text, int start, int end)
"\\f", "\\g", "\\gexec", "\\gset", "\\h", "\\help", "\\H", "\\i", "\\ir", "\\l",
"\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
"\\o", "\\p", "\\password", "\\prompt", "\\pset", "\\q", "\\qecho", "\\r",
- "\\s", "\\set", "\\setenv", "\\sf", "\\sv", "\\t", "\\T",
+ "\\s", "\\set", "\\setenv", "\\setfileref", "\\sf", "\\sv", "\\t", "\\T",
"\\timing", "\\unset", "\\x", "\\w", "\\watch", "\\z", "\\!", NULL
};
@@ -3128,6 +3128,12 @@ psql_completion(const char *text, int start, int end)
matches = completion_matches(text, complete_from_files);
}
+ else if (Matches2("\\setfileref", MatchAny))
+ {
+ completion_charp = "\\";
+ matches = completion_matches(text, complete_from_files);
+ }
+
/*
* Finally, we look through the list of "things", such as TABLE, INDEX and
* check if that was the previous word. If so, execute the query to get a
diff --git a/src/bin/psql/variables.c b/src/bin/psql/variables.c
index f43f418..d75a590 100644
--- a/src/bin/psql/variables.c
+++ b/src/bin/psql/variables.c
@@ -71,6 +71,31 @@ GetVariable(VariableSpace space, const char *name)
if (strcmp(current->name, name) == 0)
{
/* this is correct answer when value is NULL, too */
+
+ return current->value;
+ }
+ }
+
+ return NULL;
+}
+
+const char *
+GetVariableOrFileRef(VariableSpace space, const char *name, bool *is_file_ref)
+{
+ struct _variable *current;
+
+ if (!space)
+ return NULL;
+
+ for (current = space->next; current; current = current->next)
+ {
+ if (strcmp(current->name, name) == 0)
+ {
+ /* this is correct answer when value is NULL, too */
+
+ if (is_file_ref)
+ *is_file_ref = current->is_file_ref;
+
return current->value;
}
}
@@ -178,7 +203,7 @@ PrintVariables(VariableSpace space)
for (ptr = space->next; ptr; ptr = ptr->next)
{
if (ptr->value)
- printf("%s = '%s'\n", ptr->name, ptr->value);
+ printf("%s = %s'%s'\n", ptr->name, ptr->is_file_ref ? "^" : "", ptr->value);
if (cancel_pressed)
break;
}
@@ -218,6 +243,64 @@ SetVariable(VariableSpace space, const char *name, const char *value)
/* not present, make new entry */
current = pg_malloc(sizeof *current);
current->name = pg_strdup(name);
+ current->is_file_ref = false;
+ current->value = pg_strdup(value);
+ current->assign_hook = NULL;
+ current->next = NULL;
+ previous->next = current;
+ return true;
+}
+
+void
+PrintSetFileRefVariables(VariableSpace space)
+{
+ struct _variable *ptr;
+
+ if (!space)
+ return;
+
+ for (ptr = space->next; ptr; ptr = ptr->next)
+ {
+ if (ptr->is_file_ref && ptr->value)
+ printf("%s = ^'%s'\n", ptr->name, ptr->value);
+ if (cancel_pressed)
+ break;
+ }
+}
+
+bool
+SetFileRef(VariableSpace space, const char *name, const char *value)
+{
+ struct _variable *current,
+ *previous;
+
+ if (!space)
+ return false;
+
+ if (!valid_variable_name(name))
+ return false;
+
+ if (!value)
+ return DeleteVariable(space, name);
+
+ for (previous = space, current = space->next;
+ current;
+ previous = current, current = current->next)
+ {
+ if (strcmp(current->name, name) == 0)
+ {
+ /* found entry, so update */
+ if (current->value)
+ free(current->value);
+ current->value = pg_strdup(value);
+ return true;
+ }
+ }
+
+ /* not present, make new entry */
+ current = pg_malloc(sizeof *current);
+ current->is_file_ref = true;
+ current->name = pg_strdup(name);
current->value = pg_strdup(value);
current->assign_hook = NULL;
current->next = NULL;
diff --git a/src/bin/psql/variables.h b/src/bin/psql/variables.h
index d7a05a1..bad4e37 100644
--- a/src/bin/psql/variables.h
+++ b/src/bin/psql/variables.h
@@ -26,6 +26,7 @@ struct _variable
{
char *name;
char *value;
+ bool is_file_ref;
VariableAssignHook assign_hook;
struct _variable *next;
};
@@ -34,6 +35,7 @@ typedef struct _variable *VariableSpace;
VariableSpace CreateVariableSpace(void);
const char *GetVariable(VariableSpace space, const char *name);
+const char *GetVariableOrFileRef(VariableSpace space, const char *name, bool *is_file_ref);
bool ParseVariableBool(const char *value, const char *name);
int ParseVariableNum(const char *val,
@@ -47,8 +49,11 @@ int GetVariableNum(VariableSpace space,
bool allowtrail);
void PrintVariables(VariableSpace space);
+void PrintSetFileRefVariables(VariableSpace space);
+
bool SetVariable(VariableSpace space, const char *name, const char *value);
+bool SetFileRef(VariableSpace space, const char *name, const char *value);
bool SetVariableAssignHook(VariableSpace space, const char *name, VariableAssignHook hook);
bool SetVariableBool(VariableSpace space, const char *name);
bool DeleteVariable(VariableSpace space, const char *name);
diff --git a/src/test/regress/input/misc.source b/src/test/regress/input/misc.source
index dd2d1b2..1ca29fb 100644
--- a/src/test/regress/input/misc.source
+++ b/src/test/regress/input/misc.source
@@ -263,6 +263,29 @@ select i, length(t), octet_length(t), oldstyle_length(i,t) from oldstyle_test;
drop table oldstyle_test;
--
+-- psql refvariables
+--
+
+CREATE TABLE test_setref(a text, b bytea);
+
+-- use two different ways for import data - result should be same
+\lo_import @abs_builddir@/data/hash.data
+\set lo_oid :LASTOID
+INSERT INTO test_setref (b) VALUES(lo_get(:lo_oid));
+\lo_unlink :lo_oid
+SELECT md5(b) FROM test_setref;
+TRUNCATE test_setref;
+
+\setfileref testdata @abs_builddir@/data/hash.data
+INSERT INTO test_setref VALUES(convert_from(:testdata, current_setting('server_encoding')), :testdata);
+SELECT md5(a), md5(b) FROM test_setref;
+TRUNCATE test_setref;
+INSERT INTO test_setref(a) VALUES(:'testdata');
+SELECT md5(a) FROM test_setref;
+
+DROP TABLE test_setref;
+
+--
-- functional joins
--
diff --git a/src/test/regress/output/misc.source b/src/test/regress/output/misc.source
index 574ef0d..a26b461 100644
--- a/src/test/regress/output/misc.source
+++ b/src/test/regress/output/misc.source
@@ -700,6 +700,39 @@ select i, length(t), octet_length(t), oldstyle_length(i,t) from oldstyle_test;
drop table oldstyle_test;
--
+-- psql refvariables
+--
+CREATE TABLE test_setref(a text, b bytea);
+-- use two different ways for import data - result should be same
+\lo_import @abs_builddir@/data/hash.data
+\set lo_oid :LASTOID
+INSERT INTO test_setref (b) VALUES(lo_get(:lo_oid));
+\lo_unlink :lo_oid
+SELECT md5(b) FROM test_setref;
+ md5
+----------------------------------
+ e446fe6ea5a347e69670633412c7f8cb
+(1 row)
+
+TRUNCATE test_setref;
+\setfileref testdata @abs_builddir@/data/hash.data
+INSERT INTO test_setref VALUES(convert_from(:testdata, current_setting('server_encoding')), :testdata);
+SELECT md5(a), md5(b) FROM test_setref;
+ md5 | md5
+----------------------------------+----------------------------------
+ e446fe6ea5a347e69670633412c7f8cb | e446fe6ea5a347e69670633412c7f8cb
+(1 row)
+
+TRUNCATE test_setref;
+INSERT INTO test_setref(a) VALUES(:'testdata');
+SELECT md5(a) FROM test_setref;
+ md5
+----------------------------------
+ e446fe6ea5a347e69670633412c7f8cb
+(1 row)
+
+DROP TABLE test_setref;
+--
-- functional joins
--
--
Le 11/10/2016 � 07:53, Pavel Stehule a �crit :
2016-10-10 19:50 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com
<mailto:pavel.stehule@gmail.com>>:2016-10-10 15:17 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com
<mailto:gilles.darold@dalibo.com>>:Le 10/10/2016 � 14:38, Daniel Verite a �crit :
Gilles Darold wrote:
postgres=# \setfileref b /dev/random
postgres=# insert into test (:b);Process need to be killed using SIGTERM.
This can be fixed by setting sigint_interrupt_enabled to true
before operating on the file. Then ctrl-C would be able tocancel
the command.
If we do not prevent the user to be able to load special files
that
would be useful yes.I don't think so special files has some sense, just I had not time
fix this issue. I will do it early - I hope.fresh patch - only regular files are allowed
Regards
Pavel
Hi,
The patch works as expected, the last two remaining issues have been
fixed. I've changed the status to "Ready for committers".
Thanks Pavel.
--
Gilles Darold
Consultant PostgreSQL
http://dalibo.com - http://dalibo.org
2016-10-11 9:32 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com>:
Le 11/10/2016 à 07:53, Pavel Stehule a écrit :
2016-10-10 19:50 GMT+02:00 Pavel Stehule <pavel.stehule@gmail.com>:
2016-10-10 15:17 GMT+02:00 Gilles Darold <gilles.darold@dalibo.com>:
Le 10/10/2016 à 14:38, Daniel Verite a écrit :
Gilles Darold wrote:
postgres=# \setfileref b /dev/random
postgres=# insert into test (:b);Process need to be killed using SIGTERM.
This can be fixed by setting sigint_interrupt_enabled to true
before operating on the file. Then ctrl-C would be able to cancel
the command.If we do not prevent the user to be able to load special files that
would be useful yes.I don't think so special files has some sense, just I had not time fix
this issue. I will do it early - I hope.fresh patch - only regular files are allowed
Regards
Pavel
Hi,
The patch works as expected, the last two remaining issues have been
fixed. I've changed the status to "Ready for committers".Thanks Pavel.
Thank you very much
Regards
Pavel
Show quoted text
--
Gilles Darold
Consultant PostgreSQLhttp://dalibo.com - http://dalibo.org
Pavel Stehule <pavel.stehule@gmail.com> writes:
[ psql-setfileref-2016-10-11.patch ]
I haven't been paying any attention to this thread, but I got around to
looking at it finally. I follow the idea of wanting to be able to shove
the contents of a file into a query literal, but there isn't much else
that I like about this patch. In particular:
* I really dislike the notion of keying the behavior to a special type of
psql variable. psql variables are untyped at the moment, and if we were
going to introduce typing, this wouldn't be what I'd want to use it for.
I really don't want to introduce typing and then invent one-off,
unextensible syntax like '^' prefixes to denote what type a variable is.
Aside from being conceptually a mess, I don't even find it particularly
convenient. In the shell, if you want to source from a file, you write
"<filename". You aren't compelled to assign the filename to a variable
and then write "<$filename" ... although you can if that's actually
helpful.
Going by the notion of driving it off syntax not variable type, I'd
suggest that we extend the colon-variablename syntax to indicate
desire to read a file. :<filename< is one pretty obvious idea.
Maybe we could use :<:variablename< to indicate substituting the
content of a variable as the file name to read.
* I'm a bit queasy about the idea of automatically switching over to
parameterized queries when we have one of these things in the query.
I'm afraid that that will have user-visible consequences, so I would
rather that psql not do that behind the user's back. Plus, that assumes
a fact not in evidence, namely that you only ever want to insert data
and not code this way. (If \i were more flexible, that objection would
be moot, but you can't source just part of a query from \i AFAIK.)
There might be something to be said for a psql setting that controls
whether to handle colon-insertions this way, and make it apply to
the existing :'var' syntax as well as the filename syntax.
* I find the subthread about attaching this to COPY to be pretty much of
a red herring. What would that do that you can't do today with \copy?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi
2016-11-09 22:47 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
[ psql-setfileref-2016-10-11.patch ]
I haven't been paying any attention to this thread, but I got around to
looking at it finally. I follow the idea of wanting to be able to shove
the contents of a file into a query literal, but there isn't much else
that I like about this patch. In particular:* I really dislike the notion of keying the behavior to a special type of
psql variable. psql variables are untyped at the moment, and if we were
going to introduce typing, this wouldn't be what I'd want to use it for.
I really don't want to introduce typing and then invent one-off,
unextensible syntax like '^' prefixes to denote what type a variable is.
still I am thinking so some differencing can be nice, because we can use
psql file path tab autocomplete.
Maybe \setfileref can stay - it will set any variable, but the autocomplete
will be based on file path.
Aside from being conceptually a mess, I don't even find it particularly
convenient. In the shell, if you want to source from a file, you write
"<filename". You aren't compelled to assign the filename to a variable
and then write "<$filename" ... although you can if that's actually
helpful.Going by the notion of driving it off syntax not variable type, I'd
suggest that we extend the colon-variablename syntax to indicate
desire to read a file. :<filename< is one pretty obvious idea.
Maybe we could use :<:variablename< to indicate substituting the
content of a variable as the file name to read.
I used the concept of file references because I would not to invent new
syntax of psql variables evaluation.
If we introduce new syntax, then the variables are not necessary. The
syntax :some op has sense, and be used and enhanced in future.
What do you thing about following example?
INSERT INTO tab VALUES(1, :<varname); -- insert text value -- used text
escaping
INSERT INTO tab VALUES(1, :<#varname); -- insert bytea value -- used bytea
escaping
* I'm a bit queasy about the idea of automatically switching over to
parameterized queries when we have one of these things in the query.
I'm afraid that that will have user-visible consequences, so I would
rather that psql not do that behind the user's back. Plus, that assumes
a fact not in evidence, namely that you only ever want to insert data
and not code this way. (If \i were more flexible, that objection would
be moot, but you can't source just part of a query from \i AFAIK.)
There might be something to be said for a psql setting that controls
whether to handle colon-insertions this way, and make it apply to
the existing :'var' syntax as well as the filename syntax.
I understand to this objection - The my motivation for parametrized queries
was better (user friendly) reaction on syntax errors. In this case the
content can be big, the query can be big. When we use parametrized queries,
then the error message can be short and readable. Another advantage of
parametrized queries is possibility to set parameter type. It is important
for binary content. And last advantage is a possibility to use binary
passing - it is interesting for XML - it allows automatic encoding
conversions. These features are nice, but are not necessary for this patch.
* I find the subthread about attaching this to COPY to be pretty much of
a red herring. What would that do that you can't do today with \copy?
The primary task is simple - import big XML, JSON document or some binary
data to database. This can be partially solved by ref variables, but COPY
has more verbose and more natural syntax - the file path autocomplete can
be used.
\COPY table(column) FROM file FLAG;
Second task is not too complex too - export binary data from Postgres and
store these data in binary files. Now I have to use final transformation on
client side.
Third task - one interesting feature of XML type (automatic encoding
conversion) is available only with binary input output functions. I would
to find a way how this functionality can be accessed without "hard"
programming.
\COPY (SELECT xmldoc FROM xxx WHERE id = 111) TO file BINARY ENCODING
latin1;
Regards
Pavel
Show quoted text
regards, tom lane
Pavel Stehule <pavel.stehule@gmail.com> writes:
2016-11-09 22:47 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
* I really dislike the notion of keying the behavior to a special type of
psql variable.
still I am thinking so some differencing can be nice, because we can use
psql file path tab autocomplete.
Maybe \setfileref can stay - it will set any variable, but the autocomplete
will be based on file path.
Personally, I'd rather have filename tab completion after ":<", and forget
\setfileref --- I do not think that setting a variable first is going to
be the primary use-case for this. In fact, I could happily dispense with
the variable case entirely, except that sometimes people will want to
reference file names that don't syntactically fit into the colon syntax
(eg, names with spaces in them). Even that seems surmountable, if people
are okay with requiring the '<' trailer --- I don't think we need to worry
too much about supporting file names with '<' in them. (Likewise for
'>', if you feel like :<filename> is a less ugly syntax.)
What do you thing about following example?
INSERT INTO tab VALUES(1, :<varname); -- insert text value -- used text
escaping
INSERT INTO tab VALUES(1, :<#varname); -- insert bytea value -- used bytea
escaping
Seems a bit arbitrary, and not readily extensible to more datatypes.
I guess an advantage of the parameterized-query approach is that we
wouldn't really have to distinguish different datatypes in the syntax,
because we could use the result of Describe Statement to figure out what
to do. Maybe that outweighs the concern about magic behavioral changes.
On the other hand, it's also arguable that trying to cater for automatic
handling of raw files as bytea literals is overcomplicating the feature
in support of a very infrequent use-case, and giving up some other
infrequent use-cases to get that. An example is that if we insist on
doing this through parameterized queries, it will not work for inserting
literals into utility commands. I don't know which of these things is
more important to have.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-11-10 18:56 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
2016-11-09 22:47 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
* I really dislike the notion of keying the behavior to a special type
of
psql variable.
still I am thinking so some differencing can be nice, because we can use
psql file path tab autocomplete.
Maybe \setfileref can stay - it will set any variable, but theautocomplete
will be based on file path.
Personally, I'd rather have filename tab completion after ":<", and forget
\setfileref --- I do not think that setting a variable first is going to
be the primary use-case for this. In fact, I could happily dispense with
the variable case entirely, except that sometimes people will want to
reference file names that don't syntactically fit into the colon syntax
(eg, names with spaces in them). Even that seems surmountable, if people
are okay with requiring the '<' trailer --- I don't think we need to worry
too much about supporting file names with '<' in them. (Likewise for
'>', if you feel like :<filename> is a less ugly syntax.)
In this case I dislike '>' on the end. The semantic is less clear with it.
I though about dropping variables too, but I expecting a expandable
variable after colon syntax. So it need special clear syntax for disabling
variable expansion.
What about :<{filename} ?
INSERT INTO tab VALUES(1, :<{~/data/doc.txt});
What do you thing about following example?
INSERT INTO tab VALUES(1, :<varname); -- insert text value -- used text
escaping
INSERT INTO tab VALUES(1, :<#varname); -- insert bytea value -- usedbytea
escaping
Seems a bit arbitrary, and not readily extensible to more datatypes.
there are only two possibilities - text with client encoding to server
encoding conversions, and binary - without any conversions.
I guess an advantage of the parameterized-query approach is that we
wouldn't really have to distinguish different datatypes in the syntax,
because we could use the result of Describe Statement to figure out what
to do. Maybe that outweighs the concern about magic behavioral changes.
I don't understand - there is a possibility to detect type of parameter on
client side?
On the other hand, it's also arguable that trying to cater for automatic
handling of raw files as bytea literals is overcomplicating the feature
in support of a very infrequent use-case, and giving up some other
infrequent use-cases to get that. An example is that if we insist on
doing this through parameterized queries, it will not work for inserting
literals into utility commands. I don't know which of these things is
more important to have.
I had not idea a complete replacement of current mechanism by query
parameters. But a partial usage can be source of inconsistencies :(
Pavel
Show quoted text
regards, tom lane
2016-11-10 18:56 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
2016-11-09 22:47 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
* I really dislike the notion of keying the behavior to a special type
of
psql variable.
still I am thinking so some differencing can be nice, because we can use
psql file path tab autocomplete.
Maybe \setfileref can stay - it will set any variable, but theautocomplete
will be based on file path.
Personally, I'd rather have filename tab completion after ":<", and forget
\setfileref --- I do not think that setting a variable first is going to
be the primary use-case for this. In fact, I could happily dispense with
the variable case entirely, except that sometimes people will want to
reference file names that don't syntactically fit into the colon syntax
(eg, names with spaces in them). Even that seems surmountable, if people
are okay with requiring the '<' trailer --- I don't think we need to worry
too much about supporting file names with '<' in them. (Likewise for
'>', if you feel like :<filename> is a less ugly syntax.)
I found early stop - there are not easy implement any complex syntax in
lexer, without complex backup rules.
Now, I am coming with little bit different idea, different syntax.
:xxxx means insert some content based on psql variable
We can introduce psql content commands that produces some content. The
syntax can be
:{cmd parameters}
Some commands:
* r - read
* rq - read and quote, it can has a alias "<"
* rbq - read binary and quote
Other commands can be introduced in future
Usage:
INSERT INTO foo(image) VALUES(:{rbq ~/avatar.jpg})
INSERT INTO foo(xmdoc) VALUES(:{rq "~/current doc.xml"})
INSERT INTO foo(longtext) VALUES(:{< ~/book.txt})
It is general with possible simple implementation - doesn't big impact on
psql lexer complexity.
What do you think about it?
Regards
Pavel
p.s. the colon is not necessary - we can use {} as special case of ``.
INSERT INTO foo(image) VALUES({rbq ~/avatar.jpg})
INSERT INTO foo(xmdoc) VALUES({rq "~/current doc.xml"})
INSERT INTO foo(longtext) VALUES({< ~/book.txt})
Then we can support psql variables there
\set avatar ~/avatar.jpg
INSERT INTO foo(image) VALUES({rbq :avatar})
Show quoted text
What do you thing about following example?
INSERT INTO tab VALUES(1, :<varname); -- insert text value -- used text
escaping
INSERT INTO tab VALUES(1, :<#varname); -- insert bytea value -- usedbytea
escaping
Seems a bit arbitrary, and not readily extensible to more datatypes.
I guess an advantage of the parameterized-query approach is that we
wouldn't really have to distinguish different datatypes in the syntax,
because we could use the result of Describe Statement to figure out what
to do. Maybe that outweighs the concern about magic behavioral changes.On the other hand, it's also arguable that trying to cater for automatic
handling of raw files as bytea literals is overcomplicating the feature
in support of a very infrequent use-case, and giving up some other
infrequent use-cases to get that. An example is that if we insist on
doing this through parameterized queries, it will not work for inserting
literals into utility commands. I don't know which of these things is
more important to have.regards, tom lane
Hi
I am sending a initial implementation of psql content commands. This design
is reaction to Tom's objections against psql file ref variables. This
design is more cleaner, more explicit and more practical - import can be in
one step.
Now supported commands are:
r - read file without any modification
rq - read file, escape as literal and use outer quotes
rb - read binary file - transform to hex code string
rbq - read binary file, transform to hex code string and use outer quotes
Usage:
create table testt(a xml);
insert into test values( {rq /home/pavel/.local/share/rhythmbox/rhythmdb.xml}
);
\set xxx {r /home/pavel/.local/share/rhythmbox/rhythmdb.xml}
This patch is demo of this design - one part is redundant - I'll clean it
in next iteration.
Regards
Pavel
Attachments:
psql-content-commands.patchtext/x-patch; charset=US-ASCII; name=psql-content-commands.patchDownload
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a7789df..dbf3ffa 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -12,6 +12,7 @@
#include <limits.h>
#include <math.h>
#include <signal.h>
+#include <sys/stat.h>
#ifndef WIN32
#include <unistd.h> /* for write() */
#else
@@ -168,6 +169,142 @@ psql_get_variable(const char *varname, bool escape, bool as_ident)
return result;
}
+/*
+ * file-content-fetching callback for flex lexer.
+ */
+char *
+psql_get_file_content(const char *filename, bool escape, bool binary, bool quote)
+{
+ FILE *fd;
+ char *fname = pg_strdup(filename);
+ char *result = NULL;
+
+ expand_tilde(&fname);
+ canonicalize_path(fname);
+
+ fd = fopen(fname, PG_BINARY_R);
+ if (fd)
+ {
+ struct stat fst;
+
+ if (fstat(fileno(fd), &fst) != -1)
+ {
+ if (S_ISREG(fst.st_mode))
+ {
+ if (fst.st_size <= ((int64) 1024) * 1024 * 1024)
+ {
+ size_t size;
+ PQExpBufferData raw_data;
+ char buf[512];
+
+ initPQExpBuffer(&raw_data);
+
+ if (!escape && quote)
+ appendPQExpBufferChar(&raw_data, '\'');
+
+ while ((size = fread(buf, 1, sizeof(buf), fd)) > 0)
+ appendBinaryPQExpBuffer(&raw_data, buf, size);
+
+ if (!ferror(fd) && !(PQExpBufferDataBroken(raw_data)))
+ {
+ if (escape)
+ {
+ if (binary)
+ {
+ unsigned char *escaped_value;
+ size_t escaped_size;
+
+ escaped_value = PQescapeByteaConn(pset.db,
+ (const unsigned char *) raw_data.data, raw_data.len,
+ &escaped_size);
+
+ if (escaped_value != NULL)
+ {
+ if (quote)
+ {
+ PQExpBufferData finalbuf;
+
+ initPQExpBuffer(&finalbuf);
+ appendPQExpBufferChar(&finalbuf, '\'');
+ appendBinaryPQExpBuffer(&finalbuf,
+ (const char *) escaped_value, escaped_size - 1);
+ appendPQExpBufferChar(&finalbuf, '\'');
+ PQfreemem(escaped_value);
+
+ result = finalbuf.data;
+ }
+ else
+ result = (char *) escaped_value;
+ }
+ else
+ psql_error("%s\n", PQerrorMessage(pset.db));
+ }
+ else
+ {
+ /* escape text */
+ if (quote)
+ {
+ result = PQescapeLiteral(pset.db,
+ raw_data.data, raw_data.len);
+ if (result == NULL)
+ psql_error("%s\n", PQerrorMessage(pset.db));
+ }
+ else
+ {
+ int error;
+
+ result = pg_malloc(raw_data.len * 2 + 1);
+ PQescapeStringConn(pset.db, result, raw_data.data, raw_data.len, &error);
+ if (error)
+ {
+ psql_error("%s\n", PQerrorMessage(pset.db));
+ PQfreemem(result);
+ result = NULL;
+ }
+ }
+ }
+ }
+ else
+ {
+ /* returns raw data, without any transformations */
+ if (quote)
+ appendPQExpBufferChar(&raw_data, '\'');
+
+ if (PQExpBufferDataBroken(raw_data))
+ psql_error("out of memory\n");
+ else
+ result = raw_data.data;
+ }
+ }
+ else
+ {
+ if (PQExpBufferDataBroken(raw_data))
+ psql_error("out of memory\n");
+ else
+ psql_error("%s: %s\n", fname, strerror(errno));
+ }
+
+ if (result != raw_data.data)
+ termPQExpBuffer(&raw_data);
+ }
+ else
+ psql_error("%s is too big (greather than 1GB)\n", fname);
+ }
+ else
+ psql_error("%s is not regular file\n", fname);
+ }
+ else
+ psql_error("%s: %s\n", fname, strerror(errno));
+
+ fclose(fd);
+ }
+ else
+ psql_error("%s: %s\n", fname, strerror(errno));
+
+ PQfreemem(fname);
+
+ return result;
+}
/*
* Error reporting for scripts. Errors should look like
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index bdcb58f..6b8ae67 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -19,6 +19,7 @@ extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
extern bool setQFout(const char *fname);
extern char *psql_get_variable(const char *varname, bool escape, bool as_ident);
+extern char *psql_get_file_content(const char *filename, bool escape, bool binary, bool quote);
extern void psql_error(const char *fmt,...) pg_attribute_printf(1, 2);
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 37dfa4d..bf1d89f 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -20,6 +20,7 @@
/* callback functions for our flex lexer */
const PsqlScanCallbacks psqlscan_callbacks = {
psql_get_variable,
+ psql_get_file_content,
psql_error
};
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index 86832a8..1b1080c 100644
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -19,6 +19,7 @@
#include "postgres_fe.h"
#include "psqlscanslash.h"
+#include "common.h"
#include "libpq-fe.h"
}
@@ -46,6 +47,7 @@ static enum slash_option_type option_type;
static char *option_quote;
static int unquoted_option_chars;
static int backtick_start_offset;
+static int curlybracket_start_offset;
/* Return values from yylex() */
@@ -54,6 +56,7 @@ static int backtick_start_offset;
static void evaluate_backtick(PsqlScanState state);
+static void evaluate_curlybrackets(PsqlScanState state);
#define ECHO psqlscan_emit(cur_state, yytext, yyleng)
@@ -98,6 +101,7 @@ extern void slash_yyset_column(int column_no, yyscan_t yyscanner);
%x xslashdquote
%x xslashwholeline
%x xslashend
+%x xslashcurlybrackets
/*
* Assorted character class definitions that should match psqlscan.l.
@@ -228,6 +232,14 @@ other .
BEGIN(xslashdquote);
}
+"{" {
+ curlybracket_start_offset = output_buf->len;
+ ECHO;
+ *option_quote = '{';
+ unquoted_option_chars = 0;
+ BEGIN(xslashcurlybrackets);
+ }
+
:{variable_char}+ {
/* Possible psql variable substitution */
if (option_type == OT_NO_EVAL ||
@@ -362,6 +374,21 @@ other .
}
+<xslashcurlybrackets>{
+ /* curly brackets: copy everything until next right curly bracket */
+
+"}" {
+ /* In NO_EVAL mode, don't evaluate the command */
+ ECHO;
+ if (option_type != OT_NO_EVAL)
+ evaluate_curlybrackets(cur_state);
+ BEGIN(xslasharg);
+ }
+
+{other}|\n { ECHO; }
+
+}
+
<xslashdquote>{
/* double-quoted text: copy verbatim, including the double quotes */
@@ -580,6 +607,7 @@ psql_scan_slash_option(PsqlScanState state,
case xslashquote:
case xslashbackquote:
case xslashdquote:
+ case xslashcurlybrackets:
/* must have hit EOL inside quotes */
state->callbacks->write_error("unterminated quoted string\n");
termPQExpBuffer(&mybuf);
@@ -755,3 +783,107 @@ evaluate_backtick(PsqlScanState state)
termPQExpBuffer(&cmd_output);
}
+
+static void
+evaluate_curlybrackets(PsqlScanState state)
+{
+ PQExpBuffer output_buf = state->output_buf;
+ char *cmdline = output_buf->data + curlybracket_start_offset;
+ char *cmdlinebuf;
+ char *endptr;
+ bool read_file = false;
+ bool binary = false;
+ bool escape = false;
+ bool quote = false;
+
+ cmdlinebuf = pg_strdup(output_buf->data + curlybracket_start_offset);
+
+ /* skip initial left bracket */
+ cmdline = cmdlinebuf + 1;
+
+ /* skip initial spaces */
+ while (*cmdline == ' ')
+ cmdline++;
+
+ /* we should to remove final right bracket and trim spaces */
+ endptr = cmdline + strlen(cmdline);
+ *(--endptr) = '\0';
+ endptr--;
+ while (*endptr == ' ' && endptr > cmdline)
+ endptr--;
+
+ endptr[1] = '\0';
+
+ if (*cmdline != '\0')
+ {
+ char *cptr = cmdline;
+ int clen;
+ char cname[10];
+
+ /* find a end of statement */
+ while (*cptr != ' ' && *cptr != '\0')
+ cptr++;
+
+ clen = cptr - cmdline;
+ if (clen < 10)
+ {
+ strncpy(cname, cmdline, clen);
+ cname[clen] = '\0';
+
+ if (strcmp(cname, "r") == 0)
+ {
+ read_file = true;
+ }
+ else if (strcmp(cname, "rb") == 0)
+ {
+ read_file = true;
+ binary = true;
+ escape = true;
+ }
+ else if (strcmp(cname, "rq") == 0)
+ {
+ read_file = true;
+ escape = true;
+ quote = true;
+ }
+ else if (strcmp(cname, "rbq") == 0)
+ {
+ read_file = true;
+ escape = true;
+ binary = true;
+ quote = true;
+ }
+ else
+ state->callbacks->write_error("%s: unsupported psql content command", cname);
+
+ if (read_file)
+ {
+ /* skip initial spaces */
+ while (*cptr == ' ')
+ cptr++;
+
+ if (cptr != '\0')
+ {
+ char *content = psql_get_file_content(cptr, escape, binary, quote);
+
+ if (content != NULL)
+ {
+ output_buf->len = curlybracket_start_offset;
+ output_buf->data[output_buf->len] = '\0';
+
+ appendPQExpBufferStr(output_buf, content);
+ PQfreemem(content);
+ }
+ }
+ else
+ state->callbacks->write_error("%s: missing expected file name", cname);
+ }
+ }
+ else
+ state->callbacks->write_error("%s: psql content command is too long", cmdline);
+ }
+ else
+ state->callbacks->write_error("empty psql content command");
+
+ PQfreemem(cmdlinebuf);
+}
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b556c00..b8d6c83 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -58,7 +58,7 @@ extern char *filename_completion_function();
#endif
/* word break characters */
-#define WORD_BREAKS "\t\n@$><=;|&{() "
+#define WORD_BREAKS "\t\n@$><=;|&() "
/*
* Since readline doesn't let us pass any state through to the tab completion
@@ -1343,6 +1343,11 @@ psql_completion(const char *text, int start, int end)
"\\timing", "\\unset", "\\x", "\\w", "\\watch", "\\z", "\\!", NULL
};
+ /* psql's content commands. */
+ static const char *const content_commands[] = {
+ "{r", "{rq", "{rb", "{rbq", NULL
+ };
+
(void) end; /* "end" is not used */
#ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER
@@ -1368,6 +1373,10 @@ psql_completion(const char *text, int start, int end)
if (text[0] == '\\')
COMPLETE_WITH_LIST_CS(backslash_commands);
+ else if (text[0] == '{')
+ {
+ COMPLETE_WITH_LIST_CS(content_commands);
+ }
/* If current word is a variable interpolation, handle that case */
else if (text[0] == ':' && text[1] != ':')
{
@@ -3222,6 +3231,11 @@ psql_completion(const char *text, int start, int end)
completion_charp = "\\";
matches = completion_matches(text, complete_from_files);
}
+ else if (TailMatchesCS1("\{r|\{rb|\{rq|\{rbq"))
+ {
+ completion_charp = "{";
+ matches = completion_matches(text, complete_from_files);
+ }
/*
* Finally, we look through the list of "things", such as TABLE, INDEX and
diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l
index 55067b4..1bfbf12 100644
--- a/src/fe_utils/psqlscan.l
+++ b/src/fe_utils/psqlscan.l
@@ -47,6 +47,8 @@
*/
typedef int YYSTYPE;
+static int curlybracket_start_offset;
+
/*
* Set the type of yyextra; we use it as a pointer back to the containing
* PsqlScanState.
@@ -71,6 +73,8 @@ typedef int YYSTYPE;
extern int psql_yyget_column(yyscan_t yyscanner);
extern void psql_yyset_column(int column_no, yyscan_t yyscanner);
+static void evaluate_curlybrackets(PsqlScanState state, int start_offset);
+
%}
%option reentrant
@@ -115,6 +119,7 @@ extern void psql_yyset_column(int column_no, yyscan_t yyscanner);
* <xuiend> end of a quoted identifier with Unicode escapes, UESCAPE can follow
* <xus> quoted string with Unicode escapes
* <xusend> end of a quoted string with Unicode escapes, UESCAPE can follow
+ * <xcb> curly bracket string
*
* Note: we intentionally don't mimic the backend's <xeu> state; we have
* no need to distinguish it from <xe> state, and no good way to get out
@@ -133,6 +138,7 @@ extern void psql_yyset_column(int column_no, yyscan_t yyscanner);
%x xuiend
%x xus
%x xusend
+%x xcb
/*
* In order to make the world safe for Windows and Mac clients as well as
@@ -673,6 +679,22 @@ other .
}
}
+"{" {
+ curlybracket_start_offset = cur_state->output_buf->len;
+ BEGIN(xcb);
+ ECHO;
+ }
+
+<xcb>"}" {
+ ECHO;
+ evaluate_curlybrackets(cur_state, curlybracket_start_offset);
+ BEGIN(INITIAL);
+ }
+
+<xcb>. {
+ ECHO;
+ }
+
/*
* psql-specific rules to handle backslash commands and variable
* substitution. We want these before {self}, also.
@@ -1426,3 +1448,114 @@ psqlscan_escape_variable(PsqlScanState state, const char *txt, int len,
psqlscan_emit(state, txt, len);
}
}
+
+/*
+ * evaluate a content command in curly brackets
+ */
+static void
+evaluate_curlybrackets(PsqlScanState state, int start_offset)
+{
+ PQExpBuffer output_buf = state->output_buf;
+ char *cmdline;
+ char *cmdlinebuf;
+ char *endptr;
+ bool read_file = false;
+ bool binary = false;
+ bool escape = false;
+ bool quote = false;
+
+ cmdlinebuf = pg_strdup(output_buf->data + start_offset);
+
+ /* skip initial left bracket */
+ cmdline = cmdlinebuf + 1;
+
+ /* skip initial spaces */
+ while (*cmdline == ' ')
+ cmdline++;
+
+ /* we should to remove final right bracket, and trim spaces */
+ endptr = cmdline + strlen(cmdline);
+ *(--endptr) = '\0';
+ endptr--;
+ while (*endptr == ' ' && endptr > cmdline)
+ endptr--;
+
+ endptr[1] = '\0';
+
+ if (*cmdline != '\0')
+ {
+ char *cptr = cmdline;
+ int clen;
+ char cname[10];
+
+ /* find a end of statement */
+ while (*cptr != ' ' && *cptr != '\0')
+ cptr++;
+
+ clen = cptr - cmdline;
+ if (clen < 10)
+ {
+ strncpy(cname, cmdline, clen);
+ cname[clen] = '\0';
+
+ if (strcmp(cname, "r") == 0)
+ {
+ read_file = true;
+ }
+ else if (strcmp(cname, "rb") == 0)
+ {
+ read_file = true;
+ binary = true;
+ escape = true;
+ }
+ else if (strcmp(cname, "rq") == 0)
+ {
+ read_file = true;
+ escape = true;
+ quote = true;
+ }
+ else if (strcmp(cname, "rbq") == 0)
+ {
+ read_file = true;
+ escape = true;
+ binary = true;
+ quote = true;
+ }
+ else
+ state->callbacks->write_error("%s: unsupported psql content command\n", cname);
+
+ if (read_file)
+ {
+ /* skip initial spaces */
+ while (*cptr == ' ')
+ cptr++;
+
+ if (cptr != '\0')
+ {
+ if (state->callbacks->get_file_content)
+ {
+ char *content = state->callbacks->get_file_content(cptr,
+ escape, binary, quote);
+
+ if (content != NULL)
+ {
+ output_buf->len = start_offset;
+ output_buf->data[output_buf->len] = '\0';
+
+ appendPQExpBufferStr(output_buf, content);
+ PQfreemem(content);
+ }
+ }
+ }
+ else
+ state->callbacks->write_error("%s: missing expected file name\n", cname);
+ }
+ }
+ else
+ state->callbacks->write_error("%s: psql content command is too long\n", cmdline);
+ }
+ else
+ state->callbacks->write_error("empty psql content command\n");
+
+ PQfreemem(cmdlinebuf);
+}
diff --git a/src/include/fe_utils/psqlscan.h b/src/include/fe_utils/psqlscan.h
index 1f10ecc..93dfa5e 100644
--- a/src/include/fe_utils/psqlscan.h
+++ b/src/include/fe_utils/psqlscan.h
@@ -54,6 +54,8 @@ typedef struct PsqlScanCallbacks
/* Fetch value of a variable, as a pfree'able string; NULL if unknown */
/* This pointer can be NULL if no variable substitution is wanted */
char *(*get_variable) (const char *varname, bool escape, bool as_ident);
+ /* Fetch content of a file, as a pfree'able string */
+ char *(*get_file_content) (const char *filename, bool escape, bool binary, bool quote);
/* Print an error message someplace appropriate */
/* (very old gcc versions don't support attributes on function pointers) */
#if defined(__GNUC__) && __GNUC__ < 4
On Sun, Nov 13, 2016 at 1:46 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
r - read file without any modification
rq - read file, escape as literal and use outer quotes
rb - read binary file - transform to hex code string
rbq - read binary file, transform to hex code string and use outer quotesUsage:
I am not asking for this feature now, but do you see any barriers to later
adding x/xq/xb/xbq equivalents for executing a shell command?
2016-11-15 16:41 GMT+01:00 Corey Huinker <corey.huinker@gmail.com>:
On Sun, Nov 13, 2016 at 1:46 PM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:r - read file without any modification
rq - read file, escape as literal and use outer quotes
rb - read binary file - transform to hex code string
rbq - read binary file, transform to hex code string and use outer quotesUsage:
I am not asking for this feature now, but do you see any barriers to later
adding x/xq/xb/xbq equivalents for executing a shell command?
any other new commands can be appended simply - this is really generic
Regards
Pavel
Hi
2016-11-13 19:46 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
I am sending a initial implementation of psql content commands. This
design is reaction to Tom's objections against psql file ref variables.
This design is more cleaner, more explicit and more practical - import can
be in one step.Now supported commands are:
r - read file without any modification
rq - read file, escape as literal and use outer quotes
rb - read binary file - transform to hex code string
rbq - read binary file, transform to hex code string and use outer quotesUsage:
create table testt(a xml);
insert into test values( {rq /home/pavel/.local/share/rhythmbox/rhythmdb.xml}
);
\set xxx {r /home/pavel/.local/share/rhythmbox/rhythmdb.xml}This patch is demo of this design - one part is redundant - I'll clean it
in next iteration.Regards
here is cleaned patch
* the behave is much more psqlish - show error only when the command is
exactly detected - errors are related to processed files only - the behave
is similar to psql variables - we doesn't raise a error, when the variable
doesn't exists.
* no more duplicate code
* some basic doc
* some basic regress tests
Regards
Pavel
Show quoted text
Pavel
Attachments:
psql-content-commands-01.patchtext/x-patch; charset=US-ASCII; name=psql-content-commands-01.patchDownload
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index 2410bee..141df6f 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2991,6 +2991,57 @@ testdb=> <userinput>\setenv LESS -imx4F</userinput>
</refsect3>
</refsect2>
+ <refsect2 id="APP-PSQL-content-commands">
+ <title>Content Commands</title>
+
+ <para>
+ These commands inject some content to processed query.
+
+<programlisting>
+testdb=> <userinput>CREATE TABLE my_table(id int, image bytea);</userinput>
+testdb=> <userinput>INSERT INTO my_table VALUES(1, {rbq ~/avatar.gif } ); </userinput>
+</programlisting>
+
+ <variablelist>
+ <varlistentry>
+ <term><literal>{r <replaceable class="parameter">filename</>}</literal></term>
+ <listitem>
+ <para>
+ Returns content of file.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>{rb <replaceable class="parameter">filename</>}</literal></term>
+ <listitem>
+ <para>
+ Returns content of binary file encoded to hex code.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>{rq <replaceable class="parameter">filename</>}</literal></term>
+ <listitem>
+ <para>
+ Returns content of file, escaped and quoted as string literal
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>{rb <replaceable class="parameter">filename</>}</literal></term>
+ <listitem>
+ <para>
+ Returns content of binary file encoded to hex code and quoted.
+ </para>
+ </listitem>
+ </varlistentry>
+ </variablelist>
+ </para>
+ </refsect2>
+
<refsect2>
<title>Advanced Features</title>
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index a7fdd8a..43f699f 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -462,6 +462,7 @@ static void setalarm(int seconds);
/* callback functions for our flex lexer */
static const PsqlScanCallbacks pgbench_callbacks = {
NULL, /* don't need get_variable functionality */
+ NULL, /* don't need eval_content_command functionality */
pgbench_error
};
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a7789df..de7fecd 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -12,6 +12,7 @@
#include <limits.h>
#include <math.h>
#include <signal.h>
+#include <sys/stat.h>
#ifndef WIN32
#include <unistd.h> /* for write() */
#else
@@ -168,6 +169,211 @@ psql_get_variable(const char *varname, bool escape, bool as_ident)
return result;
}
+#define READFILE_NONE (0)
+#define READFILE_NOESCAPE (1 << 0)
+#define READFILE_ESCAPE_TEXT (1 << 1)
+#define READFILE_ESCAPE_BINARY (1 << 2)
+#define READFILE_QUOTE (1 << 3)
+
+/*
+ * file-content-fetching callback for read file content commands.
+ */
+static char *
+get_file_content(char *fname, int mode)
+{
+ FILE *fd;
+ char *result = NULL;
+
+ expand_tilde(&fname);
+ canonicalize_path(fname);
+
+ fd = fopen(fname, PG_BINARY_R);
+ if (fd)
+ {
+ struct stat fst;
+
+ if (fstat(fileno(fd), &fst) != -1)
+ {
+ if (S_ISREG(fst.st_mode))
+ {
+ if (fst.st_size <= ((int64) 1024) * 1024 * 1024)
+ {
+ size_t size;
+ PQExpBufferData raw_data;
+ char buf[512];
+
+ initPQExpBuffer(&raw_data);
+
+ while ((size = fread(buf, 1, sizeof(buf), fd)) > 0)
+ appendBinaryPQExpBuffer(&raw_data, buf, size);
+
+ if (!ferror(fd) && !(PQExpBufferDataBroken(raw_data)))
+ {
+ if (!(mode & READFILE_NOESCAPE))
+ {
+ if (mode & READFILE_ESCAPE_BINARY)
+ {
+ unsigned char *escaped_value;
+ size_t escaped_size;
+
+ escaped_value = PQescapeByteaConn(pset.db,
+ (const unsigned char *) raw_data.data, raw_data.len,
+ &escaped_size);
+
+ if (escaped_value)
+ {
+ if (mode & READFILE_QUOTE)
+ {
+ PQExpBufferData resultbuf;
+
+ initPQExpBuffer(&resultbuf);
+
+ appendPQExpBufferChar(&resultbuf, '\'');
+ appendBinaryPQExpBuffer(&resultbuf,
+ (const char *) escaped_value, escaped_size - 1);
+ appendPQExpBufferChar(&resultbuf, '\'');
+ PQfreemem(escaped_value);
+
+ if (PQExpBufferDataBroken(resultbuf))
+ psql_error("out of memory\n");
+ else
+ result = resultbuf.data;
+ }
+ else
+ result = (char *) escaped_value;
+ }
+ else
+ psql_error("%s\n", PQerrorMessage(pset.db));
+ }
+ else
+ {
+ /*
+ * should be READFILE_ESCAPE_TEXT & READFILE_QUOTE
+ * escaping of text file has not sense without quoting
+ */
+ result = PQescapeLiteral(pset.db,
+ raw_data.data, raw_data.len);
+ if (result == NULL)
+ psql_error("%s\n", PQerrorMessage(pset.db));
+ }
+ }
+ else
+ result = raw_data.data;
+ }
+ else
+ {
+ if (PQExpBufferDataBroken(raw_data))
+ psql_error("out of memory\n");
+ else
+ psql_error("%s: %s\n", fname, strerror(errno));
+ }
+
+ if (result != raw_data.data)
+ termPQExpBuffer(&raw_data);
+ }
+ else
+ psql_error("%s is too big (greather than 1GB)\n", fname);
+ }
+ else
+ psql_error("%s is not regular file\n", fname);
+ }
+ else
+ psql_error("%s: %s\n", fname, strerror(errno));
+
+ fclose(fd);
+ }
+ else
+ psql_error("%s: %s\n", fname, strerror(errno));
+
+ return result;
+}
+
+/*
+ * a implementation of psql content commands - curly bracket
+ * commands.
+ *
+ * It does nothing, when command is not detected - like evaluation of
+ * psql variables. Usually the SQL statement fails on syntax error - the
+ * behave is same like evaluation of psql variable.
+ */
+void
+psql_eval_content_command(PQExpBuffer output_buf, int start_offset)
+{
+ char *content_command;
+ char *cptr;
+ int readfile_mode = READFILE_NONE;
+
+ /* do updatable copy of input buffer, input buffer should not be changed */
+ content_command = pg_strdup(output_buf->data + start_offset);
+
+ /* skip initial left bracket */
+ cptr = content_command + 1;
+
+ /* skip initial spaces */
+ while (*cptr == ' ')
+ cptr++;
+
+ if (*cptr != '\0')
+ {
+ char *cname = cptr;
+
+ /* find a end of statement */
+ while (*cptr != ' ' && *cptr != '\0')
+ cptr++;
+
+ /* the space is required in supported commands */
+ if (*cptr != '\0')
+ {
+ *cptr++ = '\0';
+
+ if (strcmp(cname, "r") == 0)
+ readfile_mode = READFILE_NOESCAPE;
+ else if (strcmp(cname, "rb") == 0)
+ readfile_mode = READFILE_ESCAPE_BINARY;
+ else if (strcmp(cname, "rq") == 0)
+ readfile_mode = READFILE_ESCAPE_TEXT | READFILE_QUOTE;
+ else if (strcmp(cname, "rbq") == 0)
+ readfile_mode = READFILE_ESCAPE_BINARY | READFILE_QUOTE;
+
+ if (readfile_mode != READFILE_NONE)
+ {
+ /* parse file name from the rest of content command */
+
+ /* skip initial spaces */
+ while (*cptr == ' ')
+ cptr++;
+
+ if (cptr != '\0')
+ {
+ char *eptr = cptr + strlen(cptr) - 1 - 1; /* drop right bracket */
+
+ while (*eptr == ' ' && eptr > cptr)
+ eptr--;
+
+ eptr[1] = '\0';
+
+ if (cptr != '\0')
+ {
+ char *result = get_file_content(cptr, readfile_mode);
+
+ if (result != NULL)
+ {
+ /* remove content command text from output buffer */
+ output_buf->len = start_offset;
+ output_buf->data[output_buf->len] = '\0';
+
+ /* append the result of content command */
+ appendPQExpBufferStr(output_buf, result);
+ PQfreemem(result);
+ }
+ }
+ }
+ }
+ }
+ }
+
+ PQfreemem(content_command);
+}
/*
* Error reporting for scripts. Errors should look like
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index bdcb58f..18a9c86 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -11,6 +11,7 @@
#include <setjmp.h>
#include "libpq-fe.h"
+#include "pqexpbuffer.h"
#include "fe_utils/print.h"
#define atooid(x) ((Oid) strtoul((x), NULL, 10))
@@ -19,6 +20,7 @@ extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
extern bool setQFout(const char *fname);
extern char *psql_get_variable(const char *varname, bool escape, bool as_ident);
+extern void psql_eval_content_command(PQExpBuffer output_buf, int start_offset);
extern void psql_error(const char *fmt,...) pg_attribute_printf(1, 2);
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 37dfa4d..69c697b 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -20,6 +20,7 @@
/* callback functions for our flex lexer */
const PsqlScanCallbacks psqlscan_callbacks = {
psql_get_variable,
+ psql_eval_content_command,
psql_error
};
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index 86832a8..06b34a4 100644
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -19,6 +19,7 @@
#include "postgres_fe.h"
#include "psqlscanslash.h"
+#include "common.h"
#include "libpq-fe.h"
}
@@ -46,6 +47,7 @@ static enum slash_option_type option_type;
static char *option_quote;
static int unquoted_option_chars;
static int backtick_start_offset;
+static int curlybracket_start_offset;
/* Return values from yylex() */
@@ -98,6 +100,7 @@ extern void slash_yyset_column(int column_no, yyscan_t yyscanner);
%x xslashdquote
%x xslashwholeline
%x xslashend
+%x xslashcurlybrackets
/*
* Assorted character class definitions that should match psqlscan.l.
@@ -228,6 +231,14 @@ other .
BEGIN(xslashdquote);
}
+"{" {
+ curlybracket_start_offset = output_buf->len;
+ ECHO;
+ *option_quote = '{';
+ unquoted_option_chars = 0;
+ BEGIN(xslashcurlybrackets);
+ }
+
:{variable_char}+ {
/* Possible psql variable substitution */
if (option_type == OT_NO_EVAL ||
@@ -362,6 +373,25 @@ other .
}
+<xslashcurlybrackets>{
+ /* curly brackets: copy everything until next right curly bracket */
+
+"}" {
+ /* In NO_EVAL mode, don't evaluate the command */
+ ECHO;
+ if (option_type != OT_NO_EVAL)
+ {
+ if (cur_state->callbacks->eval_content_command)
+ cur_state->callbacks->eval_content_command(cur_state->output_buf,
+ curlybracket_start_offset);
+ }
+ BEGIN(xslasharg);
+ }
+
+{other}|\n { ECHO; }
+
+}
+
<xslashdquote>{
/* double-quoted text: copy verbatim, including the double quotes */
@@ -580,6 +610,7 @@ psql_scan_slash_option(PsqlScanState state,
case xslashquote:
case xslashbackquote:
case xslashdquote:
+ case xslashcurlybrackets:
/* must have hit EOL inside quotes */
state->callbacks->write_error("unterminated quoted string\n");
termPQExpBuffer(&mybuf);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b556c00..b8d6c83 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -58,7 +58,7 @@ extern char *filename_completion_function();
#endif
/* word break characters */
-#define WORD_BREAKS "\t\n@$><=;|&{() "
+#define WORD_BREAKS "\t\n@$><=;|&() "
/*
* Since readline doesn't let us pass any state through to the tab completion
@@ -1343,6 +1343,11 @@ psql_completion(const char *text, int start, int end)
"\\timing", "\\unset", "\\x", "\\w", "\\watch", "\\z", "\\!", NULL
};
+ /* psql's content commands. */
+ static const char *const content_commands[] = {
+ "{r", "{rq", "{rb", "{rbq", NULL
+ };
+
(void) end; /* "end" is not used */
#ifdef HAVE_RL_COMPLETION_APPEND_CHARACTER
@@ -1368,6 +1373,10 @@ psql_completion(const char *text, int start, int end)
if (text[0] == '\\')
COMPLETE_WITH_LIST_CS(backslash_commands);
+ else if (text[0] == '{')
+ {
+ COMPLETE_WITH_LIST_CS(content_commands);
+ }
/* If current word is a variable interpolation, handle that case */
else if (text[0] == ':' && text[1] != ':')
{
@@ -3222,6 +3231,11 @@ psql_completion(const char *text, int start, int end)
completion_charp = "\\";
matches = completion_matches(text, complete_from_files);
}
+ else if (TailMatchesCS1("\{r|\{rb|\{rq|\{rbq"))
+ {
+ completion_charp = "{";
+ matches = completion_matches(text, complete_from_files);
+ }
/*
* Finally, we look through the list of "things", such as TABLE, INDEX and
diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l
index 55067b4..49d6dae 100644
--- a/src/fe_utils/psqlscan.l
+++ b/src/fe_utils/psqlscan.l
@@ -47,6 +47,8 @@
*/
typedef int YYSTYPE;
+static int curlybracket_start_offset;
+
/*
* Set the type of yyextra; we use it as a pointer back to the containing
* PsqlScanState.
@@ -115,6 +117,7 @@ extern void psql_yyset_column(int column_no, yyscan_t yyscanner);
* <xuiend> end of a quoted identifier with Unicode escapes, UESCAPE can follow
* <xus> quoted string with Unicode escapes
* <xusend> end of a quoted string with Unicode escapes, UESCAPE can follow
+ * <xcb> curly bracket string
*
* Note: we intentionally don't mimic the backend's <xeu> state; we have
* no need to distinguish it from <xe> state, and no good way to get out
@@ -133,6 +136,7 @@ extern void psql_yyset_column(int column_no, yyscan_t yyscanner);
%x xuiend
%x xus
%x xusend
+%x xcb
/*
* In order to make the world safe for Windows and Mac clients as well as
@@ -673,6 +677,24 @@ other .
}
}
+"{" {
+ curlybracket_start_offset = cur_state->output_buf->len;
+ BEGIN(xcb);
+ ECHO;
+ }
+
+<xcb>"}" {
+ BEGIN(INITIAL);
+ ECHO;
+ if (cur_state->callbacks->eval_content_command)
+ cur_state->callbacks->eval_content_command(cur_state->output_buf,
+ curlybracket_start_offset);
+ }
+
+<xcb>.|\n {
+ ECHO;
+ }
+
/*
* psql-specific rules to handle backslash commands and variable
* substitution. We want these before {self}, also.
diff --git a/src/include/fe_utils/psqlscan.h b/src/include/fe_utils/psqlscan.h
index 1f10ecc..a70a40e 100644
--- a/src/include/fe_utils/psqlscan.h
+++ b/src/include/fe_utils/psqlscan.h
@@ -54,6 +54,8 @@ typedef struct PsqlScanCallbacks
/* Fetch value of a variable, as a pfree'able string; NULL if unknown */
/* This pointer can be NULL if no variable substitution is wanted */
char *(*get_variable) (const char *varname, bool escape, bool as_ident);
+ /* eval psql content command */
+ void (*eval_content_command) (PQExpBuffer output_buf, int start_offset);
/* Print an error message someplace appropriate */
/* (very old gcc versions don't support attributes on function pointers) */
#if defined(__GNUC__) && __GNUC__ < 4
diff --git a/src/test/regress/input/misc.source b/src/test/regress/input/misc.source
index dd2d1b2..bca752f 100644
--- a/src/test/regress/input/misc.source
+++ b/src/test/regress/input/misc.source
@@ -273,3 +273,21 @@ drop table oldstyle_test;
--
-- rewrite rules
--
+
+--
+-- psql content commands
+--
+CREATE TABLE test_setref(a text, b bytea);
+
+-- use two different ways for import data - result should be same
+\lo_import @abs_builddir@/data/hash.data
+\set lo_oid :LASTOID
+INSERT INTO test_setref (b) VALUES(lo_get(:lo_oid));
+\lo_unlink :lo_oid
+SELECT md5(b) FROM test_setref;
+TRUNCATE test_setref;
+
+INSERT INTO test_setref(a,b) VALUES({rq @abs_builddir@/data/hash.data}, {rbq @abs_builddir@/data/hash.data});
+SELECT md5(a), md5(b) FROM test_setref;
+
+DROP TABLE test_setref;
diff --git a/src/test/regress/output/misc.source b/src/test/regress/output/misc.source
index 574ef0d..0c8eb97 100644
--- a/src/test/regress/output/misc.source
+++ b/src/test/regress/output/misc.source
@@ -708,3 +708,27 @@ drop table oldstyle_test;
--
-- rewrite rules
--
+--
+-- psql content commands
+--
+CREATE TABLE test_setref(a text, b bytea);
+-- use two different ways for import data - result should be same
+\lo_import @abs_builddir@/data/hash.data
+\set lo_oid :LASTOID
+INSERT INTO test_setref (b) VALUES(lo_get(:lo_oid));
+\lo_unlink :lo_oid
+SELECT md5(b) FROM test_setref;
+ md5
+----------------------------------
+ e446fe6ea5a347e69670633412c7f8cb
+(1 row)
+
+TRUNCATE test_setref;
+INSERT INTO test_setref(a,b) VALUES({rq @abs_builddir@/data/hash.data}, {rbq @abs_builddir@/data/hash.data});
+SELECT md5(a), md5(b) FROM test_setref;
+ md5 | md5
+----------------------------------+----------------------------------
+ e446fe6ea5a347e69670633412c7f8cb | e446fe6ea5a347e69670633412c7f8cb
+(1 row)
+
+DROP TABLE test_setref;
Corey Huinker wrote:
I am not asking for this feature now, but do you see any barriers to later
adding x/xq/xb/xbq equivalents for executing a shell command?
For text contents, we already have this (pasted right from the doc):
testdb=> \set content `cat my_file.txt`
testdb=> INSERT INTO my_table VALUES (:'content');
Maybe we might look at why it doesn't work for binary string and fix that?
I can think of three reasons:
- psql use C-style '\0' terminated string implying no nul byte in variables.
That could potentially be fixed by tweaking the data structures and
accessors.
- the backtick operator trims ending '\n' from the result of the command
(like the shell), but we could add a variant that doesn't have this behavior.
- we don't have "interpolate as binary", an operator that will
essentially apply PQescapeByteaConn rather than PQescapeStringConn.
For example, I'd suggest this syntax:
-- slurp a file into a variable, with no translation whatsoever
\set content ``cat my_binary_file``
-- interpolate as binary (backquotes instead of quotes)
INSERT INTO my_table VALUES(:`content`);
If we had something like this, would we still need filerefs? I feel like
the point of filerefs is mainly to work around lack of support for
binary in variables, but maybe supporting the latter directly would
be an easier change to accept.
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-11-15 17:39 GMT+01:00 Daniel Verite <daniel@manitou-mail.org>:
Corey Huinker wrote:
I am not asking for this feature now, but do you see any barriers to
later
adding x/xq/xb/xbq equivalents for executing a shell command?
For text contents, we already have this (pasted right from the doc):
testdb=> \set content `cat my_file.txt`
testdb=> INSERT INTO my_table VALUES (:'content');Maybe we might look at why it doesn't work for binary string and fix that?
I can think of three reasons:
- psql use C-style '\0' terminated string implying no nul byte in
variables.
That could potentially be fixed by tweaking the data structures and
accessors.- the backtick operator trims ending '\n' from the result of the command
(like the shell), but we could add a variant that doesn't have this
behavior.- we don't have "interpolate as binary", an operator that will
essentially apply PQescapeByteaConn rather than PQescapeStringConn.For example, I'd suggest this syntax:
-- slurp a file into a variable, with no translation whatsoever
\set content ``cat my_binary_file``-- interpolate as binary (backquotes instead of quotes)
INSERT INTO my_table VALUES(:`content`);If we had something like this, would we still need filerefs? I feel like
the point of filerefs is mainly to work around lack of support for
binary in variables, but maybe supporting the latter directly would
be an easier change to accept.
I leaved a concept of fileref - see Tom's objection. I know, so I can hack
``, but I would not do it. It is used for shell (system) calls, and these
functionality can depends on used os. The proposed content commands can be
extended more, and you are doesn't depends on o.s. Another issue of your
proposal is hard support for tab complete (file path).
Please, try to test my last patch.
Regards
Pavel
Show quoted text
Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite
On Tue, Nov 15, 2016 at 11:48 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
For text contents, we already have this (pasted right from the doc):
testdb=> \set content `cat my_file.txt`
testdb=> INSERT INTO my_table VALUES (:'content');Maybe we might look at why it doesn't work for binary string and fix that?
I can think of three reasons:
- psql use C-style '\0' terminated string implying no nul byte in
variables.
That could potentially be fixed by tweaking the data structures and
accessors.- the backtick operator trims ending '\n' from the result of the command
(like the shell), but we could add a variant that doesn't have this
behavior.- we don't have "interpolate as binary", an operator that will
essentially apply PQescapeByteaConn rather than PQescapeStringConn.For example, I'd suggest this syntax:
-- slurp a file into a variable, with no translation whatsoever
\set content ``cat my_binary_file``-- interpolate as binary (backquotes instead of quotes)
INSERT INTO my_table VALUES(:`content`);If we had something like this, would we still need filerefs? I feel like
the point of filerefs is mainly to work around lack of support for
binary in variables, but maybe supporting the latter directly would
be an easier change to accept.I leaved a concept of fileref - see Tom's objection. I know, so I can hack
``, but I would not do it. It is used for shell (system) calls, and these
functionality can depends on used os. The proposed content commands can be
extended more, and you are doesn't depends on o.s. Another issue of your
proposal is hard support for tab complete (file path).
On the other hand, it seems like you've invented a whole new system of
escaping and interpolation that is completely disconnected from the
one we already have. psql is already full of features that nobody
knows about identified by incomprehensible character combinations.
Daniel's suggestion would at least be more similar to what already
exists.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-11-16 16:59 GMT+01:00 Robert Haas <robertmhaas@gmail.com>:
On Tue, Nov 15, 2016 at 11:48 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:For text contents, we already have this (pasted right from the doc):
testdb=> \set content `cat my_file.txt`
testdb=> INSERT INTO my_table VALUES (:'content');Maybe we might look at why it doesn't work for binary string and fix
that?
I can think of three reasons:
- psql use C-style '\0' terminated string implying no nul byte in
variables.
That could potentially be fixed by tweaking the data structures and
accessors.- the backtick operator trims ending '\n' from the result of the command
(like the shell), but we could add a variant that doesn't have this
behavior.- we don't have "interpolate as binary", an operator that will
essentially apply PQescapeByteaConn rather than PQescapeStringConn.For example, I'd suggest this syntax:
-- slurp a file into a variable, with no translation whatsoever
\set content ``cat my_binary_file``-- interpolate as binary (backquotes instead of quotes)
INSERT INTO my_table VALUES(:`content`);If we had something like this, would we still need filerefs? I feel like
the point of filerefs is mainly to work around lack of support for
binary in variables, but maybe supporting the latter directly would
be an easier change to accept.I leaved a concept of fileref - see Tom's objection. I know, so I can
hack
``, but I would not do it. It is used for shell (system) calls, and these
functionality can depends on used os. The proposed content commands canbe
extended more, and you are doesn't depends on o.s. Another issue of your
proposal is hard support for tab complete (file path).On the other hand, it seems like you've invented a whole new system of
escaping and interpolation that is completely disconnected from the
one we already have. psql is already full of features that nobody
knows about identified by incomprehensible character combinations.
Daniel's suggestion would at least be more similar to what already
exists.
The Daniel's proposal has important issues:
1. you need to store and hold the content in memory
2. you cannot use tab complete - without it this feature lost a usability
3. you have to do two steps
4. Depends on o.s.
Regards
Pavel
Show quoted text
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Pavel Stehule <pavel.stehule@gmail.com> writes:
The Daniel's proposal has important issues:
1. you need to store and hold the content in memory
2. you cannot use tab complete - without it this feature lost a usability
3. you have to do two steps
4. Depends on o.s.
I think you're putting *way* too much emphasis on tab completion of the
filename. That's a nice-to-have, but it should not cause us to contort
the feature design to get it.
I'm not especially impressed by objections 3 and 4, either. Point #1
has some merit, but since the wire protocol is going to limit us to
circa 1GB anyway, it doesn't seem fatal.
What I like about Daniel's proposal is that because it adds two separate
new behaviors, it can be used for more things than just "interpolate a
file". What I don't like is that we're still stuck in the land of textual
interpolation into query strings, which as you noted upthread is not
very user-friendly for long values. I thought there was some value in
your original idea of having a way to get psql to use extended query mode
with the inserted value being sent as a parameter. But again, I'd rather
see that decoupled as a separate feature and not tightly tied to the
use-case of interpolating a file.
Maybe using extended mode could be driven off a setting rather than being
identified by syntax? There doesn't seem to be much reason why you'd want
some of the :-interpolated values in a query to be inlined and others sent
as parameters. Also, for testing purposes, it'd be mighty nice to have a
way of invoking extended query mode in psql with a clear way to define
which things are sent as parameters. But I don't want to have to make
separate files to contain the values being sent.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
2016-11-16 17:58 GMT+01:00 Tom Lane <tgl@sss.pgh.pa.us>:
Pavel Stehule <pavel.stehule@gmail.com> writes:
The Daniel's proposal has important issues:
1. you need to store and hold the content in memory
2. you cannot use tab complete - without it this feature lost a usability
3. you have to do two steps
4. Depends on o.s.I think you're putting *way* too much emphasis on tab completion of the
filename. That's a nice-to-have, but it should not cause us to contort
the feature design to get it.
I am sorry, I cannot to agree. When you cannot use tab complete in
interactive mode, then you lost valuable help.
I'm not especially impressed by objections 3 and 4, either. Point #1
has some merit, but since the wire protocol is going to limit us to
circa 1GB anyway, it doesn't seem fatal.
There are not "cat" on ms win. For relative basic functionality you have to
switch between platforms.
What I like about Daniel's proposal is that because it adds two separate
new behaviors, it can be used for more things than just "interpolate a
file". What I don't like is that we're still stuck in the land of textual
interpolation into query strings, which as you noted upthread is not
very user-friendly for long values. I thought there was some value in
your original idea of having a way to get psql to use extended query mode
with the inserted value being sent as a parameter. But again, I'd rather
see that decoupled as a separate feature and not tightly tied to the
use-case of interpolating a file.
With content commands syntax, I am able to control it simply - there is
space for invention of new features.
the my patch has full functionality of Daniel's proposal
\set xxx {rb somefile} - is supported already in my last patch
Now I am working only with real files, but the patch can be simply extended
to work with named pipes, with everything. There is a space for extending.
Maybe using extended mode could be driven off a setting rather than being
identified by syntax?
It is possible, but I don't think so it is user friendly - what is worst -
use special syntax or search setting some psql set value?
Show quoted text
There doesn't seem to be much reason why you'd want
some of the :-interpolated values in a query to be inlined and others sent
as parameters. Also, for testing purposes, it'd be mighty nice to have a
way of invoking extended query mode in psql with a clear way to define
which things are sent as parameters. But I don't want to have to make
separate files to contain the values being sent.regards, tom lane
Hi
a independent implementation of parametrized queries can looks like
attached patch:
this code is simple without any unexpected behave.
parameters are used when user doesn't require escaping and when
PARAMETRIZED_QUERIES variable is on
Regards
Pavel
Attachments:
psql-parametrized-queries.patchtext/x-patch; charset=US-ASCII; name=psql-parametrized-queries.patchDownload
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index a7789df..57dd8f0 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -119,9 +119,13 @@ setQFout(const char *fname)
* If "escape" is true, return the value suitably quoted and escaped,
* as an identifier or string literal depending on "as_ident".
* (Failure in escaping should lead to returning NULL.)
+ *
+ * When "from_query" is true, then the variable can be passed as query parameter,
+ * when it is not used as identifier (as_ident:false), when escape is not required
+ * (escaping changes the content).
*/
char *
-psql_get_variable(const char *varname, bool escape, bool as_ident)
+psql_get_variable(const char *varname, bool escape, bool as_ident, bool from_query)
{
char *result;
const char *value;
@@ -130,6 +134,26 @@ psql_get_variable(const char *varname, bool escape, bool as_ident)
if (!value)
return NULL;
+ if (from_query && pset.parametrized_queries)
+ {
+ if (!escape && !as_ident)
+ {
+ char printbuf[5];
+
+ if (pset.nparams > MAX_QUERY_PARAMS)
+ {
+ psql_error("too much parameters (more than %d)\n",
+ MAX_QUERY_PARAMS);
+ return NULL;
+ }
+
+ pset.params[pset.nparams++] = value;
+ snprintf(printbuf, sizeof(printbuf) - 1, "$%d", pset.nparams);
+
+ return pstrdup(printbuf);
+ }
+ }
+
if (escape)
{
char *escaped_value;
@@ -1287,7 +1311,16 @@ SendQuery(const char *query)
if (pset.timing)
INSTR_TIME_SET_CURRENT(before);
- results = PQexec(pset.db, query);
+ if (pset.nparams > 0)
+ results = PQexecParams(pset.db, query,
+ pset.nparams,
+ NULL,
+ (const char * const *) pset.params,
+ NULL,
+ NULL,
+ 0);
+ else
+ results = PQexec(pset.db, query);
/* these operations are included in the timing result: */
ResetCancelConn();
@@ -1382,6 +1415,9 @@ SendQuery(const char *query)
ClearOrSaveResult(results);
+ /* the number of query parameters are not necessary now */
+ pset.nparams = 0;
+
/* Possible microtiming output */
if (pset.timing)
PrintTiming(elapsed_msec);
@@ -1488,7 +1524,16 @@ ExecQueryUsingCursor(const char *query, double *elapsed_msec)
appendPQExpBuffer(&buf, "DECLARE _psql_cursor NO SCROLL CURSOR FOR\n%s",
query);
- results = PQexec(pset.db, buf.data);
+ if (pset.nparams > 0)
+ results = PQexecParams(pset.db, buf.data,
+ pset.nparams,
+ NULL,
+ (const char * const *) pset.params,
+ NULL,
+ NULL,
+ 0);
+ else
+ results = PQexec(pset.db, buf.data);
OK = AcceptResult(results) &&
(PQresultStatus(results) == PGRES_COMMAND_OK);
ClearOrSaveResult(results);
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index bdcb58f..79f8c91 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -18,7 +18,7 @@
extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
extern bool setQFout(const char *fname);
-extern char *psql_get_variable(const char *varname, bool escape, bool as_ident);
+extern char *psql_get_variable(const char *varname, bool escape, bool as_ident, bool from_query);
extern void psql_error(const char *fmt,...) pg_attribute_printf(1, 2);
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index a69c4dd..3c62146 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -325,7 +325,7 @@ helpVariables(unsigned short int pager)
* Windows builds currently print one more line than non-Windows builds.
* Using the larger number is fine.
*/
- output = PageOutput(88, pager ? &(pset.popt.topt) : NULL);
+ output = PageOutput(90, pager ? &(pset.popt.topt) : NULL);
fprintf(output, _("List of specially treated variables\n\n"));
@@ -352,6 +352,8 @@ helpVariables(unsigned short int pager)
fprintf(output, _(" LASTOID value of the last affected OID\n"));
fprintf(output, _(" ON_ERROR_ROLLBACK if set, an error doesn't stop a transaction (uses implicit savepoints)\n"));
fprintf(output, _(" ON_ERROR_STOP stop batch execution after error\n"));
+ fprintf(output, _(" PARAMETRIZED_QUERIES\n"
+ " pass psql's variables as query parameters\n"));
fprintf(output, _(" PORT server port of the current connection\n"));
fprintf(output, _(" PROMPT1 specifies the standard psql prompt\n"));
fprintf(output, _(" PROMPT2 specifies the prompt used when a statement continues from a previous line\n"));
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index 37dfa4d..cb6a2ee 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -403,6 +403,9 @@ MainLoop(FILE *source)
psql_scan_finish(scan_state);
free(line);
+ /* reset a number of query parameters */
+ pset.nparams = 0;
+
if (slashCmdStatus == PSQL_CMD_TERMINATE)
{
successResult = EXIT_SUCCESS;
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index 86832a8..4a34f29 100644
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -243,6 +243,7 @@ other .
yyleng - 1);
value = cur_state->callbacks->get_variable(varname,
false,
+ false,
false);
free(varname);
@@ -271,7 +272,7 @@ other .
ECHO;
else
{
- psqlscan_escape_variable(cur_state, yytext, yyleng, false);
+ psqlscan_escape_variable(cur_state, yytext, yyleng, false, false);
*option_quote = ':';
}
unquoted_option_chars = 0;
@@ -283,7 +284,7 @@ other .
ECHO;
else
{
- psqlscan_escape_variable(cur_state, yytext, yyleng, true);
+ psqlscan_escape_variable(cur_state, yytext, yyleng, true, false);
*option_quote = ':';
}
unquoted_option_chars = 0;
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 8cfe9d2..77e4611 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -77,6 +77,8 @@ enum trivalue
TRI_YES
};
+#define MAX_QUERY_PARAMS 64
+
typedef struct _psqlSettings
{
PGconn *db; /* connection to backend */
@@ -120,6 +122,7 @@ typedef struct _psqlSettings
* functions.
*/
bool autocommit;
+ bool parametrized_queries;
bool on_error_stop;
bool quiet;
bool singleline;
@@ -135,6 +138,8 @@ typedef struct _psqlSettings
const char *prompt3;
PGVerbosity verbosity; /* current error verbosity level */
PGContextVisibility show_context; /* current context display level */
+ int nparams; /* number of query parameters */
+ const char *params[MAX_QUERY_PARAMS]; /* query parameters */
} PsqlSettings;
extern PsqlSettings pset;
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index 7ce05fb..f8fceee 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -139,6 +139,8 @@ main(int argc, char *argv[])
pset.last_error_result = NULL;
pset.cur_cmd_source = stdin;
pset.cur_cmd_interactive = false;
+ pset.parametrized_queries = false;
+ pset.nparams = 0;
/* We rely on unmentioned fields of pset.popt to start out 0/false/NULL */
pset.popt.topt.format = PRINT_ALIGNED;
@@ -793,6 +795,12 @@ autocommit_hook(const char *newval)
}
static void
+parametrized_queries_hook(const char *newval)
+{
+ pset.parametrized_queries = ParseVariableBool(newval, "PARAMETRIZED_QUERIES");
+}
+
+static void
on_error_stop_hook(const char *newval)
{
pset.on_error_stop = ParseVariableBool(newval, "ON_ERROR_STOP");
@@ -990,6 +998,7 @@ EstablishVariableSpace(void)
SetVariableAssignHook(pset.vars, "ON_ERROR_ROLLBACK", on_error_rollback_hook);
SetVariableAssignHook(pset.vars, "COMP_KEYWORD_CASE", comp_keyword_case_hook);
SetVariableAssignHook(pset.vars, "HISTCONTROL", histcontrol_hook);
+ SetVariableAssignHook(pset.vars, "PARAMETRIZED_QUERIES", parametrized_queries_hook);
SetVariableAssignHook(pset.vars, "PROMPT1", prompt1_hook);
SetVariableAssignHook(pset.vars, "PROMPT2", prompt2_hook);
SetVariableAssignHook(pset.vars, "PROMPT3", prompt3_hook);
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index b556c00..03984c5 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -3191,8 +3191,8 @@ psql_completion(const char *text, int start, int end)
}
else if (TailMatchesCS2("\\set", MatchAny))
{
- if (TailMatchesCS1("AUTOCOMMIT|ON_ERROR_STOP|QUIET|"
- "SINGLELINE|SINGLESTEP"))
+ if (TailMatchesCS1("AUTOCOMMIT|ON_ERROR_STOP|PARAMETRIZED_QUERIES|"
+ "QUIET|SINGLELINE|SINGLESTEP"))
COMPLETE_WITH_LIST_CS2("on", "off");
else if (TailMatchesCS1("COMP_KEYWORD_CASE"))
COMPLETE_WITH_LIST_CS4("lower", "upper",
@@ -3684,7 +3684,7 @@ complete_from_variables(const char *text, const char *prefix, const char *suffix
"AUTOCOMMIT", "COMP_KEYWORD_CASE", "DBNAME", "ECHO", "ECHO_HIDDEN",
"ENCODING", "FETCH_COUNT", "HISTCONTROL", "HISTFILE", "HISTSIZE",
"HOST", "IGNOREEOF", "LASTOID", "ON_ERROR_ROLLBACK", "ON_ERROR_STOP",
- "PORT", "PROMPT1", "PROMPT2", "PROMPT3", "QUIET",
+ "PARAMETRIZED_QUERIES", "PORT", "PROMPT1", "PROMPT2", "PROMPT3", "QUIET",
"SHOW_CONTEXT", "SINGLELINE", "SINGLESTEP",
"USER", "VERBOSITY", NULL
};
diff --git a/src/fe_utils/psqlscan.l b/src/fe_utils/psqlscan.l
index 55067b4..06a6519 100644
--- a/src/fe_utils/psqlscan.l
+++ b/src/fe_utils/psqlscan.l
@@ -700,7 +700,8 @@ other .
if (cur_state->callbacks->get_variable)
value = cur_state->callbacks->get_variable(varname,
false,
- false);
+ false,
+ true);
else
value = NULL;
@@ -736,11 +737,11 @@ other .
}
:'{variable_char}+' {
- psqlscan_escape_variable(cur_state, yytext, yyleng, false);
+ psqlscan_escape_variable(cur_state, yytext, yyleng, false, true);
}
:\"{variable_char}+\" {
- psqlscan_escape_variable(cur_state, yytext, yyleng, true);
+ psqlscan_escape_variable(cur_state, yytext, yyleng, true, true);
}
/*
@@ -1401,7 +1402,7 @@ psqlscan_extract_substring(PsqlScanState state, const char *txt, int len)
*/
void
psqlscan_escape_variable(PsqlScanState state, const char *txt, int len,
- bool as_ident)
+ bool as_ident, bool from_query)
{
char *varname;
char *value;
@@ -1409,7 +1410,8 @@ psqlscan_escape_variable(PsqlScanState state, const char *txt, int len,
/* Variable lookup. */
varname = psqlscan_extract_substring(state, txt + 2, len - 3);
if (state->callbacks->get_variable)
- value = state->callbacks->get_variable(varname, true, as_ident);
+ value = state->callbacks->get_variable(varname,
+ true, as_ident, from_query);
else
value = NULL;
free(varname);
diff --git a/src/include/fe_utils/psqlscan.h b/src/include/fe_utils/psqlscan.h
index 1f10ecc..3854117 100644
--- a/src/include/fe_utils/psqlscan.h
+++ b/src/include/fe_utils/psqlscan.h
@@ -53,7 +53,8 @@ typedef struct PsqlScanCallbacks
{
/* Fetch value of a variable, as a pfree'able string; NULL if unknown */
/* This pointer can be NULL if no variable substitution is wanted */
- char *(*get_variable) (const char *varname, bool escape, bool as_ident);
+ char *(*get_variable) (const char *varname,
+ bool escape, bool as_ident, bool from_query);
/* Print an error message someplace appropriate */
/* (very old gcc versions don't support attributes on function pointers) */
#if defined(__GNUC__) && __GNUC__ < 4
diff --git a/src/include/fe_utils/psqlscan_int.h b/src/include/fe_utils/psqlscan_int.h
index a52929d..53210b2 100644
--- a/src/include/fe_utils/psqlscan_int.h
+++ b/src/include/fe_utils/psqlscan_int.h
@@ -139,6 +139,7 @@ extern char *psqlscan_extract_substring(PsqlScanState state,
const char *txt, int len);
extern void psqlscan_escape_variable(PsqlScanState state,
const char *txt, int len,
- bool as_ident);
+ bool as_ident,
+ bool from_query);
#endif /* PSQLSCAN_INT_H */
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 464436a..7fed568 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -2714,3 +2714,36 @@ NOTICE: foo
CONTEXT: PL/pgSQL function inline_code_block line 3 at RAISE
ERROR: bar
CONTEXT: PL/pgSQL function inline_code_block line 4 at RAISE
+-- parametrized queries
+\set PARAMETRIZED_QUERIES off
+\set a1 'AHOJ SVETE'
+-- should fail
+SELECT :a1;
+ERROR: column "ahoj" does not exist
+LINE 1: SELECT AHOJ SVETE;
+ ^
+-- ok
+SELECT :'a1';
+ ?column?
+------------
+ AHOJ SVETE
+(1 row)
+
+\set PARAMETRIZED_QUERIES on
+-- should fail - unknown type
+SELECT :a1;
+ERROR: could not determine data type of parameter $1
+-- ok
+SELECT :a1::text;
+ text
+------------
+ AHOJ SVETE
+(1 row)
+
+-- returns true, when value passed as parameter is same as client side evaluated variable
+SELECT :a1 = :'a1';
+ ?column?
+----------
+ t
+(1 row)
+
diff --git a/src/test/regress/sql/psql.sql b/src/test/regress/sql/psql.sql
index 900aa7e..69e2df1 100644
--- a/src/test/regress/sql/psql.sql
+++ b/src/test/regress/sql/psql.sql
@@ -379,3 +379,23 @@ begin
raise notice 'foo';
raise exception 'bar';
end $$;
+
+-- parametrized queries
+\set PARAMETRIZED_QUERIES off
+\set a1 'AHOJ SVETE'
+
+-- should fail
+SELECT :a1;
+
+-- ok
+SELECT :'a1';
+
+\set PARAMETRIZED_QUERIES on
+-- should fail - unknown type
+SELECT :a1;
+
+-- ok
+SELECT :a1::text;
+
+-- returns true, when value passed as parameter is same as client side evaluated variable
+SELECT :a1 = :'a1';
Hi
2016-11-17 12:00 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
a independent implementation of parametrized queries can looks like
attached patch:this code is simple without any unexpected behave.
parameters are used when user doesn't require escaping and when
PARAMETRIZED_QUERIES variable is onRegards
here is a Daniel's syntax patch
The independent implementation of using query parameters is good idea, the
patch looks well, and there is a agreement with Tom.
I implemented a Daniel proposal - it is few lines, but personally I prefer
curly bracket syntax against `` syntax. When separator is double char, then
the correct implementation will not be simple - the bad combinations "` ``,
`` `" should be handled better. I wrote my arguments for my design, but if
these arguments are not important for any other, then I can accept any
other designs.
regards
Pavel
Show quoted text
Pavel
Attachments:
psql-backtick-hex-output.patchtext/x-patch; charset=US-ASCII; name=psql-backtick-hex-output.patchDownload
diff --git a/src/bin/psql/psqlscanslash.l b/src/bin/psql/psqlscanslash.l
index 86832a8..15ad610 100644
--- a/src/bin/psql/psqlscanslash.l
+++ b/src/bin/psql/psqlscanslash.l
@@ -19,7 +19,7 @@
#include "postgres_fe.h"
#include "psqlscanslash.h"
-
+#include "settings.h"
#include "libpq-fe.h"
}
@@ -53,7 +53,7 @@ static int backtick_start_offset;
#define LEXRES_OK 1 /* OK completion of backslash argument */
-static void evaluate_backtick(PsqlScanState state);
+static void evaluate_backtick(PsqlScanState state, bool to_hex);
#define ECHO psqlscan_emit(cur_state, yytext, yyleng)
@@ -221,6 +221,13 @@ other .
BEGIN(xslashbackquote);
}
+"``" {
+ backtick_start_offset = output_buf->len;
+ *option_quote = '@';
+ unquoted_option_chars = 0;
+ BEGIN(xslashbackquote);
+ }
+
{dquote} {
ECHO;
*option_quote = '"';
@@ -354,10 +361,18 @@ other .
"`" {
/* In NO_EVAL mode, don't evaluate the command */
if (option_type != OT_NO_EVAL)
- evaluate_backtick(cur_state);
+ evaluate_backtick(cur_state, false);
BEGIN(xslasharg);
}
+"``" {
+ /* In NO_EVAL mode, don't evaluate the command */
+ if (option_type != OT_NO_EVAL && *option_quote == '@')
+ evaluate_backtick(cur_state, true);
+ BEGIN(xslasharg);
+ }
+
+
{other}|\n { ECHO; }
}
@@ -693,7 +708,7 @@ dequote_downcase_identifier(char *str, bool downcase, int encoding)
* as a shell command and then replaced by the command's output.
*/
static void
-evaluate_backtick(PsqlScanState state)
+evaluate_backtick(PsqlScanState state, bool to_hex)
{
PQExpBuffer output_buf = state->output_buf;
char *cmd = output_buf->data + backtick_start_offset;
@@ -750,7 +765,20 @@ evaluate_backtick(PsqlScanState state)
if (cmd_output.len > 0 &&
cmd_output.data[cmd_output.len - 1] == '\n')
cmd_output.len--;
- appendBinaryPQExpBuffer(output_buf, cmd_output.data, cmd_output.len);
+
+ if (to_hex)
+ {
+ unsigned char *escaped_value;
+ size_t escaped_size;
+
+ escaped_value = PQescapeByteaConn(pset.db,
+ (const unsigned char *) cmd_output.data, cmd_output.len,
+ &escaped_size);
+
+ appendBinaryPQExpBuffer(output_buf, (const char *) escaped_value, escaped_size);
+ }
+ else
+ appendBinaryPQExpBuffer(output_buf, cmd_output.data, cmd_output.len);
}
termPQExpBuffer(&cmd_output);
On Fri, Nov 18, 2016 at 5:53 AM, Pavel Stehule <pavel.stehule@gmail.com>
wrote:
Hi
2016-11-17 12:00 GMT+01:00 Pavel Stehule <pavel.stehule@gmail.com>:
Hi
a independent implementation of parametrized queries can looks like
attached patch:this code is simple without any unexpected behave.
parameters are used when user doesn't require escaping and when
PARAMETRIZED_QUERIES variable is onRegards
here is a Daniel's syntax patch
The independent implementation of using query parameters is good idea, the
patch looks well, and there is a agreement with Tom.I implemented a Daniel proposal - it is few lines, but personally I prefer
curly bracket syntax against `` syntax. When separator is double char, then
the correct implementation will not be simple - the bad combinations "` ``,
`` `" should be handled better. I wrote my arguments for my design, but if
these arguments are not important for any other, then I can accept any
other designs.
The recent updated patch as per the agreement hasn't received any feedback
from reviewers. Moved to next CF with "needs review" status.
Regards,
Hari Babu
Fujitsu Australia