Обсуждение: Regarding feature #6841

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.

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 <dpage@pgadmin.org> wrote:

Hi

On Wed, 17 Apr 2024 at 15:08, Anil Sahoo <anil.sahoo@enterprisedb.com> 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

Hi

On Thu, 18 Apr 2024 at 15:26, Anil Sahoo <anil.sahoo@enterprisedb.com> 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 <dpage@pgadmin.org> wrote:

Hi

On Wed, 17 Apr 2024 at 15:08, Anil Sahoo <anil.sahoo@enterprisedb.com> 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

On Thu, Apr 18, 2024, 15:26 Anil Sahoo <anil.sahoo@enterprisedb.com> 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

Hi Dave,

On Thu, Apr 18, 2024 at 8:07 PM Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thu, 18 Apr 2024 at 15:26, Anil Sahoo <anil.sahoo@enterprisedb.com> 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.

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

Hi Dave,

On Thu, Apr 18, 2024 at 8:07 PM Dave Page <dpage@pgadmin.org> wrote:

Hi

On Thu, 18 Apr 2024 at 15:26, Anil Sahoo <anil.sahoo@enterprisedb.com> 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

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.

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.

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

Hi Dave,

On Fri, Apr 19, 2024 at 6:22 PM Dave Page <dpage@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 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"

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 PM Dave Page <dpage@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 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

Hi Dave,

On Fri, Apr 19, 2024 at 7:05 PM Dave Page <dpage@pgadmin.org> 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 PM Dave Page <dpage@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 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"

Hi Dave,

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

Hi Dave,

On Fri, Apr 19, 2024 at 7:05 PM Dave Page <dpage@pgadmin.org> 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 PM Dave Page <dpage@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 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.

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 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi Dave,

On Fri, Apr 19, 2024 at 7:05 PM Dave Page <dpage@pgadmin.org> 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 PM Dave Page <dpage@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 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.

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.

On Tue, Apr 23, 2024, 09:15 Dave Page <dpage@pgadmin.org> 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 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi Dave,

On Fri, Apr 19, 2024 at 7:05 PM Dave Page <dpage@pgadmin.org> 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 PM Dave Page <dpage@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 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.

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.

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 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 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi Dave,

On Fri, Apr 19, 2024 at 7:05 PM Dave Page <dpage@pgadmin.org> 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 PM Dave Page <dpage@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 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.

So the plan is:

1) We automatically highlight the "current" query in the editor, similarly to the mockup above.

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

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

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

5) The confirmation dialogue will have a "Don't show this again" option for those that trust the CodeMirror parser enough.

6) A button above the resultset will be added to allow you to see the query that was executed to generate that resultset in all cases.

A button above the resultset? Do you mean a tab, or an actual button?

A button, alongside the ones that are already there for editing data etc. 

 

I guess I would like to understand the rationale for this feature. Users supposedly want this as described, but what is the precedent? I mean, I've used GUIs for other DMBS's where you select what you want to run, and it just runs the selection, even if it doesn't grab the whole query (e.g. excluding ORDER BY or WHERE predicates).

Other GUIs (for PostgreSQL and other DBMSs) have this functionality, and we've had multiple requests for it.

 

The latter seems more flexible and predictable IMHO. And that way you can dispense with the confirmation dialogue, and there's no need for any additional configuration options because there's probably no need to disable it.

You've been able to do the "Select and run" thing for years. If you select text in the editor and hit the execute button, only the selected text is sent to the server. If nothing is selected, the entire string is sent. This feature will complement that for convenience, but for safety will have a separate button/shortcut.

You've been able to do the "Select and run" thing for years. If you select text in the editor and hit the execute button, only the selected text is sent to the server. If nothing is selected, the entire string is sent. This feature will complement that for convenience, but for safety will have a separate button/shortcut.

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

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

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

On Tue, 23 Apr 2024 at 12:03, Thom Brown <thom@linux.com> 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.

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 <dpage@pgadmin.org> 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 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi Dave,

On Fri, Apr 19, 2024 at 7:05 PM Dave Page <dpage@pgadmin.org> 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 PM Dave Page <dpage@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 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.

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

Hi

On Wed, 24 Apr 2024 at 12:31, Anil Sahoo <anil.sahoo@enterprisedb.com> 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 <dpage@pgadmin.org> 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 PM Aditya Toshniwal <aditya.toshniwal@enterprisedb.com> wrote:

Hi Dave,

On Fri, Apr 19, 2024 at 7:05 PM Dave Page <dpage@pgadmin.org> 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 PM Dave Page <dpage@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 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.

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

--

Dave Page

pgAdmin: https://www.pgadmin.org

PostgreSQL: https://www.postgresql.org

EDB: https://www.enterprisedb.com

On Tue, 23 Apr 2024 at 12:03, Thom Brown <thom@linux.com> 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.

On Tue, 23 Apr 2024 at 13:50, Dave Page <dpage@pgadmin.org> wrote:

On Tue, 23 Apr 2024 at 12:03, Thom Brown <thom@linux.com> 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

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.