Re: Regarding feature #6841

public inbox for [email protected]  
help / color / mirror / Atom feed

From: Thom Brown <[email protected]>
To: Dave Page <[email protected]>
Cc: Aditya Toshniwal <[email protected]>
Cc: Anil Sahoo <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: Regarding feature #6841
Date: Tue, 23 Apr 2024 12:03:01 +0100
Message-ID: <CAA-aLv54foGwCFYgsEN=tHM=Jzf=HBdsK96X+M0AaKigXh9OTg@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoyH6SNE_T+2p=4sQHHoTzsOOVuT+HEzqZMzgbKRXn_EAA@mail.gmail.com>
References: <CAO+oWtBxAaDxdPOkcf=bTU0D6sDogX_az4SgN-5zKZt-9G=2xQ@mail.gmail.com>
	<CA+OCxozieNmgfoGFq0G0Mxy+bu2Ct6hK9goZA3xtVJm9EaK6HA@mail.gmail.com>
	<CAO+oWtA13KM+jmWm+MkGbmLps2dh3B06mEZu6-PQjZ0ip7+G6g@mail.gmail.com>
	<CA+OCxoxMdX9Lsyugx7=_CZ3Z6K9jxHVTOBu4q=5_1cwgXsP7LQ@mail.gmail.com>
	<CAM9w-_mdXSmj2xv9W2k6hdmdY+3V8rfi4gnjBA_Jy28ZYkza2Q@mail.gmail.com>
	<CA+OCxozSdrD1ziKmfd_xLh72P031vdEo60wZxniyYP9sBLoM7Q@mail.gmail.com>
	<CAM9w-_=tRn_zM6C8Pm6B5w6du8BjdtrRnb1XxX1qyEcaSx4kEg@mail.gmail.com>
	<CA+OCxoz30kYxbska9TFHDKO3kXoervwY6OYE2kcY6b-k6-iYvg@mail.gmail.com>
	<CAM9w-_kjmG0rthcqss2TL+eFtC93qn_5ts_5=Fkqes82VG_EMg@mail.gmail.com>
	<CA+OCxozJpVhOTkPMv3_+EoEto+v+EeL=p1-dxjHFEvJM1EMBEQ@mail.gmail.com>
	<CAM9w-_n9i5fZSzRRNywJiCGv7TSRbSHLv+LVEBR=qn3zczrAxg@mail.gmail.com>
	<CAM9w-_nvA83wZ8m47B3fLswAQygrg1X9tk1g9hyQZEFqoH_zog@mail.gmail.com>
	<CA+OCxowig3kDr7APS0jitjBcE=Mq_vbnUNYYApt6-7_xp9yg6w@mail.gmail.com>
	<CAA-aLv6Lp-zHhnrh-SqzSeO7L8XanNjwhBQpsaP_4SqgwUHcmw@mail.gmail.com>
	<CA+OCxoyH6SNE_T+2p=4sQHHoTzsOOVuT+HEzqZMzgbKRXn_EAA@mail.gmail.com>

On Tue, Apr 23, 2024, 11:49 Dave Page <[email protected]> wrote:

