Re: Regarding feature #6841

[Dave Page]

On Wed, 15 May 2024 at 12:33, Anil Sahoo 
wrote:

> Hi Team,
>
> I have implemented the Execute query feature and tested it with different
> kinds of test cases.
>
> FYI the PostgreSQL Do block does not work as a single statement when we
> put the cursor inside the block at different positions.
>
> The cursor position is mentioned using pipe('|') and underlined query is
> the active query that will run on click of Execute query button.
>
> I have attached some screenshots for reference.
>
> Please let me know if you have any questions.
>

That is exactly the sort of behaviour I was afraid of :-(. At least we have
safeguards in place to minimise the chances of the user running something
unexpected.

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

Re: Regarding feature #6841

[Dave Page]

On Wed, 24 Apr 2024 at 14:43, Thom Brown  wrote:

> On Tue, 23 Apr 2024 at 13:50, Dave Page  wrote:
>
>>
>>
>> On Tue, 23 Apr 2024 at 12:03, Thom Brown  wrote:
>>
>>>
 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.
>>>
>>
>> Boo!
>>
>>
>>>
>>> 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.
>>>
>>
>> Yes, I have made that very clear to the team. Suggestions for test
>> scenarios are welcome of course - a good way to experiment might be to see
>> how the current version of pgAdmin (which uses the new CodeMirror code)
>> manages to mess up syntax highlighting of anything weird.
>>
>
> I guess here's a few to try out:
>

Very helpful - thanks Thom!


>
> -- Put the cursor on every relation name, and every SELECT, DELETE and
> INSERT
> WITH deleted_rows AS (
>   DELETE FROM mytable WHERE id IN (
> -- Does this run on its own?
> SELECT id FROM mytable
>   )
>   RETURNING id, content
> ),
> move_rows AS (
>   INSERT INTO newtable
>   -- Does this SELECT run on its own, or does it backtrack to the INSERT?
>   SELECT id, content
>   FROM deleted_rows
> ),
> combined_result AS(
>   SELECT tableoid::regclass, id, content
>   FROM mytable
>   UNION ALL
>   -- Does this SELECT get run on its own?
>   SELECT tableoid::regclass, id, content
>   FROM newtable
> )
> -- Does this SELECT get run on its own?
> SELECT id, content
> INTO backuptable
> FROM combined_result;
>
>
> SELECT id, content
> FROM (
>   /*
> We are just performing:
> SELECT id, content
> FROM newtable;
> ... at 2 levels
> Does that commented query above highlight?
>
> Does each level of the query and nested queries run correctly?
>   */
>   SELECT id, content, 'dummy1'
>   FROM (
>  SELECT id, content, 'dummmy1', 'dummy2'
>  FROM newtable
>   )
> );
>
>
> DO LANGUAGE plpgsql $SELECT$
> DECLARE
>   myrec RECORD;
>   -- Does either SELECT in the cursor try to run when under PgAdmin's
> cursor?
>   -- Is there any backtracking when selecting the 2nd one?
>   mycur CURSOR FOR SELECT 1 FROM (SELECT (VALUES (1)));
> BEGIN
>   SELECT INTO STRICT myrec FROM (
> -- Does selecting the following SELECT correctly run without going
> -- into the SELECT INTO?
> SELECT
>  -- Can you run the query that appears in the value?
>   $$SELECT * FROM mytable$$ AS query,
>
>  -- What happens when you select either of these SELECTs?
>   'SELECT' AS "SELECT",
>
>  -- And what happens on each one of these 4 DELETEs
>   $DELETE$DELETE$DELETE$ AS "DELETE"
>   );
> END
> $SELECT$;
>
> None of this renders incorrectly in PgAdmin though.
>
> Thom
>


-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

Re: Regarding feature #6841

[Thom Brown]

On Tue, 23 Apr 2024 at 13:50, Dave Page  wrote:

>
>
> On Tue, 23 Apr 2024 at 12:03, Thom Brown  wrote:
>
>>
>>> 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.
>>
>
> Boo!
>
>
>>
>> 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.
>>
>
> Yes, I have made that very clear to the team. Suggestions for test
> scenarios are welcome of course - a good way to experiment might be to see
> how the current version of pgAdmin (which uses the new CodeMirror code)
> manages to mess up syntax highlighting of anything weird.
>

I guess here's a few to try out:

-- Put the cursor on every relation name, and every SELECT, DELETE and
INSERT
WITH deleted_rows AS (
  DELETE FROM mytable WHERE id IN (
-- Does this run on its own?
SELECT id FROM mytable
  )
  RETURNING id, content
),
move_rows AS (
  INSERT INTO newtable
  -- Does this SELECT run on its own, or does it backtrack to the INSERT?
  SELECT id, content
  FROM deleted_rows
),
combined_result AS(
  SELECT tableoid::regclass, id, content
  FROM mytable
  UNION ALL
  -- Does this SELECT get run on its own?
  SELECT tableoid::regclass, id, content
  FROM newtable
)
-- Does this SELECT get run on its own?
SELECT id, content
INTO backuptable
FROM combined_result;


SELECT id, content
FROM (
  /*
We are just performing:
SELECT id, content
FROM newtable;
... at 2 levels
Does that commented query above highlight?

Does each level of the query and nested queries run correctly?
  */
  SELECT id, content, 'dummy1'
  FROM (
 SELECT id, content, 'dummmy1', 'dummy2'
 FROM newtable
  )
);


DO LANGUAGE plpgsql $SELECT$
DECLARE
  myrec RECORD;
  -- Does either SELECT in the cursor try to run when under PgAdmin's
cursor?
  -- Is there any backtracking when selecting the 2nd one?
  mycur CURSOR FOR SELECT 1 FROM (SELECT (VALUES (1)));
BEGIN
  SELECT INTO STRICT myrec FROM (
-- Does selecting the following SELECT correctly run without going
-- into the SELECT INTO?
SELECT
 -- Can you run the query that appears in the value?
  $$SELECT * FROM mytable$$ AS query,

 -- What happens when you select either of these SELECTs?
  'SELECT' AS "SELECT",

 -- And what happens on each one of these 4 DELETEs
  $DELETE$DELETE$DELETE$ AS "DELETE"
  );
END
$SELECT$;

None of this renders incorrectly in PgAdmin though.

Thom

Re: Regarding feature #6841

[Anil Sahoo]

Thanks for the confirmation.

—



*Anil Sahoo*

Software Engineer

www.enterprisedb.com

Power to Postgres







On Wed, 24 Apr 2024 at 6:03 PM, Dave Page  wrote:

> Hi
>
> On Wed, 24 Apr 2024 at 12:31, Anil Sahoo 
> wrote:
>
>> Hi Dave,
>>
>> For the Point-2, Edit dropdown shows options that are one time
>> actionable. In place of showing the turning off highlight option in both
>> Edit dropdown and Preferences, we can show it only in Preferences.
>>
>> Please give your suggestion on this.
>>
>
> I think preferences only is fine.
>
>
>>
>> Regards,
>> Anil
>> --
>>
>> 
>>
>> *Anil Sahoo*
>>
>> Software Engineer
>>
>> www.enterprisedb.com
>>
>> Power to Postgres
>>
>> 
>> 
>> 
>> 
>>
>>
>> On Tue, Apr 23, 2024 at 1:45 PM 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.toshni...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 On Fri, Apr 19, 2024 at 7:15 PM Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Fri, Apr 19, 2024 at 7:05 PM Dave Page  wrote:
>
>> Hi
>>
>> On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:
>>>
 Hi

 On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
 aditya.toshni...@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 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 

Re: Regarding feature #6841

[Dave Page]

Hi

On Wed, 24 Apr 2024 at 12:31, Anil Sahoo 
wrote:

> Hi Dave,
>
> For the Point-2, Edit dropdown shows options that are one time actionable.
> In place of showing the turning off highlight option in both Edit dropdown
> and Preferences, we can show it only in Preferences.
>
> Please give your suggestion on this.
>

I think preferences only is fine.


>
> Regards,
> Anil
> --
>
> 
>
> *Anil Sahoo*
>
> Software Engineer
>
> www.enterprisedb.com
>
> Power to Postgres
>
> 
> 
> 
> 
>
>
> On Tue, Apr 23, 2024 at 1:45 PM 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.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Fri, Apr 19, 2024 at 7:15 PM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 On Fri, Apr 19, 2024 at 7:05 PM Dave Page  wrote:

> Hi
>
> On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
>>> aditya.toshni...@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 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 

Re: Regarding feature #6841

[Anil Sahoo]

Hi Dave,

For the Point-2, Edit dropdown shows options that are one time actionable.
In place of showing the turning off highlight option in both Edit dropdown
and Preferences, we can show it only in Preferences.

Please give your suggestion on this.

Regards,
Anil
--



*Anil Sahoo*

Software Engineer

www.enterprisedb.com

Power to Postgres







On Tue, Apr 23, 2024 at 1:45 PM 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.toshni...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> On Fri, Apr 19, 2024 at 7:15 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Fri, Apr 19, 2024 at 7:05 PM Dave Page  wrote:
>>>
 Hi

 On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:
>
>> Hi
>>
>> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
>> aditya.toshni...@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 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.
>
>
> --
> Dave Page
> pgAdmin: https://www.pgadmin.org
> PostgreSQL: https://www.postgresql.org
> EDB: https://www.enterprisedb.com
>
>

Re: Regarding feature #6841

[Anthony DeBarros]

Hi,


>
> On Tue, 23 Apr 2024 at 12:03, Thom Brown  wrote:
>
>
>>
>> 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.
>>
>>
>>
The scenario described by Dave sounds similar to how DataGrip handles this.
When I click into a query in a script file, the editor highlights the
query. Then I have the option to run that portion.

Enabling this functionality will save lots of time normally spent on
highlighting a query -- it will be awesome.

I'm also glad the team is adding a separate button to run just the
highlighted portion rather than change the behavior of the existing
run/execute button.

Re: Regarding feature #6841

[Dave Page]

On Tue, 23 Apr 2024 at 12:03, Thom Brown  wrote:

>
>> 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.
>

Boo!


>
> 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.
>

Yes, I have made that very clear to the team. Suggestions for test
scenarios are welcome of course - a good way to experiment might be to see
how the current version of pgAdmin (which uses the new CodeMirror code)
manages to mess up syntax highlighting of anything weird.

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

Re: Regarding feature #6841

[Thom Brown]

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.toshni...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 On Fri, Apr 19, 2024 at 7:15 PM Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Fri, Apr 19, 2024 at 7:05 PM Dave Page  wrote:
>
>> Hi
>>
>> On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:
>>>
 Hi

 On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
 aditya.toshni...@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 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 

Re: Regarding feature #6841

[Dave Page]

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.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Fri, Apr 19, 2024 at 7:15 PM Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 On Fri, Apr 19, 2024 at 7:05 PM Dave Page  wrote:

> Hi
>
> On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
>>> aditya.toshni...@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 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 

Re: Regarding feature #6841

[Thom Brown]

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.toshni...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> On Fri, Apr 19, 2024 at 7:15 PM Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Fri, Apr 19, 2024 at 7:05 PM Dave Page  wrote:
>>>
 Hi

 On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
 aditya.toshni...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:
>
>> Hi
>>
>> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
>> aditya.toshni...@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 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?

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).

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.

