Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rzDvm-002a14-9L for pgadmin-hackers@arkaria.postgresql.org; Tue, 23 Apr 2024 11:03:22 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1rzDvk-007on4-I1 for pgadmin-hackers@arkaria.postgresql.org; Tue, 23 Apr 2024 11:03:20 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rzDvk-007omw-7c for pgadmin-hackers@lists.postgresql.org; Tue, 23 Apr 2024 11:03:20 +0000 Received: from mail-wr1-f43.google.com ([209.85.221.43]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1rzDvf-002VYF-V5 for pgadmin-hackers@postgresql.org; Tue, 23 Apr 2024 11:03:19 +0000 Received: by mail-wr1-f43.google.com with SMTP id ffacd0b85a97d-343c7fae6e4so4810213f8f.1 for ; Tue, 23 Apr 2024 04:03:15 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713870195; x=1714474995; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fal8Jh9Bve6k7uXPodrbpeHcDMqxAIMPD1TuikupXK4=; b=KgRjZbt2UpSE2N5vPcQE5cWUt4yD6ldJSHk6khZXMrYOZp6o+FcA+nN0MhOBae1NkD OJ6iX6qIqj5M4BaSY0lbl/pnWMWeP2RmBQWE0Rd96F4GZ4Mx44AMGLkNY/Hx5y7dtYEb 0qKNQkJ7QUwxUxfKUucOQTr7qeNdNluIT8+lFrZDx2QFpvMUDEi1J10cXJbXA72Q5Znk WNvgxU+2w04F7JIUWJsfqsWkcK0M0VPprJkbbfCFQ1wCZ+liTcEHmVENmonTbjnBPy7L n1clLAfPU17bsbOrO79z6a8+Kf43ftPtI9IwXnCUK/cj4osjYQIw+PKLjVysjlsvbmUY ScGQ== X-Forwarded-Encrypted: i=1; AJvYcCUZ4RBNWxOZW5BSf8DNQVaS/tvLRLxZOtGQ20vDHXqpkQRAkE5jaCNQZ3L31QBRnUIj/PLjKXvJdrRtrZ1pEXernQ5sNEjUQDqely6B+7w= X-Gm-Message-State: AOJu0YzWfiL+edWF9ukukjkOXY2Ar3fygaFBLQOP4ZRPPPB6GoNWMIdr q7LOCXl+0brNRfgJP+Be41B9VDKQXHTl1bRM52mh0Zs844XzQgjZ1Hn+HBB68e7OoWYUvULPYFq 3NrkLF7Dj8ROOXPaPvU5s8z/XRnI= X-Google-Smtp-Source: AGHT+IFI+5y93z++QmieY/wQ7eDhY1sQDYDAFvfBsUBcqjjAI++ckzPjdzfBwbpXerhm59VxuVTbtDGzv7WpDlqfD/I= X-Received: by 2002:adf:f58a:0:b0:343:e45c:c91d with SMTP id f10-20020adff58a000000b00343e45cc91dmr7418755wro.41.1713870194618; Tue, 23 Apr 2024 04:03:14 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Thom Brown Date: Tue, 23 Apr 2024 12:03:01 +0100 Message-ID: Subject: Re: Regarding feature #6841 To: Dave Page Cc: Aditya Toshniwal , Anil Sahoo , pgadmin-hackers Content-Type: multipart/alternative; boundary="000000000000243ed40616c180f1" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --000000000000243ed40616c180f1 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Apr 23, 2024, 11:49 Dave Page wrote: > > > On Tue, 23 Apr 2024 at 11:29, Thom Brown wrote: > >> On Tue, Apr 23, 2024, 09:15 Dave Page 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 < >>> aditya.toshniwal@enterprisedb.com> wrote: >>> >>>> Hi Dave, >>>> >>>> On Fri, Apr 19, 2024 at 7:15=E2=80=AFPM Aditya Toshniwal < >>>> aditya.toshniwal@enterprisedb.com> wrote: >>>> >>>>> Hi Dave, >>>>> >>>>> On Fri, Apr 19, 2024 at 7:05=E2=80=AFPM Dave Page = wrote: >>>>> >>>>>> Hi >>>>>> >>>>>> On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal < >>>>>> aditya.toshniwal@enterprisedb.com> wrote: >>>>>> >>>>>>> Hi Dave, >>>>>>> >>>>>>> On Fri, Apr 19, 2024 at 6:22=E2=80=AFPM Dave Page wrote: >>>>>>> >>>>>>>> Hi >>>>>>>> >>>>>>>> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal < >>>>>>>> aditya.toshniwal@enterprisedb.com> 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 ca= se, the >>>>>>>>>> Python multiline string. Presumably then it would also understan= d 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 fro= m 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 EPA= S.... >>>>>> >>>>> 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 butto= n without >>>>>>>>> touching the existing working. If a user chooses to use the new b= utton, 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 w= e >>>>>>>>> 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 al= l >>>>>>>> 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 a= bout >>>> a highlighter on what is the query based on cursor position? Example b= elow. >>>> 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 wil= l >>> 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 ca= n >> 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 selec= t > 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. Th= is > 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 > --000000000000243ed40616c180f1 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Tue, Apr 23, 2024, 11:49 Dave Page <dpage@pgadmin.org> wrote:


On Tue, 23 Apr= 2024 at 11:29, Thom Brown <thom@linux.com> wrote:
On Tue, Apr 23, 2024, 09:15 Dave Page <dpage@pgadmin.org> wrote:
Adding some notes below to summarise a discussion we had on t= his in a call...

On Mon, 22 Apr 2024 at 08:26, Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wro= te:
Hi=C2=A0Dave,

On Fri, Apr 19, 2024= at 7:15=E2=80=AFPM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:
Hi=C2=A0Dave,

On Fri, Apr 19, 2024 at 7:05=E2=80=AFPM Dave Pag= e <dpage@pgadmin.org> wrote:
Hi

On Fri, 19 Apr 2024 at 14:32, Aditya Tos= hniwal <aditya.toshniwal@enterprise= db.com> wrote:
Hi=C2=A0Dave,
<= /div>
O= n Fri, Apr 19, 2024 at 6:22=E2=80=AFPM Dave Page <d= page@pgadmin.org> wrote:
=
Hi

On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Even = if you put the cursor on the "SELECT"? If so, that would imply th= e parser understands the string quoting; e.g. in this case, the Python mult= iline string. Presumably then it would also understand regular single and d= ouble 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 synt= ax that isn't even in the standard. However, I suspect you mean Postgre= SQL-standard, as we are using the PostgreSQL dialect in CodeMirror. But, pg= Admin also supports EPAS....
We'll have = to test different scenarios to know exactly what works and what doesn't= .
=
=C2=A0

=C2=A0
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 apar= t from standard SQL queries. Like I said, we're adding it as a new butt= on 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 i= s 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=C2= =A0not showing it again.

Ten minutes later they will have forgotten that warning.
<= br>
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.
=C2=A0
We gave a thought on how a person would know what the query is when usi= ng keyboard shortcuts. So we came up with another suggestion. How about a h= ighlighter on what is the query based on cursor position? Example below. We= can disable it from preferences. We still need to check how the performanc= e will be, although we'll add debouncing.

3D"im=
So the plan is:

1) We automatically hi= ghlight the "current" query in the editor, similarly to the mocku= p above.

