Re: psql: Allow editing query results with \gedit

Re: Pavel Stehule
> 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)

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 purpose
> 
> Maybe \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
> > magic
> 
> Yeah, 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