Regards

Thom

>

Re: Regarding feature #6841

[Dave Page]

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.toshni...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Fri, Apr 19, 2024 at 7:15 PM Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> On Fri, Apr 19, 2024 at 7:05 PM Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
>>> aditya.toshni...@enterprisedb.com> wrote:
>>>
 Hi Dave,

 On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:

> Hi
>
> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
> aditya.toshni...@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 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.


-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

Re: Regarding feature #6841

[Aditya Toshniwal]

Hi Dave,

On Fri, Apr 19, 2024 at 7:15 PM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Fri, Apr 19, 2024 at 7:05 PM Dave Page  wrote:
>
>> Hi
>>
>> On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
>> aditya.toshni...@enterprisedb.com> wrote:
>>
>>> Hi Dave,
>>>
>>> On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:
>>>
 Hi

 On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
 aditya.toshni...@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 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]


>
>>
>>>
 BTW, if we do figure out a way of doing this that we all agree is safe,
 I'm going to want to see a bunch of automated tests against valid EPAS and
 PG queries, as weird and bizarre as we can think of.

>>> I agree.
>>>

 --
 Dave Page
 pgAdmin: https://www.pgadmin.org
 PostgreSQL: https://www.postgresql.org
 EDB: https://www.enterprisedb.com


>>>
>>> --
>>> Thanks,
>>> Aditya Toshniwal
>>> pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
>>> 
>>> "Don't Complain about Heat, Plant a TREE"
>>>
>>
>>
>> --
>> Dave Page
>> pgAdmin: https://www.pgadmin.org
>> PostgreSQL: https://www.postgresql.org
>> EDB: https://www.enterprisedb.com
>>
>>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
> 
> "Don't Complain about Heat, Plant a TREE"
>


-- 
Thanks,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*

"Don't Complain about Heat, Plant a TREE"

Re: Regarding feature #6841

[Aditya Toshniwal]

Hi Dave,

On Fri, Apr 19, 2024 at 7:05 PM Dave Page  wrote:

> Hi
>
> On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
>>> aditya.toshni...@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 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.

>
>
>>
>>> BTW, if we do figure out a way of doing this that we all agree is safe,
>>> I'm going to want to see a bunch of automated tests against valid EPAS and
>>> PG queries, as weird and bizarre as we can think of.
>>>
>> I agree.
>>
>>>
>>> --
>>> Dave Page
>>> pgAdmin: https://www.pgadmin.org
>>> PostgreSQL: https://www.postgresql.org
>>> EDB: https://www.enterprisedb.com
>>>
>>>
>>
>> --
>> Thanks,
>> Aditya Toshniwal
>> pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
>> 
>> "Don't Complain about Heat, Plant a TREE"
>>
>
>
> --
> Dave Page
> pgAdmin: https://www.pgadmin.org
> PostgreSQL: https://www.postgresql.org
> EDB: https://www.enterprisedb.com
>
>

-- 
Thanks,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*

"Don't Complain about Heat, Plant a TREE"

Re: Regarding feature #6841

[Dave Page]

Hi

On Fri, 19 Apr 2024 at 14:32, Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:
>
>> Hi
>>
>> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
>> aditya.toshni...@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 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


>
>>
>>
>>>
 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.


>
>> BTW, if we do figure out a way of doing this that we all agree is safe,
>> I'm going to want to see a bunch of automated tests against valid EPAS and
>> PG queries, as weird and bizarre as we can think of.
>>
> I agree.
>
>>
>> --
>> Dave Page
>> pgAdmin: https://www.pgadmin.org
>> PostgreSQL: https://www.postgresql.org
>> EDB: https://www.enterprisedb.com
>>
>>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
> 
> "Don't Complain about Heat, Plant a TREE"
>


-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

Re: Regarding feature #6841

[Aditya Toshniwal]

Hi Dave,

On Fri, Apr 19, 2024 at 6:22 PM Dave Page  wrote:

> Hi
>
> On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
> aditya.toshni...@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 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.

>
>
>
>>
>>> 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?

>
> BTW, if we do figure out a way of doing this that we all agree is safe,
> I'm going to want to see a bunch of automated tests against valid EPAS and
> PG queries, as weird and bizarre as we can think of.
>
I agree.

>
> --
> Dave Page
> pgAdmin: https://www.pgadmin.org
> PostgreSQL: https://www.postgresql.org
> EDB: https://www.enterprisedb.com
>
>

-- 
Thanks,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*

"Don't Complain about Heat, Plant a TREE"

Re: Regarding feature #6841

[Dave Page]

Hi

On Fri, 19 Apr 2024 at 11:56, Aditya Toshniwal <
aditya.toshni...@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 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?



>
>> 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).

BTW, if we do figure out a way of doing this that we all agree is safe, I'm
going to want to see a bunch of automated tests against valid EPAS and PG
queries, as weird and bizarre as we can think of.

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

Re: Regarding feature #6841

[Aditya Toshniwal]

Hi Dave,

On Fri, Apr 19, 2024 at 4:10 PM Dave Page  wrote:

>
>
> On Fri, 19 Apr 2024 at 05:15, Aditya Toshniwal <
> aditya.toshni...@enterprisedb.com> wrote:
>
>> Hi Dave,
>>
>> On Thu, Apr 18, 2024 at 8:07 PM Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Thu, 18 Apr 2024 at 15:26, Anil Sahoo 
>>> wrote:
>>>
 Hi Dave,
 We took help from Code Mirror, i.e Code Mirror gives the parsed SQL
 from the editor through a tree called syntaxTree and by using some logic we
 extracted the statements which have semicolon in it and also added some
 extra logic to break the whole query on next of next line as empty or if
 comments are there.

 Using all this logic we got the individual queries and checked where
 our cursor is in editor and checked with the query and through this we got
 the actual query at cursor position.

 For example,

1. if the cursor is at starting or ending position or anywhere in
between a query with semicolon or without semicolon, that can be single
line or multi line then the query gets extracted.
2. if the cursor is at starting or ending position or anywhere in
between a comment that can be single line or multi line then the comment
gets extracted.
3. if the cursor is at a position where the previous line has a
query then that query gets extracted.

 For the anonymous block containing multiple queries, code mirror gives
 the statements differently. That is an incomplete query we can say, so the
 query tool gives error. We can say some limitations are there with Code
 Mirror.

 Please let me know if you have any questions on this.

>>>
>>> My main concern is that it doesn't get it wrong. Ever. Consider:
>>>
>>> DELETE FROM foo; SELECT * FROM foo;
>>>
>> It will depend where the cursor is and will pick one of the query, not
>> both.
>>
>>>
>>> Is that one statement or two? What if it's in the middle of a pl/python3
>>> function:
>>>
>>> my_sql = 'DELETE FROM foo; SELECT * FROM foo;'
>>>
>>> or
>>>
>>> my_sql = """DELETE FROM foo;
>>> SELECT * FROM foo;
>>> """
>>>
>> Since it is a part of the string, it will not run the string part. It
>> will execute along with my_sql=
>>
>
> 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.

>
>
>
>>
>>> (those are just simple examples from the top of my head).
>>>
>>> It could be extremely dangerous if we or CodeMirror mis-parses
>>> something, which seems quite possible unless it has access to the actual
>>> parser that PostgreSQL uses. Which makes me think... what of EPAS? It has
>>> an extended parser to handle some of the Oracle compatible syntax. Will
>>> CodeMirror get that right?
>>>
>> CodeMirror parser only provides parsing for standard SQL grammar. It
>> doesn't even understand pl/pgsql. It detects the query based on semicolons
>> very effectively. We have added our own logic to take that query
>> provided by CM and separate it by new line.
>> Instead of making it as the main execute button, I realise we should make
>> it as the second execute, and keep the main execute untouched.
>>
>
> Anil's examples mostly didn't have semicolons though - and in many cases,
> a user's script might not if they're writing a bunch of scratch queries and
> (as I do currently) executing them as needed by highlighting.
>
> As I'm sure you're starting to understand, I'm extremely concerned that
> this automatic parsing could get it wrong in some cases, which could
> potentially lead to users inadvertently running a destructive query without
> realising it. I'm also concerned that it will lead to less severe
> unexpected results; the parser doesn't understand pl/pgsql, so by
> extension, it also cannot understand an anonymous function written in
> pg/pgsql. Yet, in PostgreSQL an anonymous function (a DO statement) is a
> perfectly valid single statement that will contain sub-statements such as
> SELECTs or DELETEs etc. that the user may or may not expect to be
> considered part of the top-level DO statement.
>
> 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 

Re: Regarding feature #6841

[Dave Page]

On Fri, 19 Apr 2024 at 05:15, Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Dave,
>
> On Thu, Apr 18, 2024 at 8:07 PM Dave Page  wrote:
>
>> Hi
>>
>> On Thu, 18 Apr 2024 at 15:26, Anil Sahoo 
>> wrote:
>>
>>> Hi Dave,
>>> We took help from Code Mirror, i.e Code Mirror gives the parsed SQL from
>>> the editor through a tree called syntaxTree and by using some logic we
>>> extracted the statements which have semicolon in it and also added some
>>> extra logic to break the whole query on next of next line as empty or if
>>> comments are there.
>>>
>>> Using all this logic we got the individual queries and checked where our
>>> cursor is in editor and checked with the query and through this we got the
>>> actual query at cursor position.
>>>
>>> For example,
>>>
>>>1. if the cursor is at starting or ending position or anywhere in
>>>between a query with semicolon or without semicolon, that can be single
>>>line or multi line then the query gets extracted.
>>>2. if the cursor is at starting or ending position or anywhere in
>>>between a comment that can be single line or multi line then the comment
>>>gets extracted.
>>>3. if the cursor is at a position where the previous line has a
>>>query then that query gets extracted.
>>>
>>> For the anonymous block containing multiple queries, code mirror gives
>>> the statements differently. That is an incomplete query we can say, so the
>>> query tool gives error. We can say some limitations are there with Code
>>> Mirror.
>>>
>>> Please let me know if you have any questions on this.
>>>
>>
>> My main concern is that it doesn't get it wrong. Ever. Consider:
>>
>> DELETE FROM foo; SELECT * FROM foo;
>>
> It will depend where the cursor is and will pick one of the query, not
> both.
>
>>
>> Is that one statement or two? What if it's in the middle of a pl/python3
>> function:
>>
>> my_sql = 'DELETE FROM foo; SELECT * FROM foo;'
>>
>> or
>>
>> my_sql = """DELETE FROM foo;
>> SELECT * FROM foo;
>> """
>>
> Since it is a part of the string, it will not run the string part. It will
> execute along with my_sql=
>

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?