>
>
> On Tue, 23 Apr 2024 at 11:29, Thom Brown <[email protected]> wrote:
>
>> On Tue, Apr 23, 2024, 09:15 Dave Page <[email protected]> wrote:
>>
>>> Adding some notes below to summarise a discussion we had on this in a
>>> call...
>>>
>>> On Mon, 22 Apr 2024 at 08:26, Aditya Toshniwal <
>>> [email protected]> wrote:
>>>
>>>> Hi Dave,
>>>>
>>>> On Fri, Apr 19, 2024 at 7:15 PM Aditya Toshniwal <
>>>> [email protected]> wrote:
>>>>
>>>>> Hi Dave,
>>>>>
>>>>> On Fri, Apr 19, 2024 at 7:05 PM Dave Page <[email protected]> wrote:
>>>>>
>>>>>> Hi
>>>>>>
>>>>>> On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
>>>>>> [email protected]> wrote:
>>>>>>
>>>>>>> Hi Dave,
>>>>>>>
>>>>>>> On Fri, Apr 19, 2024 at 6:22 PM Dave Page <[email protected]> wrote:
>>>>>>>
>>>>>>>> Hi
>>>>>>>>
>>>>>>>> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
>>>>>>>> [email protected]> wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Even if you put the cursor on the "SELECT"? If so, that would
>>>>>>>>>> imply the parser understands the string quoting; e.g. in this case, the
>>>>>>>>>> Python multiline string. Presumably then it would also understand regular
>>>>>>>>>> single and double quotes - what about (for example) a heredoc in a pl/sh
>>>>>>>>>> function?
>>>>>>>>>>
>>>>>>>>> Yes, the parser understands all the aspects of a SQL query and so
>>>>>>>>> it understands what type of token the cursor is based on which it does the
>>>>>>>>> syntax highlighting I believe.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Does it? Even EPAS extensions?
>>>>>>>>
>>>>>>> I mean only standard SQL grammar.
>>>>>>>
>>>>>>
>>>>>> Standard SQL grammar doesn't help us much - PostgreSQL is probably
>>>>>> the most standard compliant dialect there is, but if it deviates from the
>>>>>> standard in a few cases, and has a ton of syntax that isn't even in the
>>>>>> standard. However, I suspect you mean PostgreSQL-standard, as we are using
>>>>>> the PostgreSQL dialect in CodeMirror. But, pgAdmin also supports EPAS....
>>>>>>
>>>>> We'll have to test different scenarios to know exactly what works and
>>>>> what doesn't.
>>>>>
>>>>>>
>>>>>>
>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>>
>>>>>>>>>> It sounds like Thom has similar concerns, and I know him well
>>>>>>>>>> enough to know he wouldn't chime in without good reason.
>>>>>>>>>>
>>>>>>>>> There are limitations and it won't work correctly apart from
>>>>>>>>> standard SQL queries. Like I said, we're adding it as a new button without
>>>>>>>>> touching the existing working. If a user chooses to use the new button, he
>>>>>>>>> knows that pgAdmin will try to find the query on its own. This is an
>>>>>>>>> optional feature.
>>>>>>>>> Additionally, what we could do is when the user hits the button we
>>>>>>>>> will show a warning and the user can opt for not showing it again.
>>>>>>>>>
>>>>>>>>
>>>>>>>> Ten minutes later they will have forgotten that warning.
>>>>>>>>
>>>>>>>> I'm currently thinking that we should display the current query all
>>>>>>>> the time somehow (though I'm not sure how, without taking up a lot of
>>>>>>>> space).
>>>>>>>>
>>>>>>> Can't we add some kind of tooltip or popover on hover over the
>>>>>>> execute query button?
>>>>>>>
>>>>>>
>>>>>> Possibly :-). Let's try a PoC.
>>>>>>
>>>>> OK. I'll ask Anil to create some samples.
>>>>>
>>>>
>>>> We gave a thought on how a person would know what the query is when
>>>> using keyboard shortcuts. So we came up with another suggestion. How about
>>>> a highlighter on what is the query based on cursor position? Example below.
>>>> We can disable it from preferences. We still need to check how the
>>>> performance will be, although we'll add debouncing.
>>>>
>>>> [image: image.png]
>>>>
>>>
>>> So the plan is:
>>>
>>> 1) We automatically highlight the "current" query in the editor,
>>> similarly to the mockup above.
>>>
>>> 2) We add an option to Preferences (also exposed under the Edit drop
>>> down in the Query Tool) to turn off that highlighting.
>>>
>>> 3) When the user clicks the "Execute Query Under Cursor" button, it will
>>> be executed immediately if highlighting is enabled.
>>>
>>> 4) If highlighting is disabled, the query to be executed will be
>>> displayed in a confirmation dialog to allow the user to review before
>>> execution.
>>>
>>> 5) The confirmation dialogue will have a "Don't show this again" option
>>> for those that trust the CodeMirror parser enough.
>>>
>>> 6) A button above the resultset will be added to allow you to see the
>>> query that was executed to generate that resultset in all cases.
>>>
>>
>> A button above the resultset? Do you mean a tab, or an actual button?
>>
>
> A button, alongside the ones that are already there for editing data etc.
>
>
>>
>> I guess I would like to understand the rationale for this feature. Users
>> supposedly want this as described, but what is the precedent? I mean, I've
>> used GUIs for other DMBS's where you select what you want to run, and it
>> just runs the selection, even if it doesn't grab the whole query (e.g.
>> excluding ORDER BY or WHERE predicates).
>>
>
> Other GUIs (for PostgreSQL and other DBMSs) have this functionality, and
> we've had multiple requests for it.
>
>
>>
>> The latter seems more flexible and predictable IMHO. And that way you can
>> dispense with the confirmation dialogue, and there's no need for any
>> additional configuration options because there's probably no need to
>> disable it.
>>
>
> You've been able to do the "Select and run" thing for years. If you select
> text in the editor and hit the execute button, only the selected text is
> sent to the server. If nothing is selected, the entire string is sent. This
> feature will complement that for convenience, but for safety will have a
> separate button/shortcut.
>

Oh, I clearly don't use PgAdmin enough to know this already.

I still find the proposal somewhat unintuitive, but the foot-gun safeguards
that have been suggested sound like any pedal injuries will solely be the
fault of the user.

I would want to see it tested in a diverse range of scenarios though, which
will require some imagination given what users will no doubt try to use it
on.

Regards

Thom

>

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Regarding feature #6841
  In-Reply-To: <CAA-aLv54foGwCFYgsEN=tHM=Jzf=HBdsK96X+M0AaKigXh9OTg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox