From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | Christoph Berg <myon(at)debian(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: psql: Allow editing query results with \gedit |
Date: | 2024-01-22 16:15:27 |
Message-ID: | 3743018.1705940127@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> po 22. 1. 2024 v 16:06 odesílatel Christoph Berg <myon(at)debian(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Andy Fan | 2024-01-22 16:16:10 | Re: the s_lock_stuck on perform_spin_delay |
Previous Message | Dmitry Dolgov | 2024-01-22 16:11:23 | Re: pg_stat_statements and "IN" conditions |