psql: Allow editing query results with \gedit
Assuming a SELECT statement reading from a single table, it is quite an
effort to transform that statement to an UPDATE statement on that table,
perhaps to fix a typo that the user has spotted in the query result.
First, the general syntax is not the same with the order of syntax
elements changed. Then the row in question needs to be pinned down by
the primary key, requiring cut-and-paste of the PK columns. Furthermore,
the value to be updated needs to be put into the command, with proper
quoting. If the original value spans multiple line, copy-and-pasting it
for editing is especially tedious.
Suppose the following query where we spot a typo in the 2nd message:
=# select id, language, message from messages where language = 'en';
id | language | message
1 | en | Good morning
2 | en | Hello warld
The query needs to be transformed into this update:
=# update messages set message = 'Hello world' where id = 2;
This patch automates the tedious parts by opening the query result in a
editor in JSON format, where the user can edit the data. On closing the
editor, the JSON data is read back, and the differences are sent as
UPDATE commands. New rows are INSERTed, and deleted rows are DELETEd.
=# select id, language, message from messages where language = 'en' \gedit
An editor opens:
[
{ "id": 1, "language": "en", "message": "Good morning" },
{ "id": 2, "language": "en", "message": "Hello warld" }
]
Let's fix the typo and save the file:
[
{ "id": 1, "language": "en", "message": "Good morning" },
{ "id": 2, "language": "en", "message": "Hello world" }
]
UPDATE messages SET message = 'Hello world' WHERE id = '2';
UPDATE 1
In this example, typing "WHERE id = 2" would not be too hard, but the
primary key might be a composite key, with complex non-numeric values.
This is supported as well.
If expanded mode (\x) is enabled, \gedit will use the expanded JSON
format, best suitable for long values.
This patch requires the "psql JSON output format" patch.
Christoph
Attachments:
v1-0001-psql-Allow-editing-query-results-with-gedit.patchtext/x-diff; charset=us-asciiDownload
From be3fef72af0adac7d3a6962f8cc78e167785a702 Mon Sep 17 00:00:00 2001
From: Christoph Berg <myon@debian.org>
Date: Mon, 22 Jan 2024 14:40:33 +0100
Subject: [PATCH] psql: Allow editing query results with \gedit
Assuming a SELECT statement reading from a single table, it is quite an
effort to transform that statement to an UPDATE statement on that table,
perhaps to fix a typo that the user has spotted in the query result.
First, the general syntax is not the same with the order of syntax
elements changed. Then the row in question needs to be pinned down by
the primary key, requiring cut-and-paste of the PK columns. Furthermore,
the value to be updated needs to be put into the command, with proper
quoting. If the original value spans multiple line, copy-and-pasting it
for editing is especially tedious.
Suppose the following query where we spot a typo in the 2nd message:
=# select id, language, message from messages where language = 'en';
id | language | message
1 | en | Good morning
2 | en | Hello warld
The query needs to be transformed into this update:
=# update messages set message = 'Hello world' where id = 2;
This patch automates the tedious parts by opening the query result in a
editor in JSON format, where the user can edit the data. On closing the
editor, the JSON data is read back, and the differences are sent as
UPDATE commands. New rows are INSERTed, and deleted rows are DELETEd.
=# select id, language, message from messages where language = 'en' \gedit
An editor opens:
[
{ "id": 1, "language": "en", "message": "Good morning" },
{ "id": 2, "language": "en", "message": "Hello warld" }
]
Let's fix the typo and save the file:
[
{ "id": 1, "language": "en", "message": "Good morning" },
{ "id": 2, "language": "en", "message": "Hello world" }
]
UPDATE messages SET message = 'Hello world' WHERE id = '2';
UPDATE 1
In this example, typing "WHERE id = 2" would not be too hard, but the
primary key might be a composite key, with complex non-numeric values.
This is supported as well.
If expanded mode (\x) is enabled, \gedit will use the expanded JSON
format, best suitable for long values:
=# \x
=# select id, language, message from messages where language = 'en' \gedit
[{
"id": 1,
"language": "en",
"message": "Good morning"
},{
"id": 2,
"language": "en",
"message": "Hello world"
}]
\gedit requires the PK (or any UNIQUE key) to be part of the select
statement:
=# select language, message from messages \gedit
\gedit: no key of table "messages" is contained in the returned query columns.
Select more columns or manually specify a key using \gedit (key=...)
If the user knows that their operation is sound even without a PK/UNIQUE
key, they can force a (set of) column(s) to be used:
=# select language, message from messages \gedit (key=message)
[
{ "language": "en", "message": "Good morning" },
{ "language": "en_US", "message": "Hello world" }
]
UPDATE messages SET language = 'en_US' WHERE message = 'Hello world';
UPDATE 1
---
doc/src/sgml/ref/psql-ref.sgml | 62 +++
src/bin/psql/Makefile | 1 +
src/bin/psql/command.c | 119 +++-
src/bin/psql/command.h | 2 +
src/bin/psql/common.c | 63 ++-
src/bin/psql/common.h | 1 +
src/bin/psql/gedit.c | 961 +++++++++++++++++++++++++++++++++
src/bin/psql/gedit.h | 17 +
src/bin/psql/help.c | 2 +
src/bin/psql/mainloop.c | 24 +-
src/bin/psql/meson.build | 2 +
src/bin/psql/settings.h | 3 +
src/bin/psql/t/030_gedit.pl | 106 ++++
src/bin/psql/tab-complete.c | 2 +-
14 files changed, 1360 insertions(+), 5 deletions(-)
create mode 100644 src/bin/psql/gedit.c
create mode 100644 src/bin/psql/gedit.h
create mode 100644 src/bin/psql/t/030_gedit.pl
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index f1f7eda082..5cc8f0d241 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2334,6 +2334,68 @@ Tue Oct 26 21:40:57 CEST 1999
</varlistentry>
+ <varlistentry id="app-psql-meta-command-gedit">
+ <term><literal>\gedit</literal></term>
+
+ <listitem>
+ <para>
+ Execute the SELECT query, edit the result with an editor, and send back
+ the changes as UPDATE statements.
+ </para>
+
+ <para>
+ If the current query buffer is empty, the most recently sent query
+ is described instead.
+ </para>
+
+ <para>
+ The SELECT query is executed normally on the server. The result is
+ rendered as JSON and opened in external editor. The user can then edit
+ the query result to change values. On saving, the changed fields are
+ sent back to the server as UPDATE statements. Newly added rows are sent
+ as INSERTs, deleted rows are sent as DELETEs.
+<programlisting>
+=> <userinput>SELECT id, language, message FROM messages \gedit</userinput>
+-- editor opens, user edits data, saves and quits
+UPDATE messages SET message = 'Hello world' WHERE id = '2';
+</programlisting>
+ </para>
+
+ <para>
+ In non-expanded display mode, each result row is rendered as one line
+ of JSON. When expanded mode is enabled, result columns are rendered as
+ separate lines. Expanded mode can be forced for this query.
+<programlisting>
+=> <userinput>\gedit (expanded)</userinput>
+</programlisting>
+ </para>
+
+ <para>
+ The table's primary key, or any unique key, must be part of the
+ selected columns. If the table does not have any (or none are
+ selected), and the user knows that the data is sufficiently selective,
+ a key can be manually selected by providing a <literal>key</literal>
+ argument to <literal>\gedit</literal>. It is the user's responsibility
+ to ensure that the provided key selects only the rows they want to
+ change when used in a WHERE clause.
+<programlisting>
+=> <userinput>\gedit (key=language,message)</userinput>
+</programlisting>
+ </para>
+
+ <para>
+ Only SELECT statements on a single table are supported. A simple string
+ parser is used to determine the table name. If that fails, or if a
+ different base table is to be edited (perhaps the query references a
+ non-updatable view), a <literal>table</literal> argument can be provided.
+<programlisting>
+=> <userinput>\gedit (table=documents)</userinput>
+</programlisting>
+ </para>
+ </listitem>
+ </varlistentry>
+
+
<varlistentry id="app-psql-meta-command-getenv">
<term><literal>\getenv <replaceable class="parameter">psql_var</replaceable> <replaceable class="parameter">env_var</replaceable></literal></term>
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index 374c4c3ab8..896fd96b70 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -31,6 +31,7 @@ OBJS = \
copy.o \
crosstabview.o \
describe.o \
+ gedit.o \
help.o \
input.o \
large_obj.o \
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index 824900819b..1834fed907 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -36,6 +36,7 @@
#include "fe_utils/print.h"
#include "fe_utils/string_utils.h"
#include "help.h"
+#include "gedit.h"
#include "input.h"
#include "large_obj.h"
#include "libpq-fe.h"
@@ -101,6 +102,8 @@ static backslashResult process_command_g_options(char *first_option,
static backslashResult exec_command_gdesc(PsqlScanState scan_state, bool active_branch);
static backslashResult exec_command_getenv(PsqlScanState scan_state, bool active_branch,
const char *cmd);
+static backslashResult exec_command_gedit(PsqlScanState scan_state, bool active_branch,
+ PQExpBuffer query_buf, PQExpBuffer previous_buf);
static backslashResult exec_command_gexec(PsqlScanState scan_state, bool active_branch);
static backslashResult exec_command_gset(PsqlScanState scan_state, bool active_branch);
static backslashResult exec_command_help(PsqlScanState scan_state, bool active_branch);
@@ -355,6 +358,9 @@ exec_command(const char *cmd,
status = exec_command_gdesc(scan_state, active_branch);
else if (strcmp(cmd, "getenv") == 0)
status = exec_command_getenv(scan_state, active_branch, cmd);
+ else if (strcmp(cmd, "gedit") == 0)
+ status = exec_command_gedit(scan_state, active_branch, query_buf,
+ previous_buf);
else if (strcmp(cmd, "gexec") == 0)
status = exec_command_gexec(scan_state, active_branch);
else if (strcmp(cmd, "gset") == 0)
@@ -1587,6 +1593,115 @@ exec_command_getenv(PsqlScanState scan_state, bool active_branch,
return success ? PSQL_CMD_SKIP_LINE : PSQL_CMD_ERROR;
}
+/*
+ * \gedit -- send query, ask user to edit the result, and transform the diff to
+ * UPDATE commands
+ */
+static backslashResult
+exec_command_gedit(PsqlScanState scan_state, bool active_branch,
+ PQExpBuffer query_buf, PQExpBuffer previous_buf)
+{
+ backslashResult status = PSQL_CMD_SKIP_LINE;
+
+ if (active_branch)
+ {
+ char fnametmp[MAXPGPATH];
+ /* make a temp file to store result */
+#ifndef WIN32
+ const char *tmpdir = getenv("TMPDIR");
+
+ if (!tmpdir)
+ tmpdir = "/tmp";
+#else
+ char tmpdir[MAXPGPATH];
+ int ret;
+
+ ret = GetTempPath(MAXPGPATH, tmpdir);
+ if (ret == 0 || ret > MAXPGPATH)
+ {
+ pg_log_error("could not locate temporary directory: %s",
+ !ret ? strerror(errno) : "");
+ return PSQL_CMD_ERROR;
+ }
+#endif
+
+ /* save settings if not done already, then force format=json */
+ if (pset.gsavepopt == NULL)
+ pset.gsavepopt = savePsetInfo(&pset.popt);
+ pset.popt.topt.format = PRINT_JSON;
+
+ /* Consume pset options through trailing ')' ... */
+ if (! process_command_gedit_options(scan_state, active_branch))
+ goto error;
+
+ /* If query_buf is empty, recall previous query */
+ (void) copy_previous_query(query_buf, previous_buf);
+
+ /* extract table name from query if not already given in options */
+ if (! pset.gedit_table)
+ pset.gedit_table = gedit_table_name(query_buf);
+ if (! pset.gedit_table)
+ {
+ pg_log_error("\\gedit: could not determine table name from query");
+ goto error;
+ }
+
+ /* get table key */
+ if (pset.gedit_key_columns)
+ {
+ if (! gedit_check_key_columns(pset.db, query_buf->data, pset.gedit_key_columns))
+ goto error;
+ }
+ else
+ pset.gedit_key_columns = gedit_table_key_columns(pset.db, query_buf->data, pset.gedit_table);
+ if (! pset.gedit_key_columns)
+ goto error;
+
+ /*
+ * No canonicalize_path() here. EDIT.EXE run from CMD.EXE prepends the
+ * current directory to the supplied path unless we use only
+ * backslashes, so we do that.
+ */
+#ifndef WIN32
+ snprintf(fnametmp, sizeof(fnametmp), "%s%spsql.gedit.%d.json", tmpdir,
+ "/", (int) getpid());
+#else
+ snprintf(fnametmp, sizeof(fnametmp), "%s%spsql.gedit.%d.json", tmpdir,
+ "" /* trailing separator already present */ , (int) getpid());
+#endif
+
+ pset.gfname = pstrdup(fnametmp);
+ pset.gedit_flag = true;
+
+ status = PSQL_CMD_SEND;
+ }
+ else
+ ignore_slash_options(scan_state);
+
+ return status;
+
+error:
+ if (pset.gedit_table)
+ {
+ free(pset.gedit_table);
+ pset.gedit_table = NULL;
+ }
+ if (pset.gedit_key_columns)
+ {
+ char **p;
+ for (p = pset.gedit_key_columns; *p; p++)
+ free(*p);
+ free(pset.gedit_key_columns);
+ pset.gedit_key_columns = NULL;
+ }
+
+ /* Reset the query buffer as though for \r */
+ resetPQExpBuffer(query_buf);
+ psql_scan_reset(scan_state);
+
+ return PSQL_CMD_ERROR;
+}
+
/*
* \gexec -- send query and execute each field of result
*/
@@ -3929,13 +4044,13 @@ UnsyncVariables(void)
/*
- * helper for do_edit(): actually invoke the editor
+ * helper for do_edit() and gedit_build_query(): actually invoke the editor
*
* Returns true on success, false if we failed to invoke the editor or
* it returned nonzero status. (An error message is printed for failed-
* to-invoke cases, but not if the editor returns nonzero status.)
*/
-static bool
+bool
editFile(const char *fname, int lineno)
{
const char *editorName;
diff --git a/src/bin/psql/command.h b/src/bin/psql/command.h
index b41065d5a9..c3ae9958c9 100644
--- a/src/bin/psql/command.h
+++ b/src/bin/psql/command.h
@@ -46,4 +46,6 @@ extern void SyncVariables(void);
extern void UnsyncVariables(void);
+extern bool editFile(const char *fname, int lineno);
+
#endif /* COMMAND_H */
diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c
index 76e01b02a3..d2f6fdf01a 100644
--- a/src/bin/psql/common.c
+++ b/src/bin/psql/common.c
@@ -12,6 +12,8 @@
#include <math.h>
#include <pwd.h>
#include <signal.h>
+#include <sys/stat.h>
+#include <utime.h>
#ifndef WIN32
#include <unistd.h> /* for write() */
#else
@@ -83,6 +85,63 @@ openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe)
return true;
}
+
+/*
+ * editFileWithCheck -- invoke editor and check if the file was touched
+ */
+bool
+editFileWithCheck(const char *fname, int lineno, bool *edited)
+{
+ bool error = false;
+ struct stat before,
+ after;
+
+ {
+ struct utimbuf ut;
+
+ /*
+ * Try to set the file modification time of the temporary file
+ * a few seconds in the past. Otherwise, the low granularity
+ * (one second, or even worse on some filesystems) that we can
+ * portably measure with stat(2) could lead us to not
+ * recognize a modification, if the user typed very quickly.
+ *
+ * This is a rather unlikely race condition, so don't error
+ * out if the utime(2) call fails --- that would make the cure
+ * worse than the disease.
+ */
+ ut.modtime = ut.actime = time(NULL) - 2;
+ (void) utime(fname, &ut);
+ }
+
+ if (!error && stat(fname, &before) != 0)
+ {
+ pg_log_error("%s: %m", fname);
+ error = true;
+ }
+
+ /* call editor */
+ if (!error)
+ error = !editFile(fname, lineno);
+
+ if (!error && stat(fname, &after) != 0)
+ {
+ pg_log_error("%s: %m", fname);
+ error = true;
+ }
+
+ /* file was edited if the size or modification time has changed */
+ if (!error &&
+ (before.st_size != after.st_size ||
+ before.st_mtime != after.st_mtime))
+ {
+ *edited = true;
+ }
+
+ return !error;
+}
+
+
/*
* setQFout
* -- handler for -o command line option and \o command
@@ -1228,7 +1287,7 @@ sendquery_cleanup:
ResetCancelConn();
/* reset \g's output-to-filename trigger */
- if (pset.gfname)
+ if (pset.gfname && !pset.gedit_flag)
{
free(pset.gfname);
pset.gfname = NULL;
@@ -1261,6 +1320,8 @@ sendquery_cleanup:
/* reset \gdesc trigger */
pset.gdesc_flag = false;
+ /* we don't reset gedit_flag here, it's still needed in the main loop */
+
/* reset \gexec trigger */
pset.gexec_flag = false;
diff --git a/src/bin/psql/common.h b/src/bin/psql/common.h
index 6efe12274f..6e97c3ae0e 100644
--- a/src/bin/psql/common.h
+++ b/src/bin/psql/common.h
@@ -16,6 +16,7 @@
#include "libpq-fe.h"
extern bool openQueryOutputFile(const char *fname, FILE **fout, bool *is_pipe);
+extern bool editFileWithCheck(const char *fname, int lineno, bool *edited);
extern bool setQFout(const char *fname);
extern char *psql_get_variable(const char *varname, PsqlScanQuoteType quote,
diff --git a/src/bin/psql/gedit.c b/src/bin/psql/gedit.c
new file mode 100644
index 0000000000..d77d8ffb70
--- /dev/null
+++ b/src/bin/psql/gedit.c
@@ -0,0 +1,961 @@
+/*
+ * psql - the PostgreSQL interactive terminal
+ *
+ * Copyright (c) 2023, PostgreSQL Global Development Group
+ *
+ * src/bin/psql/gedit.c
+ */
+
+#include <sys/stat.h>
+#include <time.h>
+#include <utime.h>
+
+#include "postgres_fe.h"
+#include "catalog/pg_type_d.h"
+#include "command.h"
+#include "common.h"
+#include "common/jsonapi.h"
+#include "common/logging.h"
+#include "common/string.h"
+#include "lib/ilist.h"
+#include "pqexpbuffer.h"
+#include "psqlscanslash.h"
+#include "settings.h"
+
+#include "gedit.h"
+
+/*
+ * Process a single option for \gedit
+ */
+static bool
+process_command_gedit_option(char *option, char *valptr)
+{
+ bool success = false;
+
+ if (strcmp(option, "table") == 0)
+ {
+ if (valptr && *valptr)
+ {
+ if (pset.gedit_table)
+ free(pset.gedit_table);
+ pset.gedit_table = strdup(valptr);
+ success = true;
+ } else
+ pg_log_error("\\gedit: missing table name in table option");
+ }
+ else if (strcmp(option, "key") == 0)
+ {
+ if (valptr && *valptr)
+ {
+ char *p;
+ int nkeycols = 1;
+ char *saveptr = NULL;
+ int i = 0;
+ char *tok;
+
+ /* count number of tokens. Currently, this assumes no commas in table names and no quoting */
+ for (p = valptr; *p; p++)
+ {
+ if (*p == ',')
+ nkeycols++;
+ }
+
+ Assert(! pset.gedit_key_columns);
+ pset.gedit_key_columns = calloc(nkeycols + 1, sizeof(char *));
+ if (! pset.gedit_key_columns)
+ {
+ pg_log_error("\\gedit: out of memory");
+ return false;
+ }
+
+ do {
+ tok = strtok_r(saveptr ? NULL : valptr, ",", &saveptr);
+ if (tok)
+ pset.gedit_key_columns[i++] = strdup(tok);
+ } while (tok);
+ pset.gedit_key_columns[i] = NULL; /* NULL-terminate array */
+
+ success = true;
+
+ } else
+ pg_log_error("\\gedit: missing column names in key option");
+ }
+ else if (strcmp(option, "x") == 0 || strcmp(option, "expanded") == 0 || strcmp(option, "vertical") == 0)
+ {
+ success = do_pset("expanded", valptr, &pset.popt, true);
+ }
+ else
+ pg_log_error("\\gedit: unknown option \"%s\"", option);
+
+ return success;
+}
+
+/*
+ * Process parenthesized options for \gedit
+ */
+bool
+process_command_gedit_options(PsqlScanState scan_state, bool active_branch)
+{
+ bool success = true;
+ bool first_option = true;
+ bool found_r_paren = false;
+
+ do
+ {
+ char *option;
+ size_t optlen;
+
+ option = psql_scan_slash_option(scan_state,
+ OT_NORMAL, NULL, false);
+ if (!option)
+ {
+ if (active_branch && !first_option)
+ {
+ pg_log_error("\\gedit: missing right parenthesis in options");
+ success = false;
+ }
+ break;
+ }
+
+ /* Skip over '(' in first option */
+ if (first_option)
+ {
+ char *tmp;
+
+ if (option[0] != '(')
+ {
+ pg_log_error("\\gedit: missing left parenthesis in options");
+ success = false;
+ break;
+ }
+
+ tmp = strdup(option + 1);
+ free(option);
+ option = tmp;
+ first_option = false;
+ }
+
+ /* Check for terminating right paren, and remove it from string */
+ optlen = strlen(option);
+ if (optlen > 0 && option[optlen - 1] == ')')
+ {
+ option[--optlen] = '\0';
+ found_r_paren = true;
+ }
+
+ /* If there was anything besides parentheses, parse/execute it */
+ if (optlen > 0)
+ {
+ /* We can have either "name" or "name=value" */
+ char *valptr = strchr(option, '=');
+
+ if (valptr)
+ *valptr++ = '\0';
+ if (active_branch)
+ success &= process_command_gedit_option(option, valptr);
+ }
+
+ free(option);
+ } while (!found_r_paren);
+
+ return success;
+}
+
+/*
+ * Extract table name from query
+ */
+char *
+gedit_table_name(PQExpBuffer query_buf)
+{
+ char *query = strdup(query_buf->data);
+ char *tok;
+ char *saveptr = NULL;
+ char *res = NULL;
+
+ do {
+ tok = strtok_r(saveptr ? NULL : query, " \t", &saveptr);
+ if (tok && (strcasecmp(tok, "FROM") == 0 || strcasecmp(tok, "TABLE") == 0))
+ {
+ if ((res = strtok_r(NULL, " \t", &saveptr))) /* next word or NULL */
+ {
+ int l = strlen(res);
+ Assert (l > 0);
+
+ res = strdup(res);
+ if (res[l-1] == ';') /* strip trailing ; */
+ res[l-1] = '\0';
+ }
+ break;
+ }
+ } while (tok != NULL);
+
+ free(query);
+
+ return res;
+}
+
+/*
+ * Append one array-quoted value to a buffer.
+ */
+static void
+PQExpBufferAppendArrayValue(PQExpBuffer buf, const char *value)
+{
+ const char *p;
+ appendPQExpBufferChar(buf, '"');
+ for (p = value; *p; p++)
+ {
+ if (*p == '"')
+ appendPQExpBufferStr(buf, "\\\"");
+ else if (*p == '\\')
+ appendPQExpBufferStr(buf, "\\\\");
+ else
+ appendPQExpBufferChar(buf, *p);
+ }
+ appendPQExpBufferChar(buf, '"');
+}
+
+/*
+ * Describe a query, and return the list of columns as an array literal.
+ */
+static bool
+gedit_get_query_columns(PGconn *conn, const char *query, PQExpBuffer query_columns)
+{
+ PGresult *result = PQprepare(conn, "", query, 0, NULL);
+ if (PQresultStatus(result) != PGRES_COMMAND_OK)
+ {
+ pg_log_error("%s", PQerrorMessage(pset.db));
+ PQclear(result);
+ return false;
+ }
+ PQclear(result);
+
+ result = PQdescribePrepared(pset.db, "");
+ if (PQresultStatus(result) == PGRES_COMMAND_OK)
+ {
+ appendPQExpBufferChar(query_columns, '{');
+ if (PQnfields(result) > 0)
+ {
+ int i;
+ for (i = 0; i < PQnfields(result); i++)
+ {
+ char *name;
+ name = PQfname(result, i);
+ if (i > 0)
+ appendPQExpBufferChar(query_columns, ',');
+ PQExpBufferAppendArrayValue(query_columns, name);
+ }
+ }
+ appendPQExpBufferChar(query_columns, '}');
+ }
+
+ return true;
+}
+
+/*
+ * Given a query and a table, get a table key that is contained in the query
+ * columns.
+ */
+char **
+gedit_table_key_columns(PGconn *conn, const char *query, const char *table)
+{
+ PQExpBuffer query_columns = createPQExpBuffer();
+ char key_query[] =
+ "WITH keys AS (SELECT array_agg(attname ORDER BY ordinality) keycols, indisprimary "
+ "FROM pg_index, unnest(indkey) with ordinality u(keycol) "
+ "JOIN pg_attribute ON keycol = attnum "
+ "WHERE indrelid = $1::regclass AND attrelid = $1::regclass AND (indisprimary or indisunique) "
+ "GROUP BY indexrelid), "
+ "key AS (SELECT keycols FROM keys WHERE keycols <@ $2 " /* all keys contained in the query columns */
+ "ORDER BY indisprimary DESC LIMIT 1) " /* prefer primary key over unique indexes */
+ "SELECT unnest(keycols) FROM key;";
+ Oid types[] = { NAMEOID, NAMEARRAYOID };
+ const char *values[] = { table, NULL };
+ char **gedit_key_columns = NULL;
+ PGresult *res;
+
+ if (! gedit_get_query_columns(pset.db, query, query_columns))
+ {
+ destroyPQExpBuffer(query_columns);
+ return NULL;
+ }
+ values[1] = query_columns->data;
+ res = PQexecParams(conn, key_query, 2, types, values, NULL, NULL, 0);
+ destroyPQExpBuffer(query_columns);
+
+ if (PQresultStatus(res) == PGRES_TUPLES_OK)
+ {
+ int nkeycols = PQntuples(res);
+ if (nkeycols > 0)
+ {
+ int i;
+ gedit_key_columns = calloc(nkeycols + 1, sizeof(char *));
+ if (! gedit_key_columns)
+ {
+ pg_log_error("\\gedit: Out of memory.");
+ return NULL;
+ }
+
+ for (i = 0; i < nkeycols; i++)
+ {
+ gedit_key_columns[i] = strdup(PQgetvalue(res, i, 0));
+ }
+ }
+ else
+ {
+ pg_log_error("\\gedit: no key of table \"%s\" is contained in the returned query columns.", table);
+ pg_log_error_hint("Select more columns or manually specify a key using \\gedit (key=...)");
+ }
+ }
+ else if (PQresultStatus(res) == PGRES_FATAL_ERROR)
+ {
+ char *val = PQresultErrorField(res, PG_DIAG_SQLSTATE);
+ if (strcmp(val, "42P01") == 0)
+ pg_log_error("\\gedit: table \"%s\" does not exist", table);
+ else
+ pg_log_error("\\gedit: error while retrieving key columns of table \"%s\": %s",
+ table, PQerrorMessage(pset.db));
+ }
+ PQclear(res);
+
+ return gedit_key_columns;
+}
+
+/*
+ * Check if a query contains the given columns.
+ */
+bool
+gedit_check_key_columns(PGconn *conn, const char *query, char **key_columns)
+{
+ char **key_column;
+ PGresult *result = PQprepare(conn, "", query, 0, NULL);
+
+ if (PQresultStatus(result) != PGRES_COMMAND_OK)
+ {
+ pg_log_error("\\gedit: %s", PQerrorMessage(conn));
+ PQclear(result);
+ return false;
+ }
+ PQclear(result);
+
+ result = PQdescribePrepared(conn, "");
+ if (PQresultStatus(result) != PGRES_COMMAND_OK)
+ {
+ pg_log_error("\\gedit: %s", PQerrorMessage(conn));
+ PQclear(result);
+ return false;
+ }
+
+ if (PQnfields(result) == 0)
+ {
+ pg_log_error("\\gedit: query has no columns");
+ PQclear(result);
+ return false;
+ }
+
+ for (key_column = key_columns; *key_column; key_column++)
+ {
+ bool found = false;
+ int i;
+ for (i = 0; i < PQnfields(result); i++)
+ {
+ if (strcmp(*key_column, PQfname(result, i)) == 0)
+ {
+ found = true;
+ break;
+ }
+ }
+
+ if (! found)
+ {
+ pg_log_error("\\gedit: key column \"%s\" not found in query", *key_column);
+ PQclear(result);
+ return false;
+ }
+ }
+
+ PQclear(result);
+ return true;
+}
+
+/* JSON parsing */
+
+typedef struct
+{
+ char *name;
+ char *value;
+ slist_node cell_node;
+} Cell;
+
+static void
+cell_free (Cell *cell1)
+{
+ if (cell1->name)
+ free(cell1->name);
+ if (cell1->value)
+ free(cell1->value);
+ free(cell1);
+}
+
+typedef struct
+{
+ slist_head cells;
+ slist_node row_node;
+} Row;
+
+static void
+row_free (Row *row1)
+{
+ slist_mutable_iter cell_iter1;
+
+ slist_foreach_modify(cell_iter1, &row1->cells)
+ {
+ Cell *cell1 = slist_container(Cell, cell_node, cell_iter1.cur);
+ cell_free(cell1);
+ }
+
+ free(row1);
+}
+
+static void
+data_free (slist_head *data)
+{
+ slist_mutable_iter row_iter1;
+
+ slist_foreach_modify(row_iter1, data)
+ {
+ Row *row1 = slist_container(Row, row_node, row_iter1.cur);
+ row_free(row1);
+ }
+}
+
+typedef enum
+{
+ GEDIT_JSON_TOP,
+ GEDIT_JSON_ARRAY,
+ GEDIT_JSON_OBJECT,
+} gedit_context_type;
+
+typedef struct
+{
+ gedit_context_type context;
+ char *fieldname;
+ slist_head *data;
+ Row *current_row;
+ Cell *current_cell;
+} gedit_parse_state;
+
+static JsonParseErrorType
+gedit_array_start_action(void *state)
+{
+ gedit_parse_state *pstate = state;
+
+ if (pstate->context != GEDIT_JSON_TOP)
+ {
+ pg_log_error("\\gedit: arrays are only permitted as the top-level syntax element");
+ return JSON_SEM_ACTION_FAILED;
+ }
+ pstate->context = GEDIT_JSON_ARRAY;
+
+ return JSON_SUCCESS;
+}
+
+static JsonParseErrorType
+gedit_object_start_action(void *state)
+{
+ gedit_parse_state *pstate = state;
+ Row *row = calloc(sizeof(Row), 1);
+
+ if (pstate->context != GEDIT_JSON_ARRAY)
+ {
+ pg_log_error("\\gedit: objects are only permitted inside the top-level array");
+ return JSON_SEM_ACTION_FAILED;
+ }
+ pstate->context = GEDIT_JSON_OBJECT;
+
+ if(slist_is_empty(pstate->data))
+ slist_push_head(pstate->data, &row->row_node);
+ else
+ slist_insert_after(&pstate->current_row->row_node, &row->row_node);
+ pstate->current_row = row;
+
+ return JSON_SUCCESS;
+}
+
+static JsonParseErrorType
+gedit_object_end_action(void *state)
+{
+ gedit_parse_state *pstate = state;
+ pstate->context = GEDIT_JSON_ARRAY;
+ return JSON_SUCCESS;
+}
+
+static JsonParseErrorType
+gedit_object_field_start_action (void *state, char *fname, bool isnull)
+{
+ gedit_parse_state *pstate = state;
+ Cell *cell = calloc(sizeof(Cell), 1);
+
+ cell->name = fname;
+
+ if(slist_is_empty(&pstate->current_row->cells))
+ slist_push_head(&pstate->current_row->cells, &cell->cell_node);
+ else
+ slist_insert_after(&pstate->current_cell->cell_node, &cell->cell_node);
+ pstate->current_cell = cell;
+
+ return JSON_SUCCESS;
+}
+
+static JsonParseErrorType
+gedit_scalar_action(void *state, char *token, JsonTokenType tokentype)
+{
+ gedit_parse_state *pstate = state;
+ if (pstate->context != GEDIT_JSON_OBJECT)
+ {
+ pfree(token);
+ pg_log_error("\\gedit: scalars are only permitted inside objects");
+ return JSON_SEM_ACTION_FAILED;
+ }
+
+ /* when token "null", leave value NULL */
+ if (tokentype == JSON_TOKEN_NULL)
+ {
+ pfree(token);
+ pstate->current_cell->value = NULL;
+ }
+ else
+ pstate->current_cell->value = token;
+
+ return JSON_SUCCESS;
+}
+
+static bool
+read_json_file(const char *fname, int *line_number, slist_head *data)
+{
+ bool success = false;
+ FILE *stream = NULL;
+ char line[1024];
+ PQExpBuffer jsondata = createPQExpBuffer();
+
+ gedit_parse_state parse_state = {
+ .data = data
+ };
+ JsonSemAction geditSemAction = {
+ .semstate = &parse_state,
+ .array_start = gedit_array_start_action,
+ .object_start = gedit_object_start_action,
+ .object_end = gedit_object_end_action,
+ .object_field_start = gedit_object_field_start_action,
+ .scalar = gedit_scalar_action,
+ };
+ JsonLexContext jsonlexer;
+ JsonParseErrorType parseresult;
+
+ if (PQExpBufferBroken(jsondata))
+ {
+ pg_log_error("PQresultErrorField: out of memory");
+ return false;
+ }
+
+ if (!(stream = fopen(fname, PG_BINARY_R)))
+ {
+ pg_log_error("%s: %m", fname);
+ goto error;
+ }
+
+ while (fgets(line, sizeof(line), stream) != NULL)
+ appendPQExpBufferStr(jsondata, line);
+
+ if (ferror(stream))
+ {
+ pg_log_error("%s: %m", fname);
+ goto error;
+ }
+
+ makeJsonLexContextCstringLen(&jsonlexer, jsondata->data, jsondata->len, pset.encoding, true);
+ parseresult = pg_parse_json(&jsonlexer, &geditSemAction);
+ if (line_number)
+ *line_number = jsonlexer.line_number;
+
+ success = (parseresult == JSON_SUCCESS);
+
+error:
+ destroyPQExpBuffer(jsondata);
+ if (stream)
+ fclose(stream);
+
+ return success;
+}
+
+/* main code */
+
+/* NULL-aware variant of strcmp() */
+static bool
+str_distinct(char *a, char *b)
+{
+ if ((a == NULL) && (b == NULL))
+ return false;
+ if ((a == NULL) || (b == NULL))
+ return true;
+ return strcmp(a, b) != 0;
+}
+
+/*
+ * Check if value consists entirely of alphanumeric chars and the first
+ * character is not a digit.
+ */
+static bool
+identifier_needs_no_quoting(const char *value)
+{
+ return strspn(value, "abcdefghijklmnopqrstuvwxyz_0123456789") == strlen(value) &&
+ strspn(value, "0123456789") == 0;
+}
+
+/* NULL-aware variant of quote_literal */
+static void
+quote_literal_or_null(PQExpBuffer query_buf, char *value, bool as_ident)
+{
+ char *p;
+
+ if (!value)
+ {
+ appendPQExpBufferStr(query_buf, "NULL");
+ return;
+ }
+
+ if (as_ident)
+ {
+ if (identifier_needs_no_quoting(value)) /* skip quoting of simple names */
+ appendPQExpBufferStr(query_buf, value);
+ else
+ {
+ p = PQescapeIdentifier(pset.db, value, strlen(value));
+ appendPQExpBufferStr(query_buf, p);
+ free(p);
+ }
+ }
+ else
+ {
+ p = PQescapeLiteral(pset.db, value, strlen(value));
+ appendPQExpBufferStr(query_buf, p);
+ free(p);
+ }
+}
+
+static Row *
+find_matching_row(Row *row1, slist_head *data2, PQExpBuffer where_clause)
+{
+ slist_mutable_iter row_iter2;
+
+ slist_foreach_modify(row_iter2, data2)
+ {
+ Row *row2 = slist_container(Row, row_node, row_iter2.cur);
+ char **key_column;
+
+ for (key_column = pset.gedit_key_columns; *key_column; key_column++)
+ {
+ slist_iter cell_iter1;
+ Cell *cell1;
+
+ slist_foreach(cell_iter1, &row1->cells)
+ {
+ slist_iter cell_iter2;
+ cell1 = slist_container(Cell, cell_node, cell_iter1.cur);
+
+ if (strcmp(*key_column, cell1->name) != 0)
+ continue; /* look at next column */
+
+ slist_foreach(cell_iter2, &row2->cells)
+ {
+ Cell *cell2 = slist_container(Cell, cell_node, cell_iter2.cur);
+ if (strcmp(*key_column, cell2->name) != 0)
+ continue; /* look at next column */
+
+ if (str_distinct(cell1->value, cell2->value) != 0)
+ goto next_row; /* key column value doesn't match, look at next row */
+
+ /* matching column found */
+ if (key_column == pset.gedit_key_columns) /* first key column */
+ printfPQExpBuffer(where_clause, " WHERE ");
+ else
+ appendPQExpBufferStr(where_clause, " AND ");
+ quote_literal_or_null(where_clause, cell1->name, true);
+ appendPQExpBufferStr(where_clause, " = ");
+ quote_literal_or_null(where_clause, cell1->value, false);
+
+ goto next_key; /* look at next key column */
+ }
+ /* key column not found in edited result; ignore row*/
+ goto next_row;
+ }
+ /* should not happen, report error and move on */
+ pg_log_error("\\gedit: key column \"%s\" not found in original result", *key_column);
+ return NULL;
+
+next_key:
+ }
+
+ /* all columns matched */
+ slist_delete_current(&row_iter2); /* caller needs to free row */
+ appendPQExpBufferStr(where_clause, ";");
+ return row2;
+
+next_row:
+ }
+
+ return NULL;
+}
+
+static void
+generate_where_clause(PQExpBuffer query_buf, Row *row1)
+{
+ char **key_column;
+
+ appendPQExpBuffer(query_buf, " WHERE ");
+
+ for (key_column = pset.gedit_key_columns; *key_column; key_column++)
+ {
+ slist_iter cell_iter1;
+
+ slist_foreach(cell_iter1, &row1->cells)
+ {
+ Cell *cell1 = slist_container(Cell, cell_node, cell_iter1.cur);
+
+ if (strcmp(*key_column, cell1->name) == 0)
+ {
+ if (key_column != pset.gedit_key_columns) /* not the first key column */
+ appendPQExpBufferStr(query_buf, " AND ");
+ quote_literal_or_null(query_buf, cell1->name, true);
+ appendPQExpBufferStr(query_buf, " = ");
+ quote_literal_or_null(query_buf, cell1->value, false);
+ }
+ }
+ }
+
+ appendPQExpBufferStr(query_buf, ";");
+}
+
+#define APPEND_NEWLINE \
+ if (cmd_started) \
+ appendPQExpBufferChar(query_buf, '\n'); \
+ cmd_started = true
+
+#define APPEND_COMMA \
+ if (list_started) \
+ appendPQExpBufferStr(query_buf, ", "); \
+ list_started = true
+
+static bool
+generate_update_commands(PQExpBuffer query_buf, slist_head *data1, slist_head *data2)
+{
+ slist_iter row_iter1, row_iter2;
+ bool cmd_started = false;
+ PQExpBuffer where_clause = createPQExpBuffer();
+
+ /* look for differing rows, produce UPDATE and DELETE statements */
+ slist_foreach(row_iter1, data1)
+ {
+ Row *row1 = slist_container(Row, row_node, row_iter1.cur);
+ Row *row2 = find_matching_row(row1, data2, where_clause);
+ slist_mutable_iter cell_iter1, cell_iter2;
+ bool list_started = false;
+
+ /* no matching row was found, DELETE original row */
+ if (!row2)
+ {
+ APPEND_NEWLINE;
+ appendPQExpBuffer(query_buf, "DELETE FROM %s", pset.gedit_table);
+ generate_where_clause(query_buf, row1);
+ continue;
+ }
+
+ /* loop over both rows */
+ slist_foreach_modify(cell_iter1, &row1->cells)
+ {
+ Cell *cell1 = slist_container(Cell, cell_node, cell_iter1.cur);
+
+ slist_foreach_modify(cell_iter2, &row2->cells)
+ {
+ Cell *cell2 = slist_container(Cell, cell_node, cell_iter2.cur);
+
+ if (strcmp(cell1->name, cell2->name) != 0)
+ continue; /* cell names do not match */
+
+ if (str_distinct(cell1->value, cell2->value))
+ {
+ if (!list_started)
+ {
+ APPEND_NEWLINE;
+ appendPQExpBuffer(query_buf, "UPDATE %s SET ", pset.gedit_table);
+ }
+ APPEND_COMMA;
+ quote_literal_or_null(query_buf, cell2->name, true);
+ appendPQExpBufferStr(query_buf, " = ");
+ quote_literal_or_null(query_buf, cell2->value, false);
+ }
+
+ /* no need to look again at these cells, delete them */
+ slist_delete_current(&cell_iter1);
+ cell_free(cell1);
+ slist_delete_current(&cell_iter2);
+ cell_free(cell2);
+
+ break;
+ }
+ }
+
+ /* any cells left in row2 were manually added, include them in the UPDATE */
+ slist_foreach_modify(cell_iter2, &row2->cells)
+ {
+ Cell *cell2 = slist_container(Cell, cell_node, cell_iter2.cur);
+ if (list_started)
+ appendPQExpBufferStr(query_buf, ",");
+ else
+ {
+ APPEND_NEWLINE;
+ appendPQExpBuffer(query_buf, "UPDATE %s SET", pset.gedit_table);
+ list_started = true;
+ }
+ appendPQExpBufferStr(query_buf, " ");
+ quote_literal_or_null(query_buf, cell2->name, true);
+ appendPQExpBufferStr(query_buf, " = ");
+ quote_literal_or_null(query_buf, cell2->value, false);
+
+ slist_delete_current(&cell_iter2);
+ cell_free(cell2);
+ }
+ /*
+ * Any cells left in row1 were deleted while editing, ignore them.
+ * (We could set them NULL, but let's better require the user do that
+ * explicitly.)
+ */
+
+ if (list_started)
+ appendPQExpBufferStr(query_buf, where_clause->data);
+
+ row_free(row2);
+ }
+
+ /* all rows left in data2 are newly added, produce INSERT statements */
+ slist_foreach(row_iter2, data2)
+ {
+ Row *row2 = slist_container(Row, row_node, row_iter2.cur);
+ slist_iter cell_iter2;
+ bool list_started = false;
+
+ APPEND_NEWLINE;
+ appendPQExpBuffer(query_buf, "INSERT INTO %s (", pset.gedit_table);
+
+ slist_foreach(cell_iter2, &row2->cells)
+ {
+ Cell *cell2 = slist_container(Cell, cell_node, cell_iter2.cur);
+ APPEND_COMMA;
+ quote_literal_or_null(query_buf, cell2->name, true);
+ }
+
+ appendPQExpBuffer(query_buf, ") VALUES (");
+ list_started = false;
+
+ slist_foreach(cell_iter2, &row2->cells)
+ {
+ Cell *cell2 = slist_container(Cell, cell_node, cell_iter2.cur);
+ APPEND_COMMA;
+ quote_literal_or_null(query_buf, cell2->value, false);
+ }
+
+ appendPQExpBuffer(query_buf, ");");
+ }
+
+ if (cmd_started)
+ printf("%s\n", query_buf->data);
+
+ destroyPQExpBuffer(where_clause);
+ return cmd_started;
+}
+
+/*
+ * gedit_edit_and_build_query -- handler for \gedit
+ *
+ * This function reads the query result in JSON format and calls an editor. If
+ * the file was edited, it reads the file contents again, and puts the diff
+ * into the query buffer as INSERT/UPDATE/DELETE commands.
+ */
+backslashResult
+gedit_edit_and_build_query(const char *fname, PQExpBuffer query_buf)
+{
+ bool error = false;
+ bool edited = false;
+ int line_number = 0;
+ slist_head data1 = {0};
+ slist_head data2 = {0};
+ backslashResult result = PSQL_CMD_SKIP_LINE;
+
+ if (!read_json_file(fname, NULL, &data1))
+ {
+ pg_log_error("%s: invalid json data even before editing", fname);
+ error = true;
+ }
+
+ while (!error)
+ {
+ char *retry;
+ bool do_retry;
+
+ /* call editor */
+ edited = false;
+ error = !editFileWithCheck(fname, line_number, &edited);
+
+ if (!edited)
+ break;
+ if (error)
+ {
+ pg_log_error("%s: %m", fname);
+ continue;
+ }
+ if (read_json_file(fname, &line_number, &data2))
+ break;
+
+ /* loop until edit produces valid json */
+ retry = simple_prompt(_("Edit again? [y] "), true);
+ if (!retry)
+ {
+ pg_log_error("simple_prompt: out of memory");
+ error = true;
+ break;
+ }
+ do_retry = (*retry == '\0' || *retry == 'y');
+ free(retry);
+ if (!do_retry)
+ {
+ error = true;
+ break;
+ }
+ }
+
+ /* remove temp file */
+ if (remove(fname) == -1)
+ {
+ pg_log_error("%s: %m", fname);
+ error = true;
+ }
+
+ if (error)
+ result = PSQL_CMD_ERROR;
+ else if (edited)
+ {
+ /* generate command to run from diffing data1 and data2 */
+ if (generate_update_commands(query_buf, &data1, &data2))
+ result = PSQL_CMD_NEWEDIT;
+ }
+ if (result == PSQL_CMD_SKIP_LINE) /* either file wasn't edited, or editing didn't change data */
+ pg_log_info("\\gedit: no changes");
+
+ if (data1.head.next)
+ data_free(&data1);
+ if (data2.head.next)
+ data_free(&data2);
+
+ return result;
+}
+
+
diff --git a/src/bin/psql/gedit.h b/src/bin/psql/gedit.h
new file mode 100644
index 0000000000..15c9c2e5c5
--- /dev/null
+++ b/src/bin/psql/gedit.h
@@ -0,0 +1,17 @@
+/*
+ * psql - the PostgreSQL interactive terminal
+ *
+ * Copyright (c) 2023, PostgreSQL Global Development Group
+ *
+ * src/bin/psql/gedit.h
+ */
+#ifndef GEDIT_H
+#define GEDIT_H
+
+extern bool process_command_gedit_options(PsqlScanState scan_state, bool active_branch);
+extern char *gedit_table_name(PQExpBuffer query_buf);
+extern char **gedit_table_key_columns(PGconn *conn, const char *query, const char *table);
+extern bool gedit_check_key_columns(PGconn *conn, const char *query, char **key_columns);
+extern backslashResult gedit_edit_and_build_query(const char *filename_arg, PQExpBuffer query_buf);
+
+#endif
diff --git a/src/bin/psql/help.c b/src/bin/psql/help.c
index 3950ffc2c6..ec5f9c39cb 100644
--- a/src/bin/psql/help.c
+++ b/src/bin/psql/help.c
@@ -172,6 +172,8 @@ slashUsage(unsigned short int pager)
HELP0(" \\g [(OPTIONS)] [FILE] execute query (and send result to file or |pipe);\n"
" \\g with no arguments is equivalent to a semicolon\n");
HELP0(" \\gdesc describe result of query, without executing it\n");
+ HELP0(" \\gedit execute query, edit the result with an external editor,\n");
+ HELP0(" and send back the changes as UPDATE statements\n");
HELP0(" \\gexec execute query, then execute each value in its result\n");
HELP0(" \\gset [PREFIX] execute query and store result in psql variables\n");
HELP0(" \\gx [(OPTIONS)] [FILE] as \\g, but forces expanded output mode\n");
diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c
index c24e38d988..739f52eea7 100644
--- a/src/bin/psql/mainloop.c
+++ b/src/bin/psql/mainloop.c
@@ -10,6 +10,7 @@
#include "command.h"
#include "common.h"
#include "common/logging.h"
+#include "gedit.h"
#include "input.h"
#include "mainloop.h"
#include "mb/pg_wchar.h"
@@ -525,8 +526,29 @@ MainLoop(FILE *source)
/* flush any paren nesting info after forced send */
psql_scan_reset(scan_state);
+
+ /* edit query result when gedit was requested */
+ if (pset.gedit_flag)
+ {
+ char **p;
+
+ if (success)
+ slashCmdStatus = gedit_edit_and_build_query(pset.gfname, query_buf);
+
+ /* free all gedit data */
+ pset.gedit_flag = false;
+ free(pset.gfname);
+ pset.gfname = NULL;
+ free(pset.gedit_table);
+ pset.gedit_table = NULL;
+ for (p = pset.gedit_key_columns; *p; p++)
+ free(*p);
+ free(pset.gedit_key_columns);
+ pset.gedit_key_columns = NULL;
+ }
}
- else if (slashCmdStatus == PSQL_CMD_NEWEDIT)
+ /* fall-through to NEWEDIT when \gedit */
+ if (slashCmdStatus == PSQL_CMD_NEWEDIT)
{
/* should not see this in inactive branch */
Assert(conditional_active(cond_stack));
diff --git a/src/bin/psql/meson.build b/src/bin/psql/meson.build
index f3a6392138..839bc48390 100644
--- a/src/bin/psql/meson.build
+++ b/src/bin/psql/meson.build
@@ -6,6 +6,7 @@ psql_sources = files(
'copy.c',
'crosstabview.c',
'describe.c',
+ 'gedit.c',
'help.c',
'input.c',
'large_obj.c',
@@ -67,6 +68,7 @@ tests += {
't/001_basic.pl',
't/010_tab_completion.pl',
't/020_cancel.pl',
+ 't/030_gedit.pl',
],
},
}
diff --git a/src/bin/psql/settings.h b/src/bin/psql/settings.h
index 505f99d8e4..b426b2f69a 100644
--- a/src/bin/psql/settings.h
+++ b/src/bin/psql/settings.h
@@ -95,6 +95,9 @@ typedef struct _psqlSettings
char *gset_prefix; /* one-shot prefix argument for \gset */
bool gdesc_flag; /* one-shot request to describe query result */
+ bool gedit_flag; /* one-shot request to edit query result */
+ char *gedit_table; /* table used with \gedit */
+ char **gedit_key_columns; /* key columns used with \gedit */
bool gexec_flag; /* one-shot request to execute query result */
bool bind_flag; /* one-shot request to use extended query
* protocol */
diff --git a/src/bin/psql/t/030_gedit.pl b/src/bin/psql/t/030_gedit.pl
new file mode 100644
index 0000000000..5819109322
--- /dev/null
+++ b/src/bin/psql/t/030_gedit.pl
@@ -0,0 +1,106 @@
+
+# Copyright (c) 2023, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Execute a psql command and check its exit code and output on stdout and stderr.
+sub psql_like
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+
+ my ($node, $sql, $expected_exitcode, $expected_stdout, $expected_stderr, $test_name) = @_;
+
+ my ($ret, $stdout, $stderr) = $node->psql('postgres', $sql);
+
+ is($ret, $expected_exitcode, "$test_name: exit code $expected_exitcode");
+ like($stdout, $expected_stdout, "$test_name: stdout matches");
+ like($stderr, $expected_stderr, "$test_name: stderr matches");
+
+ return;
+}
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->start;
+
+# prepare test table (with a non-standard primary key layout)
+psql_like($node, "create table foo (t text, a int, b int, primary key (b, a))",
+ 0, qr/^$/, qr/^$/, 'create table');
+psql_like($node, "insert into foo values ('foo', 1, 2), ('bar', 3, 4);",
+ 0, qr/^$/, qr/^$/, 'insert data');
+
+# normal cases
+$ENV{EDITOR} = 'true';
+psql_like($node, 'select * from foo \gedit',
+ 0, qr/^$/, qr/\\gedit: no changes/, 'gedit without saving');
+
+$ENV{EDITOR} = 'touch';
+psql_like($node, 'select * from foo \gedit',
+ 0, qr/^$/, qr/\\gedit: no changes/, 'gedit without change');
+
+$ENV{EDITOR} = 'sed -i -e s/bar/moo/';
+psql_like($node, 'select * from foo \gedit',
+ 0,
+ qr/UPDATE foo SET t = 'moo' WHERE b = '4' AND a = '3';/,
+ qr/^$/, 'gedit with UPDATE');
+
+$ENV{EDITOR} = "sed -i -e '1a{\"a\":6,\"b\":7},'";
+psql_like($node, "select * from foo;\n\\gedit",
+ 0,
+ qr/foo\|1\|2\nmoo\|3\|4\nINSERT INTO foo \(a, b\) VALUES \('6', '7'\);/,
+ qr/^$/, 'gedit as separate command, with INSERT');
+
+$ENV{EDITOR} = 'sed -i -e s/1/5/';
+psql_like($node, 'select * from foo \gedit',
+ 0,
+ qr/DELETE FROM foo WHERE b = '2' AND a = '1';.*INSERT INTO foo \(t, a, b\) VALUES \('foo', '5', '2'\);/s,
+ qr/^$/, 'gedit with INSERT and DELETE');
+
+$ENV{EDITOR} = 'sed -i -e /moo/d';
+psql_like($node, 'select * from foo \gedit',
+ 0,
+ qr/DELETE FROM foo WHERE b = '4' AND a = '3';/,
+ qr/^$/, 'gedit with DELETE');
+
+$ENV{EDITOR} = 'sed -i -e s/foo/baz/';
+psql_like($node, 'select * from foo \gedit (key=b)',
+ 0,
+ qr/UPDATE foo SET t = 'baz' WHERE b = '2';/,
+ qr/^$/, 'gedit with custom key');
+
+# error cases
+$ENV{EDITOR} = 'touch';
+psql_like($node, "select blub \\gedit",
+ 3, qr/^$/, qr/\\gedit: could not determine table name from query/, 'no FROM in query');
+
+psql_like($node, "blub from blub \\gedit",
+ 3, qr/^$/, qr/ERROR: syntax error at or near "blub"/, 'syntax error reported normally');
+
+psql_like($node, "select * from blub \\gedit",
+ 3, qr/^$/, qr/ERROR: relation "blub" does not exist/, 'gedit on missing table');
+
+psql_like($node, "select * from blub \\gedit (bla)",
+ 3, qr/^$/, qr/\\gedit: unknown option "bla"/, 'unknown gedit option');
+
+psql_like($node, "select * from blub \\gedit (blub=moo)",
+ 3, qr/^$/, qr/\\gedit: unknown option "blub"/, 'unknown gedit option with value');
+
+psql_like($node, 'select * from foo \gedit (table=bar)',
+ 3, qr/^$/, qr/\\gedit: table "bar" does not exist/, 'gedit with custom table name');
+
+psql_like($node, 'select a from foo \gedit',
+ 3, qr/^$/, qr/\\gedit: no key of table "foo" is contained in the returned query columns/,
+ 'key missing in query');
+
+psql_like($node, 'select a from foo \gedit (key=a,b,c)',
+ 3, qr/^$/, qr/\\gedit: key column "b" not found in query/,
+ 'custom key missing in query');
+
+$node->stop;
+
+done_testing();
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index f4ca4d0013..d3bf835155 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -1725,7 +1725,7 @@ psql_completion(const char *text, int start, int end)
"\\echo", "\\edit", "\\ef", "\\elif", "\\else", "\\encoding",
"\\endif", "\\errverbose", "\\ev",
"\\f",
- "\\g", "\\gdesc", "\\getenv", "\\gexec", "\\gset", "\\gx",
+ "\\g", "\\gdesc", "\\getenv", "\\gedit", "\\gexec", "\\gset", "\\gx",
"\\help", "\\html",
"\\if", "\\include", "\\include_relative", "\\ir",
"\\list", "\\lo_import", "\\lo_export", "\\lo_list", "\\lo_unlink",
--
2.43.0
Hi
po 22. 1. 2024 v 16:06 odesílatel Christoph Berg <myon@debian.org> napsal:
Assuming a SELECT statement reading from a single table, it is quite an
effort to transform that statement to an UPDATE statement on that table,
perhaps to fix a typo that the user has spotted in the query result.First, the general syntax is not the same with the order of syntax
elements changed. Then the row in question needs to be pinned down by
the primary key, requiring cut-and-paste of the PK columns. Furthermore,
the value to be updated needs to be put into the command, with proper
quoting. If the original value spans multiple line, copy-and-pasting it
for editing is especially tedious.Suppose the following query where we spot a typo in the 2nd message:
=# select id, language, message from messages where language = 'en';
id | language | message
1 | en | Good morning
2 | en | Hello warldThe query needs to be transformed into this update:
=# update messages set message = 'Hello world' where id = 2;
This patch automates the tedious parts by opening the query result in a
editor in JSON format, where the user can edit the data. On closing the
editor, the JSON data is read back, and the differences are sent as
UPDATE commands. New rows are INSERTed, and deleted rows are DELETEd.=# select id, language, message from messages where language = 'en' \gedit
An editor opens:
[
{ "id": 1, "language": "en", "message": "Good morning" },
{ "id": 2, "language": "en", "message": "Hello warld" }
]Let's fix the typo and save the file:
[
{ "id": 1, "language": "en", "message": "Good morning" },
{ "id": 2, "language": "en", "message": "Hello world" }
]
UPDATE messages SET message = 'Hello world' WHERE id = '2';
UPDATE 1In this example, typing "WHERE id = 2" would not be too hard, but the
primary key might be a composite key, with complex non-numeric values.
This is supported as well.If expanded mode (\x) is enabled, \gedit will use the expanded JSON
format, best suitable for long values.This patch requires the "psql JSON output format" patch.
Introduction of \gedit is interesting idea, but in this form it looks too
magic
a) why the data are in JSON format, that is not native for psql (minimally
now)
b) the implicit transformation to UPDATEs and the next evaluation can be
pretty dangerous.
The concept of proposed feature is interesting, but the name \gedit is too
generic, maybe too less descriptive for this purpose
Maybe \geditupdates can be better - but still it can be dangerous and slow
(without limits)
In the end I am not sure if I like it or dislike it. Looks dangerous. I can
imagine possible damage when some people will see vi first time and will
try to finish vi, but in this command, it will be transformed to executed
UPDATEs. More generating UPDATEs without knowledge of table structure
(knowledge of PK) can be issue (and possibly dangerous too), and you cannot
to recognize PK from result of SELECT (Everywhere PK is not "id" and it is
not one column).
Regards
Pavel
Show quoted text
Christoph
Pavel Stehule <pavel.stehule@gmail.com> writes:
po 22. 1. 2024 v 16:06 odesílatel Christoph Berg <myon@debian.org> napsal:
This patch automates the tedious parts by opening the query result in a
editor in JSON format, where the user can edit the data. On closing the
editor, the JSON data is read back, and the differences are sent as
UPDATE commands. New rows are INSERTed, and deleted rows are DELETEd.
Introduction of \gedit is interesting idea, but in this form it looks too
magic
Yeah, I don't like it either --- it feels like something that belongs
in an ETL tool not psql. The sheer size of the patch shows how far
afield it is from anything that psql already does, necessitating
writing tons of stuff that was not there before. The bits that try
to parse the query to get necessary information seem particularly
half-baked.
In the end I am not sure if I like it or dislike it. Looks dangerous. I can
imagine possible damage when some people will see vi first time and will
try to finish vi, but in this command, it will be transformed to executed
UPDATEs.
Yup -- you'd certainly want some way of confirming that you actually
want the changes applied. Our existing edit commands load the edited
string back into the query buffer, where you can \r it if you don't
want to run it. But I fear that the results of this operation would
be too long for that to be workable.
More generating UPDATEs without knowledge of table structure
(knowledge of PK) can be issue (and possibly dangerous too), and you cannot
to recognize PK from result of SELECT (Everywhere PK is not "id" and it is
not one column).
It does look like it's trying to find out the pkey from the system
catalogs ... however, it's also accepting unique constraints without
a lot of thought about the implications of NULLs. Even if you have
a pkey, it's not exactly clear to me what should happen if the user
changes the contents of a pkey field. That could be interpreted as
either an INSERT or UPDATE, I think.
Also, while I didn't read too closely, it's not clear to me how the
code could reliably distinguish INSERT vs UPDATE vs DELETE cases ---
surely we're not going to try to put a "diff" engine into this, and
even if we did, diff is well known for producing somewhat surprising
decisions about exactly which old lines match which new ones. That's
part of the reason why I really don't like the idea that the deduced
change commands will be summarily applied without the user even
seeing them.
regards, tom lane
On Mon, Jan 22, 2024 at 8:06 AM Christoph Berg <myon@debian.org> wrote:
Assuming a SELECT statement reading from a single table, it is quite an
effort to transform that statement to an UPDATE statement on that table,
perhaps to fix a typo that the user has spotted in the query result.
Building off the other comments, I'd suggest trying to get rid of the
intermediate JSOn format and also just focus on a single row at any given
time.
For an update the first argument to the metacommand could be the unique key
value present in the previous result. The resultant UPDATE would just put
that into the where clause and every other column in the result would be a
SET clause column with the thing being set the current value, ready to be
edited.
DELETE would be similar but without the need for a SET clause.
INSERT can produce a template INSERT (cols) VALUES ... command with some
smarts regarding auto incrementing keys and placeholder values.
David J.
po 22. 1. 2024 v 17:34 odesílatel David G. Johnston <
david.g.johnston@gmail.com> napsal:
On Mon, Jan 22, 2024 at 8:06 AM Christoph Berg <myon@debian.org> wrote:
Assuming a SELECT statement reading from a single table, it is quite an
effort to transform that statement to an UPDATE statement on that table,
perhaps to fix a typo that the user has spotted in the query result.Building off the other comments, I'd suggest trying to get rid of the
intermediate JSOn format and also just focus on a single row at any given
time.For an update the first argument to the metacommand could be the unique
key value present in the previous result. The resultant UPDATE would just
put that into the where clause and every other column in the result would
be a SET clause column with the thing being set the current value, ready to
be edited.DELETE would be similar but without the need for a SET clause.
INSERT can produce a template INSERT (cols) VALUES ... command with some
smarts regarding auto incrementing keys and placeholder values.David J.
Can you imagine using it? I like psql, I like term applications, my first
database was FoxPro, but for dataeditation I am almost sure so I don't want
to use psql.
I can imagine enhancing the current \gexec command because it is executed
directly without possibility to edit. I see valuable some special clause
"edit"
like
\gexec_edit or \gexec(edit) or \gexec edit
This is like current gexec but with possibility to edit the result in
editor and with possibility to finish without saving.
Then we can introduce SQL functions UpdateTemplate(cols text[], rowvalue),
InsertTemplate, ...
and then you can write
SELECT UpdateTemplace(ARRAY['a','b','c'], foo) FROM foo WHERE id IN (1,2)
\gexec_with_edit
But still looks strange to me - like we try reintroduce of necessity sed or
awk to SQL and psql
I would have forms like FoxPro, I would have a grid like FoxPro, but not in
psql, and I would not develop it :-)
Pavel Stehule <pavel.stehule@gmail.com> writes:
I would have forms like FoxPro, I would have a grid like FoxPro, but not in
psql, and I would not develop it :-)
Yeah, that's something that was also bothering me, but I failed to
put my finger on it. "Here's some JSON, edit it, and don't forget
to keep the quoting correct" does not strike me as a user-friendly
way to adjust data content. A spreadsheet-like display where you
can change data within cells seems like a far better API, although
I don't want to write that either.
This kind of API would not readily support INSERT or DELETE cases, but
TBH I think that's better anyway --- you're adding too much ambiguity
in pursuit of a very secondary use-case. The stated complaint was
"it's too hard to build UPDATE commands", which I can sympathize with.
(BTW, I wonder how much of this already exists in pgAdmin.)
regards, tom lane
Re: Pavel Stehule
Introduction of \gedit is interesting idea, but in this form it looks too
magica) why the data are in JSON format, that is not native for psql (minimally
now)
Because we need something machine-readable. CSV would be an
alternative, but that is hardly human-readable.
b) the implicit transformation to UPDATEs and the next evaluation can be
pretty dangerous.
You can always run it in a transaction:
begin;
select * from tbl \gedit
commit;
Alternatively, we could try to make it not send the commands right away
- either by putting them into the query buffer (that currently work
only for single commands without any ';') or by opening another
editor. Doable, but perhaps the transaction Just Works?
The concept of proposed feature is interesting, but the name \gedit is too
generic, maybe too less descriptive for this purposeMaybe \geditupdates can be better - but still it can be dangerous and slow
(without limits)
An alternative I was considering was \edittable, but that would put
more emphasis on "table" when it's actually more powerful than that,
it works on a query result. (Similar in scope to updatable views.)
In the end I am not sure if I like it or dislike it. Looks dangerous. I can
imagine possible damage when some people will see vi first time and will
try to finish vi, but in this command, it will be transformed to executed
UPDATEs.
If you close the editor with touching the file, nothing will be sent.
And if you mess up the file, it will complain. I think it's unlikely
that people who end up in an editor they can't operate would be able
to modify JSON in a way that is still valid.
Also, \e has the same problem. Please don't let "there are users who
don't know what they are doing" spoil the idea.
More generating UPDATEs without knowledge of table structure
(knowledge of PK) can be issue (and possibly dangerous too), and you cannot
to recognize PK from result of SELECT (Everywhere PK is not "id" and it is
not one column).
It *does* retrieve the proper PK from the table. All updates are based
on the PK.
Re: Tom Lane
Introduction of \gedit is interesting idea, but in this form it looks too
magicYeah, I don't like it either --- it feels like something that belongs
in an ETL tool not psql.
I tried to put it elsewhere first, starting with pspg:
https://github.com/okbob/pspg/issues/200
The problem is that external programs like the pager neither have
access to the query string/table name, nor the database connection.
ETL would also not quite fit, this is meant for interactive use.
The sheer size of the patch shows how far
afield it is from anything that psql already does, necessitating
writing tons of stuff that was not there before.
I've been working on this for several months, so it's already larger
than the MVP would be. It does have features like (key=col1,col2) that
could be omitted.
The bits that try
to parse the query to get necessary information seem particularly
half-baked.
Yes, that's not pretty, and I'd be open for suggestions on how to
improve that. I was considering:
1) this (dumb query parsing)
2) EXPLAIN the query to get the table
3) use libpq's PQftable
The problem with 2+3 is that on views and partitioned tables, this
would yield the base table name, not the table name used in the query.
1 turned out to be the most practical, and worked for me so far.
If the parse tree would be available, using that would be much better.
Should we perhaps add something like "explain (parse) select...", or
add pg_parsetree(query) function?
Yup -- you'd certainly want some way of confirming that you actually
want the changes applied. Our existing edit commands load the edited
string back into the query buffer, where you can \r it if you don't
want to run it. But I fear that the results of this operation would
be too long for that to be workable.
The results are as long as you like. The intended use case would be to
change just a few rows.
As said above, I was already thinking of making it user-confirmable,
just the current version doesn't have it.
It does look like it's trying to find out the pkey from the system
catalogs ... however, it's also accepting unique constraints without
a lot of thought about the implications of NULLs.
Right, the UNIQUE part doesn't take care of NULLs yet. Will fix that.
(Probably by erroring out if any key column is NULL.)
Even if you have
a pkey, it's not exactly clear to me what should happen if the user
changes the contents of a pkey field. That could be interpreted as
either an INSERT or UPDATE, I think.
A changed PK will be interpreted as DELETE + INSERT. (I shall make
that more clear in the documentation.)
Also, while I didn't read too closely, it's not clear to me how the
code could reliably distinguish INSERT vs UPDATE vs DELETE cases ---
surely we're not going to try to put a "diff" engine into this, and
even if we did, diff is well known for producing somewhat surprising
decisions about exactly which old lines match which new ones. That's
part of the reason why I really don't like the idea that the deduced
change commands will be summarily applied without the user even
seeing them.
The "diff" is purely based on "after editing, is there still a row
with this key identity". If the PK columns are not changed, the UPDATE
will hook on the PK value.
If you changed the PK columns, that will be a DELETE and an INSERT.
During development, I was considering to forbid (error out) changing
the PK columns. But then, simply deleting rows (= DELETE) and adding
new rows (= INSERT) seemed like a nice feature by itself, so I left
that in. Perhaps that should be reconsidered?
Christoph
Import Notes
Reply to msg id not found: 3743018.1705940127@sss.pgh.pa.usCAFj8pRABUfUHqdmYSuWagwkXDtY1cwCQLV8-id8M9ZkMJLg@mail.gmail.com | Resolved by subject fallback
Re: David G. Johnston
Building off the other comments, I'd suggest trying to get rid of the
intermediate JSOn format and also just focus on a single row at any given
time.
We need *some* machine-readable format. It doesn't have to be JSON,
but JSON is actually pretty nice to read - and if values are too long,
or there are too many values, switch to extended mode:
select * from messages \gedit (expanded)
[{
"id": "1",
"language": "en",
"message": "This is a very long test text with little actual meaning."
},{
"id": "2",
"language": "en",
"message": "Another one, a bit shorter."
}]
I tweaked the indentation in the psql JSON output patch specifically
to make it readable.
Restricting to a single line might make sense if it helps editing, but
I don't think it does.
For an update the first argument to the metacommand could be the unique key
value present in the previous result. The resultant UPDATE would just put
that into the where clause and every other column in the result would be a
SET clause column with the thing being set the current value, ready to be
edited.
Hmm, then you would still have to cut-and-paste the PK value. If that
that's a multi-column non-numeric key, you are basically back to the
original problem.
Re: Tom Lane
Yeah, that's something that was also bothering me, but I failed to
put my finger on it. "Here's some JSON, edit it, and don't forget
to keep the quoting correct" does not strike me as a user-friendly
way to adjust data content. A spreadsheet-like display where you
can change data within cells seems like a far better API, although
I don't want to write that either.
Right. I wouldn't want a form editing system in there either. But
perhaps this middle ground of using a well-established format that is
easy to generate and to parse (it's using the JSON parser from
pgcommon) makes it fit into psql.
If parsing the editor result fails, the user is asked if they want to
re-edit with a parser error message, and if they go to the editor
again, the cursor is placed in the line where the error is. (Also,
what's wrong with having to strictly adhere to some syntax, we are
talking about SQL here.)
It's admittedly larger than the average \backslash command, but it
does fit into psql's interactive usage. \crosstabview is perhaps a
similar thing - it doesn't really fit into a simple "send query and
display result" client, but since it solves an actual problem, it
makes well sense to spend the extra code on it.
This kind of API would not readily support INSERT or DELETE cases, but
TBH I think that's better anyway --- you're adding too much ambiguity
in pursuit of a very secondary use-case. The stated complaint was
"it's too hard to build UPDATE commands", which I can sympathize with.
I've been using the feature already for some time, and it's a real
relief. In my actual use case here, I use it on my ham radio logbook:
=# select start, call, qrg, name from log where cty = 'CE9' order by start;
start │ call │ qrg │ name
────────────────────────┼────────┼─────────────┼───────
2019-03-12 20:34:00+00 │ RI1ANL │ 7.076253 │ ∅
2021-03-16 21:24:00+00 │ DP0GVN │ 2400.395 │ Felix
2022-01-15 17:19:00+00 │ DP0GVN │ 2400.01 │ Felix
2022-10-23 19:17:15+00 │ DP0GVN │ 2400.041597 │ ∅
2023-10-01 14:05:00+00 │ 8J1RL │ 28.182575 │ ∅
2024-01-22 21:15:15+00 │ DP1POL │ 10.138821 │ ∅
(6 Zeilen)
The primary key is (start, call).
If I now want to note that the last contact with Antarctica there was
also with Felix, I'd have to transform that into
update log set name = 'Felix' where start = '2024-01-22 21:15:15+00' and call = 'DP1POL';
\gedit is just so much easier.
UPDATE is the core feature. If we want to say INSERT and DELETE aren't
supported, but UPDATE support can go in, that'd be fine with me.
(BTW, I wonder how much of this already exists in pgAdmin.)
pgadmin seems to support it. (Most other clients don't.)
Obviously, I would want to do the updating using the client I also use
for querying.
Christoph
Import Notes
Reply to msg id not found: 3798850.1705946680@sss.pgh.pa.usCAKFQuwYzHrqCPp2PYvvp3p7fKakg9ZTHdgniCFReFZ9LraQbqg@mail.gmail.com | Resolved by subject fallback
po 22. 1. 2024 v 23:54 odesílatel Christoph Berg <myon@debian.org> napsal:
Re: David G. Johnston
Building off the other comments, I'd suggest trying to get rid of the
intermediate JSOn format and also just focus on a single row at any given
time.We need *some* machine-readable format. It doesn't have to be JSON,
but JSON is actually pretty nice to read - and if values are too long,
or there are too many values, switch to extended mode:select * from messages \gedit (expanded)
[{
"id": "1",
"language": "en",
"message": "This is a very long test text with little actual meaning."
},{
"id": "2",
"language": "en",
"message": "Another one, a bit shorter."
}]I tweaked the indentation in the psql JSON output patch specifically
to make it readable.Restricting to a single line might make sense if it helps editing, but
I don't think it does.For an update the first argument to the metacommand could be the unique
key
value present in the previous result. The resultant UPDATE would just
put
that into the where clause and every other column in the result would be
a
SET clause column with the thing being set the current value, ready to be
edited.Hmm, then you would still have to cut-and-paste the PK value. If that
that's a multi-column non-numeric key, you are basically back to the
original problem.Re: Tom Lane
Yeah, that's something that was also bothering me, but I failed to
put my finger on it. "Here's some JSON, edit it, and don't forget
to keep the quoting correct" does not strike me as a user-friendly
way to adjust data content. A spreadsheet-like display where you
can change data within cells seems like a far better API, although
I don't want to write that either.Right. I wouldn't want a form editing system in there either. But
perhaps this middle ground of using a well-established format that is
easy to generate and to parse (it's using the JSON parser from
pgcommon) makes it fit into psql.If parsing the editor result fails, the user is asked if they want to
re-edit with a parser error message, and if they go to the editor
again, the cursor is placed in the line where the error is. (Also,
what's wrong with having to strictly adhere to some syntax, we are
talking about SQL here.)It's admittedly larger than the average \backslash command, but it
does fit into psql's interactive usage. \crosstabview is perhaps a
similar thing - it doesn't really fit into a simple "send query and
display result" client, but since it solves an actual problem, it
makes well sense to spend the extra code on it.
\crosstabview is read only
This kind of API would not readily support INSERT or DELETE cases, but
TBH I think that's better anyway --- you're adding too much ambiguity
in pursuit of a very secondary use-case. The stated complaint was
"it's too hard to build UPDATE commands", which I can sympathize with.I've been using the feature already for some time, and it's a real
relief. In my actual use case here, I use it on my ham radio logbook:=# select start, call, qrg, name from log where cty = 'CE9' order by start;
start │ call │ qrg │ name
────────────────────────┼────────┼─────────────┼───────
2019-03-12 20:34:00+00 │ RI1ANL │ 7.076253 │ ∅
2021-03-16 21:24:00+00 │ DP0GVN │ 2400.395 │ Felix
2022-01-15 17:19:00+00 │ DP0GVN │ 2400.01 │ Felix
2022-10-23 19:17:15+00 │ DP0GVN │ 2400.041597 │ ∅
2023-10-01 14:05:00+00 │ 8J1RL │ 28.182575 │ ∅
2024-01-22 21:15:15+00 │ DP1POL │ 10.138821 │ ∅
(6 Zeilen)The primary key is (start, call).
If I now want to note that the last contact with Antarctica there was
also with Felix, I'd have to transform that intoupdate log set name = 'Felix' where start = '2024-01-22 21:15:15+00' and
call = 'DP1POL';\gedit is just so much easier.
It looks great for simple queries, but if somebody uses it like SELECT *
FROM pg_proc \gedit
I almost sure so \gedit is wrong name for this feature.
Can be nice if we are able:
a) export data set in some readable format
b) be possible to use more command in pipes
some like
select start, call, qrg, name from log where cty = 'CE9' order by start
\gpipexec(tsv) mypipe | bash update_pattern.sh > tmpfile; vi tmpfile; cat
tmpfile > mypipe
I understand your motivation well, but I don't like your proposal because
too many different things are pushed to one feature, and it is designed for
a single purpose.
UPDATE is the core feature. If we want to say INSERT and DELETE aren't
Show quoted text
supported, but UPDATE support can go in, that'd be fine with me.
(BTW, I wonder how much of this already exists in pgAdmin.)
pgadmin seems to support it. (Most other clients don't.)
Obviously, I would want to do the updating using the client I also use
for querying.Christoph
Re: Pavel Stehule
It looks great for simple queries, but if somebody uses it like SELECT *
FROM pg_proc \gedit
What's wrong with that? If the pager can handle the amount of data,
the editor can do that as well. (If not, the fix is to just not run
the command, and not blame the feature.)
I almost sure so \gedit is wrong name for this feature.
I'm open for suggestions.
Can be nice if we are able:
a) export data set in some readable format
b) be possible to use more command in pipes
some like
select start, call, qrg, name from log where cty = 'CE9' order by start
\gpipexec(tsv) mypipe | bash update_pattern.sh > tmpfile; vi tmpfile; cat
tmpfile > mypipe
Well yeah, that's still a lot of typing.
I understand your motivation well, but I don't like your proposal because
too many different things are pushed to one feature, and it is designed for
a single purpose.
It's one feature for one purpose. And the patch isn't *that* huge. Did
I make the mistake of adding documentation, extra command options, and
tab completion in v1?
Christoph
út 23. 1. 2024 v 11:38 odesílatel Christoph Berg <myon@debian.org> napsal:
Re: Pavel Stehule
It looks great for simple queries, but if somebody uses it like SELECT *
FROM pg_proc \geditWhat's wrong with that? If the pager can handle the amount of data,
the editor can do that as well. (If not, the fix is to just not run
the command, and not blame the feature.)
just editing wide or long command or extra long strings can be unfriendly
I almost sure so \gedit is wrong name for this feature.
I'm open for suggestions.
I have not too many ideas. The problem is in missing relation between
"edit" and "update and execute"
Can be nice if we are able:
a) export data set in some readable format
b) be possible to use more command in pipes
some like
select start, call, qrg, name from log where cty = 'CE9' order by start
\gpipexec(tsv) mypipe | bash update_pattern.sh > tmpfile; vi tmpfile; cat
tmpfile > mypipeWell yeah, that's still a lot of typing.
it should not be problem. You can hide long strings to psql variables
I understand your motivation well, but I don't like your proposal because
too many different things are pushed to one feature, and it is designedfor
a single purpose.
It's one feature for one purpose. And the patch isn't *that* huge. Did
I make the mistake of adding documentation, extra command options, and
tab completion in v1?
no - I have problem so one command does editing, generating write SQL
commands (updates) and their execution
Show quoted text
Christoph
On Mon, Jan 22, 2024 at 11:15 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Introduction of \gedit is interesting idea, but in this form it looks too
magicYeah, I don't like it either --- it feels like something that belongs
in an ETL tool not psql. The sheer size of the patch shows how far
afield it is from anything that psql already does, necessitating
writing tons of stuff that was not there before. The bits that try
to parse the query to get necessary information seem particularly
half-baked.
Based on these comments and the one from David Johnston, I think there
is a consensus that we do not want this patch, so I'm marking it as
Rejected in the CommitFest application. If I've misunderstood the
situation, then please feel free to change the status accordingly.
I feel slightly bad about rejecting this not only because rejecting
patches that people have put work into sucks but also because (1) I do
understand why it could be useful to have something like this and (2)
I think in many ways the patch is quite well-considered, e.g. it has
options like table and key to work around cases where the naive logic
doesn't get the right answer. But I also do understand why the
reactions thus far have been skeptical: there's a lot of pretty
magical stuff in this patch. That's a reliability concern: when you
type \gedit and it works, that's cool, but sometimes it isn't going to
work, and you're not always going to understand why, and you can
probably fix a lot of those cases by using the "table" or "key"
options, but you have to know they exist, and you have to realize that
they're needed, and the whole thing is suddenly a lot less convenient.
I think if we add this feature, a bunch of people won't notice, but
among those who do, I bet there will be a pretty good chance of people
complaining about cases that don't work, and perhaps not understanding
why they don't work, and I suspect fixing some of those complaints may
require something pretty close to solving the halting problem. :-(
Now maybe that's all wrong and we should adopt this patch with
enthusiasm, but if so, we need the patch to have significantly more +1
votes than -1 votes, and right now the reverse seems to be the case.
--
Robert Haas
EDB: http://www.enterprisedb.com
Re: Robert Haas
Based on these comments and the one from David Johnston, I think there
is a consensus that we do not want this patch, so I'm marking it as
Rejected in the CommitFest application. If I've misunderstood the
situation, then please feel free to change the status accordingly.
Hi Robert,
thanks for looking at the patch.
I feel slightly bad about rejecting this not only because rejecting
patches that people have put work into sucks but also because (1) I do
understand why it could be useful to have something like this and (2)
I think in many ways the patch is quite well-considered, e.g. it has
options like table and key to work around cases where the naive logic
doesn't get the right answer. But I also do understand why the
reactions thus far have been skeptical: there's a lot of pretty
magical stuff in this patch. That's a reliability concern: when you
type \gedit and it works, that's cool, but sometimes it isn't going to
work, and you're not always going to understand why, and you can
probably fix a lot of those cases by using the "table" or "key"
options, but you have to know they exist, and you have to realize that
they're needed, and the whole thing is suddenly a lot less convenient.
I think if we add this feature, a bunch of people won't notice, but
among those who do, I bet there will be a pretty good chance of people
complaining about cases that don't work, and perhaps not understanding
why they don't work, and I suspect fixing some of those complaints may
require something pretty close to solving the halting problem. :-(
That's a good summary of the situation, thanks.
I still think the feature would be cool to have, but admittedly, in
the meantime I've had cases myself where the automatism went into the
wrong direction (updating key columns results in DELETE-INSERT cycles
that aren't doing the right thing if you didn't select all columns
originally), so I now understand the objections and agree people were
right about that. This could be fixed by feeding the resulting
commands through another editor round, but that just adds complexity
instead of removing confusion.
I think there is a pretty straightforward way to address the problems,
though: instead of letting the user edit JSON, format the query result
in the form of UPDATE commands and let the user edit them. As Tom said
upthread:
Tom> The stated complaint was "it's too hard to build UPDATE commands",
Tom> which I can sympathize with.
... which this would perfectly address - it's building the commands.
The editor will have a bit more clutter (the UPDATE SET WHERE
boilerplate), and the fields are somewhat out of order (the key at the
end), but SQL commands are what people understand, and there is
absolutely no ambiguity on what is going to be executed since the
commands are exactly what is leaving the editor.
Now maybe that's all wrong and we should adopt this patch with
enthusiasm, but if so, we need the patch to have significantly more +1
votes than -1 votes, and right now the reverse seems to be the case.
I'll update the patch and ask here. Thanks!
Christoph
On Fri, May 17, 2024 at 9:24 AM Christoph Berg <myon@debian.org> wrote:
Tom> The stated complaint was "it's too hard to build UPDATE commands",
Tom> which I can sympathize with.... which this would perfectly address - it's building the commands.
The editor will have a bit more clutter (the UPDATE SET WHERE
boilerplate), and the fields are somewhat out of order (the key at the
end), but SQL commands are what people understand, and there is
absolutely no ambiguity on what is going to be executed since the
commands are exactly what is leaving the editor.
A point to consider is that the user can also do this in the query
itself, if desired. It'd just be a matter of assembling the query
string with appropriate calls to quote_literal() and quote_ident(),
which is not actually all that hard and I suspect many of us have done
that at one point or another. And then you know that you'll get the
right set of key columns and update the right table (as opposed to,
say, a view over the right table, or the wrong one out of several
tables that you joined).
Now you might say, well, that's not terribly convenient, which is
probably true, but this might be a case of -- convenient, reliable,
works with arbitrary queries -- pick two. I don't know. I wouldn't be
all that surprised if there's something clever and useful we could do
in this area, but I sure don't know what it is.
--
Robert Haas
EDB: http://www.enterprisedb.com