Subject: Re: remaining sql/json patches

From: Andrew Dunstan <[email-masked]>
Date: Mon, 27 Nov 2023 08:56:43 -0500

On 2023-11-27 Mo 05:42, Alvaro Herrera wrote:
> On 2023-Nov-27, Amit Langote wrote:
>
>>>   For example, the jump between 13 and 14 looks worse.
>>> (I do wonder what happened there.)
>> The following commit sounds like it might be related?
> Yes, but not only that one.  I did some more trolling in the commit log
> for the 14 timeframe further and found that the following commits are
> the ones with highest additions to YYLAST during that cycle:
>
>   yylast │ yylast_addition │   commit   │                                    subject
> ────────┼─────────────────┼────────────┼────────────────────────────────────────────────────────────────────────────────
>   106051 │            1883 │ 92bf7e2d02 │ Provide the OR REPLACE option for CREATE TRIGGER.
>   105325 │            1869 │ 06a7c3154f │ Allow most keywords to be used as column labels without requiring AS.
>   104395 │            1816 │ 45b9805706 │ Allow CURRENT_ROLE where CURRENT_USER is accepted
>   107537 │            1139 │ a4d75c86bf │ Extended statistics on expressions
>   105410 │            1067 │ b5913f6120 │ Refactor CLUSTER and REINDEX grammar to use DefElem for option lists
>   106007 │             965 │ 3696a600e2 │ SEARCH and CYCLE clauses
>   106864 │             733 │ be45be9c33 │ Implement GROUP BY DISTINCT
>   105886 │             609 │ 844fe9f159 │ Add the ability for the core grammar to have more than one parse target.
>   108400 │             571 │ ec48314708 │ Revert per-index collation version tracking feature.
>   108939 │             539 │ e6241d8e03 │ Rethink definition of pg_attribute.attcompression.
>
> but we also have these:
>
>   105521 │            -530 │ 926fa801ac │ Remove undocumented IS [NOT] OF syntax.
>   104202 │            -640 │ c4325cefba │ Fold AlterForeignTableStmt into AlterTableStmt
>   104168 │            -718 │ 40c24bfef9 │ Improve our ability to regurgitate SQL-syntax function calls.
>   108111 │            -828 │ e56bce5d43 │ Reconsider the handling of procedure OUT parameters.
>   106398 │            -834 │ 71f4c8c6f7 │ ALTER TABLE ... DETACH PARTITION ... CONCURRENTLY
>   104402 │            -923 │ 2453ea1422 │ Support for OUT parameters in procedures
>   103456 │            -939 │ 1ed6b89563 │ Remove support for postfix (right-unary) operators.
>   104343 │           -1178 │ 873ea9ee69 │ Refactor parsing rules for option lists of EXPLAIN, VACUUM and ANALYZE
>   102784 │           -1417 │ 8f5b596744 │ Refactor AlterExtensionContentsStmt grammar
> (59 filas)
>

Interesting. But inferring a speed effect from such changes is 
difficult. I don't have a good idea about measuring parser speed, but a 
tool to do that would be useful. Amit has made a start on such 
measurements, but it's only a start. I'd prefer to have evidence rather 
than speculation.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

---

From: Alvaro Herrera <[email-masked]>
Date: Mon, 27 Nov 2023 15:06:12 +0100

On 2023-Nov-27, Andrew Dunstan wrote:

> Interesting. But inferring a speed effect from such changes is difficult. I
> don't have a good idea about measuring parser speed, but a tool to do that
> would be useful. Amit has made a start on such measurements, but it's only a
> start. I'd prefer to have evidence rather than speculation.

At this point one thing that IMO we cannot afford to do, is stop feature
progress work on the name of parser speed.  I mean, parser speed is
important, and we need to be mindful that what we add is reasonable.
But at some point we'll probably have to fix that by parsing
differently (a top-down parser, perhaps?  Split the parser in smaller
pieces that each deal with subsets of the whole thing?)

Peter told me earlier today that he noticed that the parser changes he
proposed made the parser source code smaller, they result in larger
parser tables (in terms of the number of states, I think he said).  But
source code maintainability is also very important, so my suggestion
would be that those changes be absorbed into Amit's commits nonetheless.

The amount of effort spent on the parsing aspect on this thread seems in
line with what we should always be doing: keep an eye on it, but not
disregard the work just because the parser tables have grown.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"La persona que no quería pecar / estaba obligada a sentarse
 en duras y empinadas sillas    / desprovistas, por cierto
 de blandos atenuantes"                          (Patricio Vogel)

---

From: Andres Freund <[email-masked]>
Date: Mon, 27 Nov 2023 09:50:32 -0800

Hi,

> > start. I'd prefer to have evidence rather than speculation.

Yea, the parser table sizes are influenced by the increase in complexity of
the grammar, but it's not a trivial correlation. Bison attempts to compress
the state space and it looks like there are some heuristics involved.

> progress work on the name of parser speed.

Agreed - I don't think anyone advocated that though.

> But at some point we'll probably have to fix that by parsing differently (a
> top-down parser, perhaps?  Split the parser in smaller pieces that each deal
> with subsets of the whole thing?)

Yea. Both perhaps. Being able to have sub-grammars would be quite powerful I
think, and we might be able to do it without loosing cross-checking from bison
that our grammar is conflict free.  Even if the resulting combined state space
is larger, better locality should more than make up for that.

> disregard the work just because the parser tables have grown.

I think we've, in other threads, not paid enough attention to it and just
added stuff to the grammar in the first way that didn't produce shift/reduce
conflicts... Of course a decent part of the problem here is the SQL standard
that so seems to like adding one-off forms of grammar (yes,
func_expr_common_subexpr, I'm looking at you)...

Greetings,

Andres Freund

---

From: John Naylor <[email-masked]>
Date: Mon, 22 Jan 2024 13:19:17 +0700

> pieces that each deal with subsets of the whole thing?)

I was reorganizing some old backups and rediscovered an experiment I
did four years ago when I had some extra time on my hands, to use a
lexer generator that emits a state machine driven by code, rather than
a table. It made parsing 12% faster on the above info-schema test, but
only (maybe) 3% on parsing pgbench-like queries. My quick hack ended
up a bit uglier and more verbose than Flex, but that could be
improved, and in fact small components could be shared across the
whole code base. I might work on it again; I might not.