>
>> (those are just simple examples from the top of my head).
>>
>> It could be extremely dangerous if we or CodeMirror mis-parses something,
>> which seems quite possible unless it has access to the actual parser that
>> PostgreSQL uses. Which makes me think... what of EPAS? It has an extended
>> parser to handle some of the Oracle compatible syntax. Will CodeMirror get
>> that right?
>>
> CodeMirror parser only provides parsing for standard SQL grammar. It
> doesn't even understand pl/pgsql. It detects the query based on semicolons
> very effectively. We have added our own logic to take that query provided
> by CM and separate it by new line.
> Instead of making it as the main execute button, I realise we should make
> it as the second execute, and keep the main execute untouched.
>

Anil's examples mostly didn't have semicolons though - and in many cases, a
user's script might not if they're writing a bunch of scratch queries and
(as I do currently) executing them as needed by highlighting.

As I'm sure you're starting to understand, I'm extremely concerned that
this automatic parsing could get it wrong in some cases, which could
potentially lead to users inadvertently running a destructive query without
realising it. I'm also concerned that it will lead to less severe
unexpected results; the parser doesn't understand pl/pgsql, so by
extension, it also cannot understand an anonymous function written in
pg/pgsql. Yet, in PostgreSQL an anonymous function (a DO statement) is a
perfectly valid single statement that will contain sub-statements such as
SELECTs or DELETEs etc. that the user may or may not expect to be
considered part of the top-level DO statement.

It sounds like Thom has similar concerns, and I know him well enough to
know he wouldn't chime in without good reason.

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

Re: Regarding feature #6841

[Aditya Toshniwal]

Hi Thom,

On Thu, Apr 18, 2024 at 8:26 PM Thom Brown  wrote:

> On Thu, Apr 18, 2024, 15:26 Anil Sahoo 
> wrote:
>
>> Hi Dave,
>> We took help from Code Mirror, i.e Code Mirror gives the parsed SQL from
>> the editor through a tree called syntaxTree and by using some logic we
>> extracted the statements which have semicolon in it and also added some
>> extra logic to break the whole query on next of next line as empty or if
>> comments are there.
>>
>> Using all this logic we got the individual queries and checked where our
>> cursor is in editor and checked with the query and through this we got the
>> actual query at cursor position.
>>
>> For example,
>>
>>1. if the cursor is at starting or ending position or anywhere in
>>between a query with semicolon or without semicolon, that can be single
>>line or multi line then the query gets extracted.
>>2. if the cursor is at starting or ending position or anywhere in
>>between a comment that can be single line or multi line then the comment
>>gets extracted.
>>3. if the cursor is at a position where the previous line has a query
>>then that query gets extracted.
>>
>> For the anonymous block containing multiple queries, code mirror gives
>> the statements differently. That is an incomplete query we can say, so the
>> query tool gives error. We can say some limitations are there with Code
>> Mirror.
>>
>> Please let me know if you have any questions on this.
>>
>
> I guess my first question is, what is the requirement being fulfilled here?
>
> Also, I have more experience with selecting what to run, and having it run
> whatever was selected. I don't see the current proposal as particularly
> intuitive. Having the cursor midway through a statement (or even the last
> line of a statement) doesn't quite feel right. What would happen in this
> example?...
>
> SELECT contents
> FROM |mytable
> WHERE id BETWEEN 101 AND 200
>
> ORDER BY id ASC;
>
> Note I placed a pipe to represent the cursor before the table name. And
> would it make a difference if the first 3 lines were a single line?
>
> In any case, given all statements are separated by semicolons in
> PostgreSQL, I am resistant to the idea of supporting them without for
> specific features.
>
There is a requirement by many users to execute a query instead of the
current execute script which essentially executes all the code in the
editor if you don't have selection. The behaviour requested was to detect
the SQL query based on position of the query and execute that - just like
how DBeaver or many other SQL editors do.

>
> Regards
>
> Thom
>
>
>

-- 
Thanks,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*

"Don't Complain about Heat, Plant a TREE"

Re: Regarding feature #6841

[Aditya Toshniwal]

Hi Dave,

On Thu, Apr 18, 2024 at 8:07 PM Dave Page  wrote:

> Hi
>
> On Thu, 18 Apr 2024 at 15:26, Anil Sahoo 
> wrote:
>
>> Hi Dave,
>> We took help from Code Mirror, i.e Code Mirror gives the parsed SQL from
>> the editor through a tree called syntaxTree and by using some logic we
>> extracted the statements which have semicolon in it and also added some
>> extra logic to break the whole query on next of next line as empty or if
>> comments are there.
>>
>> Using all this logic we got the individual queries and checked where our
>> cursor is in editor and checked with the query and through this we got the
>> actual query at cursor position.
>>
>> For example,
>>
>>1. if the cursor is at starting or ending position or anywhere in
>>between a query with semicolon or without semicolon, that can be single
>>line or multi line then the query gets extracted.
>>2. if the cursor is at starting or ending position or anywhere in
>>between a comment that can be single line or multi line then the comment
>>gets extracted.
>>3. if the cursor is at a position where the previous line has a query
>>then that query gets extracted.
>>
>> For the anonymous block containing multiple queries, code mirror gives
>> the statements differently. That is an incomplete query we can say, so the
>> query tool gives error. We can say some limitations are there with Code
>> Mirror.
>>
>> Please let me know if you have any questions on this.
>>
>
> My main concern is that it doesn't get it wrong. Ever. Consider:
>
> DELETE FROM foo; SELECT * FROM foo;
>
It will depend where the cursor is and will pick one of the query, not both.


>
> Is that one statement or two? What if it's in the middle of a pl/python3
> function:
>
> my_sql = 'DELETE FROM foo; SELECT * FROM foo;'
>
> or
>
> my_sql = """DELETE FROM foo;
> SELECT * FROM foo;
> """
>
Since it is a part of the string, it will not run the string part. It will
execute along with my_sql=

