Re: Regarding feature #6841

From: Dave Page <dpage(at)pgadmin(dot)org>
To: Anil Sahoo <anil(dot)sahoo(at)enterprisedb(dot)com>
Cc: pgadmin-hackers(at)postgresql(dot)org
Subject: Re: Regarding feature #6841
Date: 2024-04-18 14:37:17
Message-ID: CA+OCxoxMdX9Lsyugx7=_CZ3Z6K9jxHVTOBu4q=5_1cwgXsP7LQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Hi

On Thu, 18 Apr 2024 at 15:26, Anil Sahoo <anil(dot)sahoo(at)enterprisedb(dot)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
> --
>
> <http://www.enterprisedb.com>
>
> *Anil Sahoo*
>
> Software Engineer
>
> www.enterprisedb.com
>
> Power to Postgres
>
> <https://www.linkedin.com/company/edbpostgres>
> <https://twitter.com/edbpostgres?lang=en>
> <https://www.facebook.com/EDBpostgres>
> <https://www.instagram.com/EDBpostgres/>
>
>
> On Thu, Apr 18, 2024 at 2:24 PM Dave Page <dpage(at)pgadmin(dot)org> wrote:
>
>> Hi
>>
>> On Wed, 17 Apr 2024 at 15:08, Anil Sahoo <anil(dot)sahoo(at)enterprisedb(dot)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
>>> <https://github.com/pgadmin-org/pgadmin4/issues/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

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Thom Brown 2024-04-18 14:56:17 Re: Regarding feature #6841
Previous Message Anil Sahoo 2024-04-18 14:25:56 Re: Regarding feature #6841