Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rxlfq-00BfWd-Tr for pgadmin-hackers@arkaria.postgresql.org; Fri, 19 Apr 2024 10:40:55 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1rxlfp-0013Hj-JT for pgadmin-hackers@arkaria.postgresql.org; Fri, 19 Apr 2024 10:40:53 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1rxlfp-0013Hb-Al for pgadmin-hackers@lists.postgresql.org; Fri, 19 Apr 2024 10:40:53 +0000 Received: from mail-lf1-x12b.google.com ([2a00:1450:4864:20::12b]) by magus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (Exim 4.94.2) (envelope-from ) id 1rxlfm-001nAh-1v for pgadmin-hackers@postgresql.org; Fri, 19 Apr 2024 10:40:52 +0000 Received: by mail-lf1-x12b.google.com with SMTP id 2adb3069b0e04-51967f75729so2151834e87.0 for ; Fri, 19 Apr 2024 03:40:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pgadmin.org; s=google; t=1713523249; x=1714128049; darn=postgresql.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Lu6ToqjlJuHQCzm7fUyRmnyR7B7/zfOMFVpRpnc0HA8=; b=W9icmB32Tylh94DXJQKgLObgMOdZZqw86ROp5aKOrCLhG6+7sna7wnlVolk+9LTTBL 8FLQoIz8R+PXLEo515V+ZG9X/Erb2D1C7ffEy2Fz+BcXmNCf+/rD+2oJ5H95TOcAQmQJ dpYFR2gaGr9actLn5HRadV8FGkRopis7Nss43826NcOHI2A6Lr9a+FwRaqyCOU9Nb076 FJyrsWZiNVfIQRVNSQof4Cna/lkY8ftltNCtJ6KgL0v18uOXViarFz5nJQB6S4Yy0pSL 3D56+If6yv+WKBwTd75FK3WZrvAnG9Eg8C9adZ/c9kV3GRvg8iDu6k37SrWYx+CYvril SREw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713523249; x=1714128049; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Lu6ToqjlJuHQCzm7fUyRmnyR7B7/zfOMFVpRpnc0HA8=; b=A4oZ3LgTXiZxNxqjGMo1crrUhFdVgVOyqmhjFOr70HAHXRKro07SuCaTstXH12QOMl 4aOkuVIT9BZbmRqmxfcLYPsZeDU5y8dWQLZaOUUXApDyEmTy0CRlfM3t8bTNr4eullhW 9QzNb7mQakT1O8uOH/r4VbfqrpVIx6JsEdcCNyRH57OCqlUhgkzhgbEqtWGLC4ZCSDtH IAeLL/O1tQmVX92DhR2ld2k8BO4reaDWtCKmoymC+f01wm3lFreCFqRmYniO8F0+pWxv N8jvBF0wGNkyspUaN+OA91fCLgMGXoO67cL1/8SnajSB/8lbE22HGodz4rtxBFRaKnVZ S3uw== X-Forwarded-Encrypted: i=1; AJvYcCW2ta3RYmPBMq+/wLQkAq5eunt1vCSPKeCj5c3gtpeUPZFGDYbc2RWs+fjn9cW7BSWuOvPJYgNmtmHROHhiENCMwYSegQukyGaEZbdF6R0= X-Gm-Message-State: AOJu0Yxqsz4vXyEViNcDTlmfMGIJBk17aGU0hsO9Tm8TmLh8d3kdUiaX fCVI5ptYnHXrJzTiGx16RoGF/mKVolZUlw/SDBAFz0im+oYMfBFjwbLHb9qDMmWyRHULWbGGyo+ aAh0bplQ06J0u9cnZoq8J+G2OLxTLIV1DIPOX X-Google-Smtp-Source: AGHT+IGE70TExCKzCfvU76MrqC2c0rZiSvRNnXtoT0P9pUjuuiPK/oY3DXV704gQ97Xz4LCuC15rxkp1BXbdlx/DpIE= X-Received: by 2002:ac2:559c:0:b0:516:d126:719a with SMTP id v28-20020ac2559c000000b00516d126719amr997842lfg.9.1713523248860; Fri, 19 Apr 2024 03:40:48 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Dave Page Date: Fri, 19 Apr 2024 11:40:37 +0100 Message-ID: Subject: Re: Regarding feature #6841 To: Aditya Toshniwal Cc: Anil Sahoo , pgadmin-hackers@postgresql.org Content-Type: multipart/alternative; boundary="00000000000090312a061670b8ae" List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --00000000000090312a061670b8ae Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=E2=80=AFPM Dave Page wro= te: > >> 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 fro= m >>> 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 i= f >>> comments are there. >>> >>> Using all this logic we got the individual queries and checked where ou= r >>> 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 sin= gle >>> 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 com= ment >>> 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 =3D 'DELETE FROM foo; SELECT * FROM foo;' >> >> or >> >> my_sql =3D """DELETE FROM foo; >> SELECT * FROM foo; >> """ >> > Since it is a part of the string, it will not run the string part. It wil= l > execute along with my_sql=3D.... > 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 tha= t >> PostgreSQL uses. Which makes me think... what of EPAS? It has an extende= d >> parser to handle some of the Oracle compatible syntax. Will CodeMirror g= et >> that right? >> > CodeMirror parser only provides parsing for standard SQL grammar. It > doesn't even understand pl/pgsql. It detects the query based on semicolon= s > 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. --=20 Dave Page pgAdmin: https://www.pgadmin.org PostgreSQL: https://www.postgresql.org EDB: https://www.enterprisedb.com --00000000000090312a061670b8ae Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