>
> (those are just simple examples from the top of my head).
>
> It could be extremely dangerous if we or CodeMirror mis-parses something,
> which seems quite possible unless it has access to the actual parser that
> PostgreSQL uses. Which makes me think... what of EPAS? It has an extended
> parser to handle some of the Oracle compatible syntax. Will CodeMirror get
> that right?
>
CodeMirror parser only provides parsing for standard SQL grammar. It
doesn't even understand pl/pgsql. It detects the query based on semicolons
very effectively. We have added our own logic to take that query provided
by CM and separate it by new line.
Instead of making it as the main execute button, I realise we should make
it as the second execute, and keep the main execute untouched.

>
>
>>
>> Regards
>> Anil
>> --
>>
>> 
>>
>> *Anil Sahoo*
>>
>> Software Engineer
>>
>> www.enterprisedb.com
>>
>> Power to Postgres
>>
>> 
>> 
>> 
>> 
>>
>>
>> On Thu, Apr 18, 2024 at 2:24 PM Dave Page  wrote:
>>
>>> Hi
>>>
>>> On Wed, 17 Apr 2024 at 15:08, Anil Sahoo 
>>> wrote:
>>>
 Hi Hackers,

 This feature is about executing a query at the cursor position. And
 that query can be a one line or multiline. I have assigned a play icon
 button and F5 as the keyboard shortcut for the Execute Query feature, and
 for Execute Script, Playlist icon button and Alt+F5(Others),Option+F5(Mac)
 as keyboard shortcut.

 As now the query can run at cursor position, so for user convenience I
 am showing the current query just beside the Data Output toolbar. And on
 hover of the text, it will show the whole query as a tooltip. This query
 text will be available for both Execute Script and Execute Query.

 I have made the UI change for the feature #6841
 .

 Please provide your suggestions and feedback if these changes look okay
 to you.

>>>
>>> How is this parsing the query to figure out the correct text to send to
>>> the server? For example, I notice you have no semi-colons on many of the
>>> queries in your test; is it breaking on newlines? What if there's a newline
>>> (or multiple of them) in the query string? How does it cope with an
>>> anonymous block containing multiple queries, or a pl/whatever function
>>> definition that might contain queries within its text? Or a view definition?
>>>
>>> --
>>> Dave Page
>>> pgAdmin: https://www.pgadmin.org
>>> PostgreSQL: https://www.postgresql.org
>>> EDB: https://www.enterprisedb.com
>>>
>>>
>
> --
> Dave Page
> pgAdmin: https://www.pgadmin.org
> PostgreSQL: https://www.postgresql.org
> EDB: https://www.enterprisedb.com
>
>

-- 
Thanks,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*

"Don't 

Re: Regarding feature #6841

[Thom Brown]

On Thu, Apr 18, 2024, 15:37 Dave Page  wrote:

> Hi
>
> On Thu, 18 Apr 2024 at 15:26, Anil Sahoo 
> wrote:
>
>> Hi Dave,
>> We took help from Code Mirror, i.e Code Mirror gives the parsed SQL from
>> the editor through a tree called syntaxTree and by using some logic we
>> extracted the statements which have semicolon in it and also added some
>> extra logic to break the whole query on next of next line as empty or if
>> comments are there.
>>
>> Using all this logic we got the individual queries and checked where our
>> cursor is in editor and checked with the query and through this we got the
>> actual query at cursor position.
>>
>> For example,
>>
>>1. if the cursor is at starting or ending position or anywhere in
>>between a query with semicolon or without semicolon, that can be single
>>line or multi line then the query gets extracted.
>>2. if the cursor is at starting or ending position or anywhere in
>>between a comment that can be single line or multi line then the comment
>>gets extracted.
>>3. if the cursor is at a position where the previous line has a query
>>then that query gets extracted.
>>
>> For the anonymous block containing multiple queries, code mirror gives
>> the statements differently. That is an incomplete query we can say, so the
>> query tool gives error. We can say some limitations are there with Code
>> Mirror.
>>
>> Please let me know if you have any questions on this.
>>
>
> My main concern is that it doesn't get it wrong. Ever. Consider:
>
> DELETE FROM foo; SELECT * FROM foo;
>
> Is that one statement or two? What if it's in the middle of a pl/python3
> function:
>
> my_sql = 'DELETE FROM foo; SELECT * FROM foo;'
>
> or
>
> my_sql = """DELETE FROM foo;
> SELECT * FROM foo;
> """
>

Or, indeed, an SQL function. Would the user want the CREATE statement to be
run, or just the embedded SQL statement? I wouldn't like to guess.

Thom

Re: Regarding feature #6841

[Thom Brown]

On Thu, Apr 18, 2024, 15:26 Anil Sahoo  wrote:

> Hi Dave,
> We took help from Code Mirror, i.e Code Mirror gives the parsed SQL from
> the editor through a tree called syntaxTree and by using some logic we
> extracted the statements which have semicolon in it and also added some
> extra logic to break the whole query on next of next line as empty or if
> comments are there.
>
> Using all this logic we got the individual queries and checked where our
> cursor is in editor and checked with the query and through this we got the
> actual query at cursor position.
>
> For example,
>
>1. if the cursor is at starting or ending position or anywhere in
>between a query with semicolon or without semicolon, that can be single
>line or multi line then the query gets extracted.
>2. if the cursor is at starting or ending position or anywhere in
>between a comment that can be single line or multi line then the comment
>gets extracted.
>3. if the cursor is at a position where the previous line has a query
>then that query gets extracted.
>
> For the anonymous block containing multiple queries, code mirror gives the
> statements differently. That is an incomplete query we can say, so the
> query tool gives error. We can say some limitations are there with Code
> Mirror.
>
> Please let me know if you have any questions on this.
>

I guess my first question is, what is the requirement being fulfilled here?

Also, I have more experience with selecting what to run, and having it run
whatever was selected. I don't see the current proposal as particularly
intuitive. Having the cursor midway through a statement (or even the last
line of a statement) doesn't quite feel right. What would happen in this
example?...

SELECT contents
FROM |mytable
WHERE id BETWEEN 101 AND 200

ORDER BY id ASC;

Note I placed a pipe to represent the cursor before the table name. And
would it make a difference if the first 3 lines were a single line?

In any case, given all statements are separated by semicolons in
PostgreSQL, I am resistant to the idea of supporting them without for
specific features.

Regards

Thom

Re: Regarding feature #6841

[Dave Page]

Hi

On Thu, 18 Apr 2024 at 15:26, Anil Sahoo 
wrote:

> Hi Dave,
> We took help from Code Mirror, i.e Code Mirror gives the parsed SQL from
> the editor through a tree called syntaxTree and by using some logic we
> extracted the statements which have semicolon in it and also added some
> extra logic to break the whole query on next of next line as empty or if
> comments are there.
>
> Using all this logic we got the individual queries and checked where our
> cursor is in editor and checked with the query and through this we got the
> actual query at cursor position.
>
> For example,
>
>1. if the cursor is at starting or ending position or anywhere in
>between a query with semicolon or without semicolon, that can be single
>line or multi line then the query gets extracted.
>2. if the cursor is at starting or ending position or anywhere in
>between a comment that can be single line or multi line then the comment
>gets extracted.
>3. if the cursor is at a position where the previous line has a query
>then that query gets extracted.
>
> For the anonymous block containing multiple queries, code mirror gives the
> statements differently. That is an incomplete query we can say, so the
> query tool gives error. We can say some limitations are there with Code
> Mirror.
>
> Please let me know if you have any questions on this.
>

My main concern is that it doesn't get it wrong. Ever. Consider:

DELETE FROM foo; SELECT * FROM foo;

Is that one statement or two? What if it's in the middle of a pl/python3
function:

my_sql = 'DELETE FROM foo; SELECT * FROM foo;'

or

my_sql = """DELETE FROM foo;
SELECT * FROM foo;
"""

(those are just simple examples from the top of my head).

It could be extremely dangerous if we or CodeMirror mis-parses something,
which seems quite possible unless it has access to the actual parser that
PostgreSQL uses. Which makes me think... what of EPAS? It has an extended
parser to handle some of the Oracle compatible syntax. Will CodeMirror get
that right?


>
> Regards
> Anil
> --
>
> 
>
> *Anil Sahoo*
>
> Software Engineer
>
> www.enterprisedb.com
>
> Power to Postgres
>
> 
> 
> 
> 
>
>
> On Thu, Apr 18, 2024 at 2:24 PM Dave Page  wrote:
>
>> Hi
>>
>> On Wed, 17 Apr 2024 at 15:08, Anil Sahoo 
>> wrote:
>>
>>> Hi Hackers,
>>>
>>> This feature is about executing a query at the cursor position. And that
>>> query can be a one line or multiline. I have assigned a play icon button
>>> and F5 as the keyboard shortcut for the Execute Query feature, and for
>>> Execute Script, Playlist icon button and Alt+F5(Others),Option+F5(Mac) as
>>> keyboard shortcut.
>>>
>>> As now the query can run at cursor position, so for user convenience I
>>> am showing the current query just beside the Data Output toolbar. And on
>>> hover of the text, it will show the whole query as a tooltip. This query
>>> text will be available for both Execute Script and Execute Query.
>>>
>>> I have made the UI change for the feature #6841
>>> .
>>>
>>> Please provide your suggestions and feedback if these changes look okay
>>> to you.
>>>
>>
>> How is this parsing the query to figure out the correct text to send to
>> the server? For example, I notice you have no semi-colons on many of the
>> queries in your test; is it breaking on newlines? What if there's a newline
>> (or multiple of them) in the query string? How does it cope with an
>> anonymous block containing multiple queries, or a pl/whatever function
>> definition that might contain queries within its text? Or a view definition?
>>
>> --
>> Dave Page
>> pgAdmin: https://www.pgadmin.org
>> PostgreSQL: https://www.postgresql.org
>> EDB: https://www.enterprisedb.com
>>
>>

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

Re: Regarding feature #6841

[Anil Sahoo]

Hi Dave,
We took help from Code Mirror, i.e Code Mirror gives the parsed SQL from
the editor through a tree called syntaxTree and by using some logic we
extracted the statements which have semicolon in it and also added some
extra logic to break the whole query on next of next line as empty or if
comments are there.

Using all this logic we got the individual queries and checked where our
cursor is in editor and checked with the query and through this we got the
actual query at cursor position.

For example,

   1. if the cursor is at starting or ending position or anywhere in
   between a query with semicolon or without semicolon, that can be single
   line or multi line then the query gets extracted.
   2. if the cursor is at starting or ending position or anywhere in
   between a comment that can be single line or multi line then the comment
   gets extracted.
   3. if the cursor is at a position where the previous line has a query
   then that query gets extracted.

For the anonymous block containing multiple queries, code mirror gives the
statements differently. That is an incomplete query we can say, so the
query tool gives error. We can say some limitations are there with Code
Mirror.

Please let me know if you have any questions on this.

Regards
Anil
--



*Anil Sahoo*

Software Engineer

www.enterprisedb.com

Power to Postgres







On Thu, Apr 18, 2024 at 2:24 PM Dave Page  wrote:

> Hi
>
> On Wed, 17 Apr 2024 at 15:08, Anil Sahoo 
> wrote:
>
>> Hi Hackers,
>>
>> This feature is about executing a query at the cursor position. And that
>> query can be a one line or multiline. I have assigned a play icon button
>> and F5 as the keyboard shortcut for the Execute Query feature, and for
>> Execute Script, Playlist icon button and Alt+F5(Others),Option+F5(Mac) as
>> keyboard shortcut.
>>
>> As now the query can run at cursor position, so for user convenience I am
>> showing the current query just beside the Data Output toolbar. And on hover
>> of the text, it will show the whole query as a tooltip. This query text
>> will be available for both Execute Script and Execute Query.
>>
>> I have made the UI change for the feature #6841
>> .
>>
>> Please provide your suggestions and feedback if these changes look okay
>> to you.
>>
>
> How is this parsing the query to figure out the correct text to send to
> the server? For example, I notice you have no semi-colons on many of the
> queries in your test; is it breaking on newlines? What if there's a newline
> (or multiple of them) in the query string? How does it cope with an
> anonymous block containing multiple queries, or a pl/whatever function
> definition that might contain queries within its text? Or a view definition?
>
> --
> Dave Page
> pgAdmin: https://www.pgadmin.org
> PostgreSQL: https://www.postgresql.org
> EDB: https://www.enterprisedb.com
>
>

Re: Regarding feature #6841

[Dave Page]

Hi

On Wed, 17 Apr 2024 at 15:08, Anil Sahoo 
wrote:

> Hi Hackers,
>
> This feature is about executing a query at the cursor position. And that
> query can be a one line or multiline. I have assigned a play icon button
> and F5 as the keyboard shortcut for the Execute Query feature, and for
> Execute Script, Playlist icon button and Alt+F5(Others),Option+F5(Mac) as
> keyboard shortcut.
>
> As now the query can run at cursor position, so for user convenience I am
> showing the current query just beside the Data Output toolbar. And on hover
> of the text, it will show the whole query as a tooltip. This query text
> will be available for both Execute Script and Execute Query.
>
> I have made the UI change for the feature #6841
> .
>
> Please provide your suggestions and feedback if these changes look okay to
> you.
>

How is this parsing the query to figure out the correct text to send to the
server? For example, I notice you have no semi-colons on many of the
queries in your test; is it breaking on newlines? What if there's a newline
(or multiple of them) in the query string? How does it cope with an
anonymous block containing multiple queries, or a pl/whatever function
definition that might contain queries within its text? Or a view definition?

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com

Re: Regarding feature #6841

[Anil Sahoo]

Hi Aditya,

Sounds good, I will work on this.

Thanks for the suggestion.

--



*Anil Sahoo*

Software Engineer

www.enterprisedb.com

Power to Postgres







On Wed, Apr 17, 2024 at 9:24 PM Aditya Toshniwal <
aditya.toshni...@enterprisedb.com> wrote:

> Hi Anil,
>
> I think the toolbar query should look like an input box. And the tooltip
> with a codemirror is more UI friendly.
>
> On Wed, Apr 17, 2024 at 7:38 PM Anil Sahoo 
> wrote:
>
>> Hi Hackers,
>>
>> This feature is about executing a query at the cursor position. And that
>> query can be a one line or multiline. I have assigned a play icon button
>> and F5 as the keyboard shortcut for the Execute Query feature, and for
>> Execute Script, Playlist icon button and Alt+F5(Others),Option+F5(Mac) as
>> keyboard shortcut.
>>
>> As now the query can run at cursor position, so for user convenience I am
>> showing the current query just beside the Data Output toolbar. And on hover
>> of the text, it will show the whole query as a tooltip. This query text
>> will be available for both Execute Script and Execute Query.
>>
>> I have made the UI change for the feature #6841
>> .
>>
>> Please provide your suggestions and feedback if these changes look okay
>> to you.
>>
>> Regards,
>> Anil
>> --
>>
>> 
>>
>> *Anil Sahoo*
>>
>> Software Engineer
>>
>> www.enterprisedb.com
>>
>> Power to Postgres
>>
>> 
>> 
>> 
>> 
>>
>
>
> --
> Thanks,
> Aditya Toshniwal
> pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*
> 
> "Don't Complain about Heat, Plant a TREE"
>

Re: Regarding feature #6841

[Aditya Toshniwal]

Hi Anil,

I think the toolbar query should look like an input box. And the tooltip
with a codemirror is more UI friendly.

On Wed, Apr 17, 2024 at 7:38 PM Anil Sahoo 
wrote:

> Hi Hackers,
>
> This feature is about executing a query at the cursor position. And that
> query can be a one line or multiline. I have assigned a play icon button
> and F5 as the keyboard shortcut for the Execute Query feature, and for
> Execute Script, Playlist icon button and Alt+F5(Others),Option+F5(Mac) as
> keyboard shortcut.
>
> As now the query can run at cursor position, so for user convenience I am
> showing the current query just beside the Data Output toolbar. And on hover
> of the text, it will show the whole query as a tooltip. This query text
> will be available for both Execute Script and Execute Query.
>
> I have made the UI change for the feature #6841
> .
>
> Please provide your suggestions and feedback if these changes look okay to
> you.
>
> Regards,
> Anil
> --
>
> 
>
> *Anil Sahoo*
>
> Software Engineer
>
> www.enterprisedb.com
>
> Power to Postgres
>
> 
> 
> 
> 
>


-- 
Thanks,
Aditya Toshniwal
pgAdmin Hacker | Sr. Software Architect | *enterprisedb.com*

"Don't Complain about Heat, Plant a TREE"