2) We add an option to Preferences (also = exposed under the Edit drop down in the Query Tool) to turn off that highli= ghting.

3) When the user clicks the "Execute = Query Under Cursor" button, it will be executed immediately if highlig= hting is enabled.

4) If highlighting is disabled, = the query to be executed will be displayed in a confirmation dialog to allo= w the user to review before execution.

5) The conf= irmation 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 quer= y that was executed to generate that resultset in all cases.

A butto= n above the resultset? Do you mean a tab, or an actual button?
<= /blockquote>

A button, alongside the ones that are alrea= dy there for editing data etc.=C2=A0
=C2=A0

I guess I would like to understand th= e 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 wher= e 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 predi= cates).

Other GUIs (for Postgre= SQL and other DBMSs) have this functionality, and we've had multiple re= quests for it.
=C2=A0

The latter seems more flexible a= nd predictable IMHO. And that way you can dispense with the confirmation di= alogue, and there's no need for any additional configuration options be= cause there's probably no need to disable it.
<= div>
You've been able to do the "Select and run"= ; thing for years. If you select text in the editor and hit the execute but= ton, only the selected text is sent to the server. If nothing is selected, = the entire string is sent. This feature will complement that for convenienc= e, but for safety will have a separate button/shortcut.

Oh, I clearl= y 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 wil= l no doubt try to use it on.

Regards

Thom
<= div class=3D"gmail_quote" dir=3D"auto">
--000000000000243ed40616c180f1--