=
On Fri, 19 Apr 2024 at 05:15, Aditya = Toshniwal <aditya.t= oshniwal@enterprisedb.com> wrote:
Hi= =C2=A0Dave,

On Thu, Apr 18, 2024 at 8:07=E2=80=AFPM Dave Page <dpage@pgadmin.org>= ; wrote:
Hi
On T= hu, 18 Apr 2024 at 15:26, Anil Sahoo <anil.sahoo@enterprisedb.com> wrote:
Hi Dave,
We took help from Co= de Mirror, i.e Code Mirror gives the parsed SQL from the editor through a t= ree called syntaxTree and by using some logic we extracted the statements w= hich have semicolon in it and also added some extra logic to break the whol= e query on next of next line as empty or if comments are there.
<= br>
Using all this logic we got the individual queries and checke= d where our cursor is in editor and checked=C2=A0with the query=C2=A0and th= rough this we got the actual query at cursor position.

=
For example,=C2=A0
  1. if the cursor is at starting or e= nding position or anywhere in between a query with semicolon or without sem= icolon, 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 i= n 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 l= ine has a query then that query gets extracted.=C2=A0
F= or the anonymous block containing multiple queries, code mirror gives the s= tatements 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.<= /div>

Please let me know if you have any questions on th= is.

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

DELETE F= ROM foo; SELECT * FROM foo;
It will depend = where the cursor is and will pick one of the query, not both.=C2=A0<= /div>

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

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

or=C2=A0
<= br>
my_sql =3D """DELETE FROM foo;=C2=A0
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=3D....=C2=A0

Even if you put the cursor on the "SELEC= T"? If so, that would imply the parser understands the string quoting;= e.g. in this case, the Python multiline string. Presumably then it would a= lso understand regular single and double quotes - what about (for example) = a heredoc in a pl/sh function?

=C2=A0

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

I= t could be extremely dangerous if we or CodeMirror mis-parses something, wh= ich seems quite possible unless it has access to the actual parser that Pos= tgreSQL uses. Which makes me think... what of EPAS? It has an extended pars= er to handle some of the Oracle compatible syntax. Will CodeMirror get that= right?
CodeMirror parser only provides pars= ing for standard SQL grammar. It doesn't even understand pl/pgsql. It d= etects the query based on semicolons very effectively.=C2=A0We have adde= d our own logic to take that query provided by CM and separate it by new li= ne.
Instead of making it as the main execute=C2=A0button, I re= alise we should make it as the second execute, and keep the main execute un= touched.

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=C2=A0= 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 wi= ll lead to less severe unexpected results; the parser doesn't understan= d pl/pgsql, so by extension, it also cannot understand an anonymous functio= n written in pg/pgsql. Yet, in PostgreSQL an anonymous function (a DO state= ment) is a perfectly valid single statement that will contain sub-statement= s 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.
=C2=A0
<= span class=3D"gmail_signature_prefix">--
--00000000000090312a061670b8ae--