Add \i option to bring in the specified file as a quoted literal
Hi,
I would like to implement item from TODO marked as easy: "Add \i option
to bring in the specified file as a quoted literal". I understand intent
of this item, to be able to have parts of query written in separate
files (now it is impossible, because \i tries to execute content of file
as a separate command by function process_file).
Implementation:
I will add command "iq" and "include_quoted" in src/bin/psql/command.c
(in the same block where "i" and "ir" are implemented). If "iq" is
detected I will call new function do_append(fname, query_buf) which will
read file into query_buffer and return status PSQL_CMD_NEWEDIT.
User perespective:
User will be able to append file to current buffer. It will be possible
to include code of procedure, or where part of select statement.
ex:
select * from
/iq where_part.sql
and lang_name='SQL'
;
Questions:
Variables like :myvar will be replaced with their values, is that ok?
Shall I start to code this patch?
Best regards
Piotr Marcinczyk
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Oct 22, 2013 at 3:04 AM, Piotr Marcinczyk <pmarcinc@gmail.com> wrote:
Hi,
I would like to implement item from TODO marked as easy: "Add \i option
to bring in the specified file as a quoted literal". I understand intent
of this item, to be able to have parts of query written in separate
files (now it is impossible, because \i tries to execute content of file
as a separate command by function process_file).
For the usecase discussed in the mail chain of that TODO item, Robert
Haas has provided an alternative to achieve it, please check below
link:
/messages/by-id/AANLkTi=7C8xFYF7uQW0y+si8oNdKoY2NX8jc4bU0GWvY@mail.gmail.com
If you think that alternative is not sufficient for the use case, then
adding new option/syntax is worth, otherwise it might be a shortcut or
other form of some existing way which can be useful depending on how
frequently users use this syntax.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Oct 23, 2013 at 10:31:39AM +0530, Amit Kapila wrote:
On Tue, Oct 22, 2013 at 3:04 AM, Piotr Marcinczyk <pmarcinc@gmail.com> wrote:
Hi,
I would like to implement item from TODO marked as easy: "Add \i option
to bring in the specified file as a quoted literal". I understand intent
of this item, to be able to have parts of query written in separate
files (now it is impossible, because \i tries to execute content of file
as a separate command by function process_file).For the usecase discussed in the mail chain of that TODO item, Robert
Haas has provided an alternative to achieve it, please check below
link:
/messages/by-id/AANLkTi=7C8xFYF7uQW0y+si8oNdKoY2NX8jc4bU0GWvY@mail.gmail.comIf you think that alternative is not sufficient for the use case, then
adding new option/syntax is worth, otherwise it might be a shortcut or
other form of some existing way which can be useful depending on how
frequently users use this syntax.
So, can we remove this TODO item?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Tue, Nov 12, 2013 at 9:37 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Oct 23, 2013 at 10:31:39AM +0530, Amit Kapila wrote:
On Tue, Oct 22, 2013 at 3:04 AM, Piotr Marcinczyk <pmarcinc@gmail.com> wrote:
Hi,
I would like to implement item from TODO marked as easy: "Add \i option
to bring in the specified file as a quoted literal". I understand intent
of this item, to be able to have parts of query written in separate
files (now it is impossible, because \i tries to execute content of file
as a separate command by function process_file).For the usecase discussed in the mail chain of that TODO item, Robert
Haas has provided an alternative to achieve it, please check below
link:
/messages/by-id/AANLkTi=7C8xFYF7uQW0y+si8oNdKoY2NX8jc4bU0GWvY@mail.gmail.comIf you think that alternative is not sufficient for the use case, then
adding new option/syntax is worth, otherwise it might be a shortcut or
other form of some existing way which can be useful depending on how
frequently users use this syntax.So, can we remove this TODO item?
TODO item is created before Robert Haas has provided an alternative
way to achieve the same thing. In some cases there are multiple ways
to
achieve the same thing (example: shortcut options in psql) if it is
used quite frequently and people want some easy way of doing it. In
this case I
don't think this is used frequently, so I don't see the need of
doing it. We should remove this TODO item.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Nov 13, 2013 at 08:58:07AM +0530, Amit Kapila wrote:
On Tue, Nov 12, 2013 at 9:37 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Oct 23, 2013 at 10:31:39AM +0530, Amit Kapila wrote:
On Tue, Oct 22, 2013 at 3:04 AM, Piotr Marcinczyk <pmarcinc@gmail.com> wrote:
Hi,
I would like to implement item from TODO marked as easy: "Add \i option
to bring in the specified file as a quoted literal". I understand intent
of this item, to be able to have parts of query written in separate
files (now it is impossible, because \i tries to execute content of file
as a separate command by function process_file).For the usecase discussed in the mail chain of that TODO item, Robert
Haas has provided an alternative to achieve it, please check below
link:
/messages/by-id/AANLkTi=7C8xFYF7uQW0y+si8oNdKoY2NX8jc4bU0GWvY@mail.gmail.comIf you think that alternative is not sufficient for the use case, then
adding new option/syntax is worth, otherwise it might be a shortcut or
other form of some existing way which can be useful depending on how
frequently users use this syntax.So, can we remove this TODO item?
TODO item is created before Robert Haas has provided an alternative
way to achieve the same thing. In some cases there are multiple ways
to
achieve the same thing (example: shortcut options in psql) if it is
used quite frequently and people want some easy way of doing it. In
this case I
don't think this is used frequently, so I don't see the need of
doing it. We should remove this TODO item.
OK, removed.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Dnia 2013-11-13, śro o godzinie 10:26 -0500, Bruce Momjian pisze:
On Wed, Nov 13, 2013 at 08:58:07AM +0530, Amit Kapila wrote:
On Tue, Nov 12, 2013 at 9:37 PM, Bruce Momjian <bruce@momjian.us> wrote:
On Wed, Oct 23, 2013 at 10:31:39AM +0530, Amit Kapila wrote:
On Tue, Oct 22, 2013 at 3:04 AM, Piotr Marcinczyk <pmarcinc@gmail.com> wrote:
Hi,
I would like to implement item from TODO marked as easy: "Add \i option
to bring in the specified file as a quoted literal". I understand intent
of this item, to be able to have parts of query written in separate
files (now it is impossible, because \i tries to execute content of file
as a separate command by function process_file).For the usecase discussed in the mail chain of that TODO item, Robert
Haas has provided an alternative to achieve it, please check below
link:
/messages/by-id/AANLkTi=7C8xFYF7uQW0y+si8oNdKoY2NX8jc4bU0GWvY@mail.gmail.comIf you think that alternative is not sufficient for the use case, then
adding new option/syntax is worth, otherwise it might be a shortcut or
other form of some existing way which can be useful depending on how
frequently users use this syntax.So, can we remove this TODO item?
TODO item is created before Robert Haas has provided an alternative
way to achieve the same thing. In some cases there are multiple ways
to
achieve the same thing (example: shortcut options in psql) if it is
used quite frequently and people want some easy way of doing it. In
this case I
don't think this is used frequently, so I don't see the need of
doing it. We should remove this TODO item.OK, removed.
Well, I wrote it few days ago. I'm sure it is not critical, but I
suppose it may be useful. This is my first patch, so I think that it is
good idea to sent it and have it reviewed anyway. First argument to send
it, is to see what kind of errors I made, to not do them in the next
patches. Second, if it (flexible appending file to buffer) appears
interesting for reviewer, it may be committed.
Attachments:
include_quotted_v1.patchtext/x-patch; charset=UTF-8; name=include_quotted_v1.patchDownload
*** a/doc/src/sgml/ref/psql-ref.sgml
--- b/doc/src/sgml/ref/psql-ref.sgml
***************
*** 1731,1736 **** hello 10
--- 1731,1749 ----
<varlistentry>
+ <term><literal>\ib <replaceable class="parameter">filename</replaceable> [ <replaceable class="parameter">quote_string</replaceable> ] </literal></term>
+ <listitem>
+ <para>
+ The <literal>\ib</> command appends content of file <literal>filename</literal>
+ to current query buffer. If parameter <literal>quote_string</literal>
+ is not set, no quotation is used. If it is set, content of file will be
+ quoted by <literal>quote_string</literal> enclosed in <literal>$</literal>.
+ </para>
+ </listitem>
+ </varlistentry>
+
+
+ <varlistentry>
<term><literal>\ir <replaceable class="parameter">filename</replaceable></literal></term>
<listitem>
<para>
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
***************
*** 59,64 **** static backslashResult exec_command(const char *cmd,
--- 59,65 ----
PQExpBuffer query_buf);
static bool do_edit(const char *filename_arg, PQExpBuffer query_buf,
int lineno, bool *edited);
+ static bool do_append(const char *filename_arg, const char *quote_string, PQExpBuffer query_buf);
static bool do_connect(char *dbname, char *user, char *host, char *port);
static bool do_shell(const char *command);
static bool do_watch(PQExpBuffer query_buf, long sleep);
***************
*** 798,806 **** exec_command(const char *cmd,
}
! /* \i and \ir include files */
else if (strcmp(cmd, "i") == 0 || strcmp(cmd, "include") == 0
! || strcmp(cmd, "ir") == 0 || strcmp(cmd, "include_relative") == 0)
{
char *fname = psql_scan_slash_option(scan_state,
OT_NORMAL, NULL, true);
--- 799,808 ----
}
! /* \i, \ib and \ir include files */
else if (strcmp(cmd, "i") == 0 || strcmp(cmd, "include") == 0
! || strcmp(cmd, "ir") == 0 || strcmp(cmd, "include_relative") == 0
! || strcmp(cmd, "ib") == 0 || strcmp(cmd, "include_buffer") == 0)
{
char *fname = psql_scan_slash_option(scan_state,
OT_NORMAL, NULL, true);
***************
*** 812,823 **** exec_command(const char *cmd,
}
else
{
! bool include_relative;
- include_relative = (strcmp(cmd, "ir") == 0
- || strcmp(cmd, "include_relative") == 0);
- expand_tilde(&fname);
- success = (process_file(fname, false, include_relative) == EXIT_SUCCESS);
free(fname);
}
}
--- 814,845 ----
}
else
{
! bool include_buffer;
!
! include_buffer = (strcmp(cmd, "ib") == 0
! || strcmp(cmd, "include_buffer") == 0);
!
!
! if (include_buffer)
! {
! char *quote_string = psql_scan_slash_option(scan_state,
! OT_NORMAL, NULL, true);
! expand_tilde(&fname);
! success = !do_append(fname, quote_string, query_buf);
! if (success) {
! status = PSQL_CMD_NEWEDIT;
! } else {
! status = PSQL_CMD_ERROR;
! }
! } else {
! bool include_relative;
!
! include_relative = (strcmp(cmd, "ir") == 0
! || strcmp(cmd, "include_relative") == 0);
! expand_tilde(&fname);
! success = (process_file(fname, false, include_relative) == EXIT_SUCCESS);
! }
free(fname);
}
}
***************
*** 2099,2104 **** do_edit(const char *filename_arg, PQExpBuffer query_buf,
--- 2121,2175 ----
}
+ /*
+ * do_append
+ *
+ * Read content from file and append it to query_buffer.
+ */
+ static bool
+ do_append(const char *fname, const char *quote_string, PQExpBuffer query_buf)
+ {
+ FILE *stream = NULL;
+ bool error = false;
+ stream = fopen(fname, PG_BINARY_R);
+ if (!stream)
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ error = true;
+ }
+ else
+ {
+ /* read file back into query_buf */
+ char line[1024];
+
+ /* insert newline to separate buffer from file input */
+ appendPQExpBufferStr(query_buf, "\n");
+ /* if quote_string exists add it before file content */
+ if (quote_string != NULL) {
+ appendPQExpBufferStr(query_buf, "$");
+ appendPQExpBufferStr(query_buf, quote_string);
+ appendPQExpBufferStr(query_buf, "$\n");
+ }
+ while (fgets(line, sizeof(line), stream) != NULL)
+ appendPQExpBufferStr(query_buf, line);
+
+ if (ferror(stream))
+ {
+ psql_error("%s: %s\n", fname, strerror(errno));
+ error = true;
+ }
+
+ fclose(stream);
+ /* if quote_string exists add it after file content */
+ if (quote_string != NULL) {
+ appendPQExpBufferStr(query_buf, "$");
+ appendPQExpBufferStr(query_buf, quote_string);
+ appendPQExpBufferStr(query_buf, "$");
+ }
+ }
+ return error;
+ }
+
/*
* process_file
*** a/src/bin/psql/help.c
--- b/src/bin/psql/help.c
***************
*** 187,192 **** slashUsage(unsigned short int pager)
--- 187,193 ----
fprintf(output, _(" \\s [FILE] display history or save it to file\n"));
#endif
fprintf(output, _(" \\w FILE write query buffer to file\n"));
+ fprintf(output, _(" \\ib FILE [QUOTE_STR] append file to query buffer\n"));
fprintf(output, "\n");
fprintf(output, _("Input/Output\n"));
Piotr Marcinczyk escribi�:
<varlistentry> + <term><literal>\ib <replaceable class="parameter">filename</replaceable> [ <replaceable class="parameter">quote_string</replaceable> ] </literal></term> + <listitem> + <para> + The <literal>\ib</> command appends content of file <literal>filename</literal> + to current query buffer. If parameter <literal>quote_string</literal> + is not set, no quotation is used. If it is set, content of file will be + quoted by <literal>quote_string</literal> enclosed in <literal>$</literal>. + </para> + </listitem> + </varlistentry>
Doesn't this quoting thing seem like a usability problem? I mean,
there's no way you can possibly know what string to use unless you first
verify the contents of the file yourself. I think this is something
that should be done automatically by psql.
But, really, having to read stuff and transform into a quoted literal
seems wrong to me. I would like something that would read into a client
variable that can later be used as a positional parameter to a
parametrized query, so
\ib homer ~/photos/homer.jpg
insert into people (name, photo) values ('Homer', :homer);
and have psql turn that into
PQexecParams("insert into people (name, photo) values ('homer', $1)",
some_array_with_homer);
so that no quoting needs to happen anywhere.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Nov 22, 2013 at 1:33 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
Piotr Marcinczyk escribió:
<varlistentry> + <term><literal>\ib <replaceable class="parameter">filename</replaceable> [ <replaceable class="parameter">quote_string</replaceable> ] </literal></term> + <listitem> + <para> + The <literal>\ib</> command appends content of file <literal>filename</literal> + to current query buffer. If parameter <literal>quote_string</literal> + is not set, no quotation is used. If it is set, content of file will be + quoted by <literal>quote_string</literal> enclosed in <literal>$</literal>. + </para> + </listitem> + </varlistentry>Doesn't this quoting thing seem like a usability problem? I mean,
there's no way you can possibly know what string to use unless you first
verify the contents of the file yourself. I think this is something
that should be done automatically by psql.But, really, having to read stuff and transform into a quoted literal
seems wrong to me. I would like something that would read into a client
variable that can later be used as a positional parameter to a
parametrized query, so\ib homer ~/photos/homer.jpg
insert into people (name, photo) values ('Homer', :homer);
Isn't something similar already supported as mentioned in docs:
One example use of this mechanism is to copy the contents of a file
into a table column. First load the file into a variable and then
interpolate the variable's value as a quoted string:
testdb=> \set content `cat my_file.txt`
testdb=> INSERT INTO my_table VALUES (:'content');
or do you prefer an alternative without any kind of quote using \ib?
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Amit Kapila escribi�:
On Fri, Nov 22, 2013 at 1:33 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
\ib homer ~/photos/homer.jpg
insert into people (name, photo) values ('Homer', :homer);Isn't something similar already supported as mentioned in docs:
One example use of this mechanism is to copy the contents of a file
into a table column. First load the file into a variable and then
interpolate the variable's value as a quoted string:testdb=> \set content `cat my_file.txt`
testdb=> INSERT INTO my_table VALUES (:'content');or do you prefer an alternative without any kind of quote using \ib?
If the only use case of the feature proposed in this thread is to load
stuff from files to use as column values, then we're pretty much done,
and this patch is not needed -- except, maybe, that the `` is unlikely
to work on Windows, as already mentioned elsewhere. But if the OP had
something else in mind, let's hear what it is.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, 2013-11-22 at 09:54 -0300, Alvaro Herrera wrote:
Amit Kapila escribi�:
On Fri, Nov 22, 2013 at 1:33 AM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:\ib homer ~/photos/homer.jpg
insert into people (name, photo) values ('Homer', :homer);Isn't something similar already supported as mentioned in docs:
One example use of this mechanism is to copy the contents of a file
into a table column. First load the file into a variable and then
interpolate the variable's value as a quoted string:testdb=> \set content `cat my_file.txt`
testdb=> INSERT INTO my_table VALUES (:'content');or do you prefer an alternative without any kind of quote using \ib?
If the only use case of the feature proposed in this thread is to load
stuff from files to use as column values, then we're pretty much done,
and this patch is not needed -- except, maybe, that the `` is unlikely
to work on Windows, as already mentioned elsewhere. But if the OP had
something else in mind, let's hear what it is.
I've test set command mentioned above, and I think it is enough. At this
moment I see no point in implementing new command.
Best Regards
Piotr Marcinczyk
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers