Parser Hook

Hi,

On 2021-02-22 11:20:54 -0500, Jim Mlodgenski wrote:

As Jan mentioned in his thread about a pluggable wire protocol [0], AWS is
working on a set of extensions for Babelfish. The intention is to not
necessarily have it as a single monolithic extension, but be possible for
people to use pieces of it as they need when they are migrating to
PostgreSQL. Some may just need the functions or data types. Others may need
the stored procedure language. Many times when enterprises are migrating
databases, they have satellite applications that they may not be able to
change or they are on a different schedules than the main application so
the database still needs to support some of the old syntax. A common need
in these situations is the parser.

Attached is a patch to place a hook at the top of the parser to allow for a
pluggable parser. It is modeled after the planner_hook [1]. To test the
hook, I have also attached a simple proof of concept that wraps the parser
in a TRY/CATCH block to catch any parse errors. That could potentially help
a class of users who are sensitive to parse errors ending up in the logs
and leaking PII data or passwords.

I don't think these are really comparable. In case of the planner hook
you can reuse the normal planner pieces, and just deal with the one part
you need to extend. But we have pretty much no infrastructure to use the
parser in a piecemeal fashion (there's a tiny bit for plpgsql).

Which in turn means that to effectively use the proposed hook to
*extend* what postgres accepts, you need to copy the existing parser,
and hack in your extensions. Which in turn invariably will lead to
complaints about parser changes / breakages the community will get
complaints about in minor releases etc.

I think the cost incurred for providing a hook that only allows
extensions to replace the parser with a modified copy of ours will be
higher than the gain. Note that I'm not saying that I'm against
extending the parser, or hooks - just that I don't think just adding the
hook is a step worth doing on its own.

Imo a functional approach would really need to do the work to allow to
extend & reuse the parser in a piecemeal fashion and *then* add a hook.

Greetings,

Andres Freund

On Mon, Feb 22, 2021 at 3:52 PM Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2021-02-22 11:20:54 -0500, Jim Mlodgenski wrote:

As Jan mentioned in his thread about a pluggable wire protocol [0], AWS

is

working on a set of extensions for Babelfish. The intention is to not
necessarily have it as a single monolithic extension, but be possible for
people to use pieces of it as they need when they are migrating to
PostgreSQL. Some may just need the functions or data types. Others may

need

the stored procedure language. Many times when enterprises are migrating
databases, they have satellite applications that they may not be able to
change or they are on a different schedules than the main application so
the database still needs to support some of the old syntax. A common need
in these situations is the parser.

Attached is a patch to place a hook at the top of the parser to allow

for a

pluggable parser. It is modeled after the planner_hook [1]. To test the
hook, I have also attached a simple proof of concept that wraps the

parser

in a TRY/CATCH block to catch any parse errors. That could potentially

help

a class of users who are sensitive to parse errors ending up in the logs
and leaking PII data or passwords.

I don't think these are really comparable. In case of the planner hook
you can reuse the normal planner pieces, and just deal with the one part
you need to extend. But we have pretty much no infrastructure to use the
parser in a piecemeal fashion (there's a tiny bit for plpgsql).

Which in turn means that to effectively use the proposed hook to
*extend* what postgres accepts, you need to copy the existing parser,
and hack in your extensions. Which in turn invariably will lead to
complaints about parser changes / breakages the community will get
complaints about in minor releases etc.

Going deeper on this, I created another POC as an example. Yes, having a
hook at the top of the parser does mean an extension needs to copy the
existing grammar and modify it. Without a total redesign of how the grammar
is handled, I'm not seeing how else this could be accomplished. The example
I have is adding a CREATE JOB command that a scheduler may use. The amount
of effort needed for an extension maintainer doesn't appear to be that
onerous. Its not ideal having to copy and patch gram.y, but certainly
doable for someone wanting to extend the parser. I also extended the patch
to add another hook in parse_expr.c to see what we would need to add
another keyword and have it call a function like SYSDATE. That appears to
be a lot of work to get all of the potentail hook points that an extension
may want to add and there may not be that many usecases worth the effort.

I think the cost incurred for providing a hook that only allows
extensions to replace the parser with a modified copy of ours will be
higher than the gain. Note that I'm not saying that I'm against
extending the parser, or hooks - just that I don't think just adding the
hook is a step worth doing on its own.

However we would want to modify the parser to allow it to be more plugable
in the future, we would very likely need to have a hook at the top of the
parser to intiailize things like keywords. Having a hook at the top of the
parser along with the existing ProcessUtility_hook allows extension to add
their own utility commands if they wish. I would image that covers many
existing use cases for extensions today.

On Mon, Mar 15, 2021 at 11:48:58AM -0400, Jim Mlodgenski wrote:

Going deeper on this, I created another POC as an example. Yes, having a
hook at the top of the parser does mean an extension needs to copy the
existing grammar and modify it. Without a total redesign of how the grammar
is handled, I'm not seeing how else this could be accomplished. The example
I have is adding a CREATE JOB command that a scheduler may use. The amount
of effort needed for an extension maintainer doesn't appear to be that
onerous. Its not ideal having to copy and patch gram.y, but certainly
doable for someone wanting to extend the parser.

AFAIK nothing in bison prevents you from silently ignoring unhandled grammar
rather than erroring out. So you could have a parser hook called first, and
if no valid command was recognized fall back on the original parser. I'm not
saying that it's a good idea or will be performant (although the added grammar
will likely be very small, so it may not be that bad), but you could definitely
avoid the need to duplicate the whole grammar in each and every extension, and
allow multiple extensions extending the grammar.

That won't reduce the difficulty of producing a correct parse tree if you want
to implement some syntactic sugar for already handled DML though.

However we would want to modify the parser to allow it to be more plugable
in the future, we would very likely need to have a hook at the top of the
parser to intiailize things like keywords. Having a hook at the top of the
parser along with the existing ProcessUtility_hook allows extension to add
their own utility commands if they wish. I would image that covers many
existing use cases for extensions today.

What happens if multiple extensions want to add their own new grammar? There
will at least be possible conflicts with the additional node tags.

Also, I'm not sure that many extensions would really benefit from custom
utility command, as you can already do pretty much anything you want using SQL
functions. For instance it would be nice for hypopg to be able to support

CREATE HYPOTHETICAL INDEX ...

rather than

SELECT hypopg_create_index('CREATE INDEX...')

But really the only benefit would be autocompletion, which still wouldn't be
possible as psql autocompletion won't be extended. And even if it somehow was,
I wouldn't expect all psql clients to be setup as needed.

On Mon, Mar 15, 2021, at 16:48, Jim Mlodgenski wrote:

The example I have is adding a CREATE JOB command that a scheduler may use.

This CREATE JOB thing sounds interesting.

Are you working on adding the ability to schedule SQL-commands to run in the background,
similar to cronjob and/or adding ampersand ("&") to a command in the terminal?

I couldn't figure it out by reading the patch.
I noted the "insert into extended_parser.jobs" query,
which to me sounds like the job would be some kind of parsing job,
but that seems strange.

/Joel

On Mon, Mar 15, 2021 at 12:43 PM Julien Rouhaud <rjuju123@gmail.com> wrote:

On Mon, Mar 15, 2021 at 11:48:58AM -0400, Jim Mlodgenski wrote:

Going deeper on this, I created another POC as an example. Yes, having a
hook at the top of the parser does mean an extension needs to copy the
existing grammar and modify it. Without a total redesign of how the

grammar

is handled, I'm not seeing how else this could be accomplished. The

example

I have is adding a CREATE JOB command that a scheduler may use. The

amount

of effort needed for an extension maintainer doesn't appear to be that
onerous. Its not ideal having to copy and patch gram.y, but certainly
doable for someone wanting to extend the parser.

AFAIK nothing in bison prevents you from silently ignoring unhandled
grammar
rather than erroring out. So you could have a parser hook called first,
and
if no valid command was recognized fall back on the original parser. I'm
not
saying that it's a good idea or will be performant (although the added
grammar
will likely be very small, so it may not be that bad), but you could
definitely
avoid the need to duplicate the whole grammar in each and every extension,
and
allow multiple extensions extending the grammar.

That's a good point. That does simplify it

That won't reduce the difficulty of producing a correct parse tree if you
want
to implement some syntactic sugar for already handled DML though.

Complex DML like Oracle's outer join syntax is tricky no matter which way
you slice it.

However we would want to modify the parser to allow it to be more

plugable

in the future, we would very likely need to have a hook at the top of the
parser to intiailize things like keywords. Having a hook at the top of

the

parser along with the existing ProcessUtility_hook allows extension to

add

their own utility commands if they wish. I would image that covers many
existing use cases for extensions today.

What happens if multiple extensions want to add their own new grammar?
There
will at least be possible conflicts with the additional node tags.

The extensions would need to play nice with one another like they do with
other hooks and properly call the previous hook.

Also, I'm not sure that many extensions would really benefit from custom
utility command, as you can already do pretty much anything you want using
SQL
functions. For instance it would be nice for hypopg to be able to support

CREATE HYPOTHETICAL INDEX ...

rather than

SELECT hypopg_create_index('CREATE INDEX...')

But really the only benefit would be autocompletion, which still wouldn't
be
possible as psql autocompletion won't be extended. And even if it somehow
was,
I wouldn't expect all psql clients to be setup as needed.

Having the functionality exposed through DDL gives it a more native feel to
it for users and for some more likely use the exentions.

On Mon, Mar 15, 2021 at 12:58 PM Joel Jacobson <joel@compiler.org> wrote:

On Mon, Mar 15, 2021, at 16:48, Jim Mlodgenski wrote:

The example I have is adding a CREATE JOB command that a scheduler may
use.

This CREATE JOB thing sounds interesting.

Are you working on adding the ability to schedule SQL-commands to run in
the background,
similar to cronjob and/or adding ampersand ("&") to a command in the
terminal?

No, it was just a sample of how the parser could be extended to all an
extension like pg_cron can use CREATE JOB instead of calling a function
like SELECT cron.schedule(...)

Also, I'm not sure that many extensions would really benefit from custom
utility command, as you can already do pretty much anything you want using
SQL
functions. For instance it would be nice for hypopg to be able to support

CREATE HYPOTHETICAL INDEX ...

rather than

SELECT hypopg_create_index('CREATE INDEX...')

But really the only benefit would be autocompletion, which still wouldn't
be
possible as psql autocompletion won't be extended. And even if it somehow
was,
I wouldn't expect all psql clients to be setup as needed.

The extending parser can be interesting for two cases

a) compatibility with other databases

b) experimental supports of some features standard (current or future)

c) some experiments - using

CREATE PIPE xxx(xxx), or CREATE HYPERCUBE xxx (xxx) is more readable more
SQLish (more natural syntax) than

SELECT create_pipe('name', 'a1', 'int', ...) or SELECT ext('CREATE PIPE ...)

Possibility to work with a parser is one main reason for forking postgres.
Lot of interestings projects fail on the cost of maintaining their own fork.

Maybe a good enough possibility is the possibility to inject an own parser
called before Postgres parser. Then it can do a transformation from "CREATE
PIPE ..." to "SELECT extparse("CREATE PIPE()". There can be a switch if
returned content is string for reparsing or already prepared AST.

It can be very interesting feature.

Pavel

On Mon, Mar 15, 2021 at 06:05:52PM +0100, Pavel Stehule wrote:

Possibility to work with a parser is one main reason for forking postgres.
Lot of interestings projects fail on the cost of maintaining their own fork.

Maybe a good enough possibility is the possibility to inject an own parser
called before Postgres parser. Then it can do a transformation from "CREATE
PIPE ..." to "SELECT extparse("CREATE PIPE()". There can be a switch if
returned content is string for reparsing or already prepared AST.

Having a hook that returns a reformatted query string would definitely be
easier to write compared to generating an AST, but the overhead of parsing the
query twice plus deparsing it will probably make that approach way too
expensive in many usecases, so we shouldn't go that way.

po 15. 3. 2021 v 18:18 odesílatel Julien Rouhaud <rjuju123@gmail.com>
napsal:

On Mon, Mar 15, 2021 at 06:05:52PM +0100, Pavel Stehule wrote:

Possibility to work with a parser is one main reason for forking

postgres.

Lot of interestings projects fail on the cost of maintaining their own

fork.

Maybe a good enough possibility is the possibility to inject an own

parser

called before Postgres parser. Then it can do a transformation from

"CREATE

PIPE ..." to "SELECT extparse("CREATE PIPE()". There can be a switch if
returned content is string for reparsing or already prepared AST.

Having a hook that returns a reformatted query string would definitely be
easier to write compared to generating an AST, but the overhead of parsing
the
query twice plus deparsing it will probably make that approach way too
expensive in many usecases, so we shouldn't go that way.

yes - so it can be nice to have more possibilities.

parsing is expensive - but on today computers, the cost of parsing is low -
the optimization is significantly more expensive.

I wrote some patches in this area (all rejected by Tom :)), and a lot of
work can be done after parser and before the analysis stage. Probably, the
parser hook is not good enough, there should be an analysis stage hook too.

On Mon, Mar 15, 2021 at 06:41:36PM +0100, Pavel Stehule wrote:

po 15. 3. 2021 v 18:18 odes�latel Julien Rouhaud <rjuju123@gmail.com>
napsal:

On Mon, Mar 15, 2021 at 06:05:52PM +0100, Pavel Stehule wrote:

Possibility to work with a parser is one main reason for forking

postgres.

Lot of interestings projects fail on the cost of maintaining their own

fork.

Maybe a good enough possibility is the possibility to inject an own

parser

called before Postgres parser. Then it can do a transformation from

"CREATE

PIPE ..." to "SELECT extparse("CREATE PIPE()". There can be a switch if
returned content is string for reparsing or already prepared AST.

Having a hook that returns a reformatted query string would definitely be
easier to write compared to generating an AST, but the overhead of parsing
the
query twice plus deparsing it will probably make that approach way too
expensive in many usecases, so we shouldn't go that way.

yes - so it can be nice to have more possibilities.

parsing is expensive - but on today computers, the cost of parsing is low -
the optimization is significantly more expensive.

I wrote some patches in this area (all rejected by Tom :)), and a lot of
work can be done after parser and before the analysis stage. Probably, the
parser hook is not good enough, there should be an analysis stage hook too.

If you need an parse/analyse hook, it means that you're extending the AST, so
you probably also need executor support for that right? Or is it only to
support syntactic sugar in the analysis rather than parsing?

po 15. 3. 2021 v 18:54 odesílatel Julien Rouhaud <rjuju123@gmail.com>
napsal:

On Mon, Mar 15, 2021 at 06:41:36PM +0100, Pavel Stehule wrote:

po 15. 3. 2021 v 18:18 odesílatel Julien Rouhaud <rjuju123@gmail.com>
napsal:

On Mon, Mar 15, 2021 at 06:05:52PM +0100, Pavel Stehule wrote:

Possibility to work with a parser is one main reason for forking

postgres.

Lot of interestings projects fail on the cost of maintaining their

own

fork.

Maybe a good enough possibility is the possibility to inject an own

parser

called before Postgres parser. Then it can do a transformation from

"CREATE

PIPE ..." to "SELECT extparse("CREATE PIPE()". There can be a switch

if

returned content is string for reparsing or already prepared AST.

Having a hook that returns a reformatted query string would definitely

be

easier to write compared to generating an AST, but the overhead of

parsing

the
query twice plus deparsing it will probably make that approach way too
expensive in many usecases, so we shouldn't go that way.

yes - so it can be nice to have more possibilities.

parsing is expensive - but on today computers, the cost of parsing is

low -

the optimization is significantly more expensive.

I wrote some patches in this area (all rejected by Tom :)), and a lot of
work can be done after parser and before the analysis stage. Probably,

the

parser hook is not good enough, there should be an analysis stage hook

too.

If you need an parse/analyse hook, it means that you're extending the AST,
so
you probably also need executor support for that right? Or is it only to
support syntactic sugar in the analysis rather than parsing?

I think all necessary executor's hooks are available already. On the
executor level, I was able to do all what I wanted.

I miss just preparsing, and postparsing hooks.

Regards

Pavel

Also, I'm not sure that many extensions would really benefit from custom
utility command, as you can already do pretty much anything you want using
SQL
functions. For instance it would be nice for hypopg to be able to support

CREATE HYPOTHETICAL INDEX ...

rather than

SELECT hypopg_create_index('CREATE INDEX...')

But really the only benefit would be autocompletion, which still wouldn't
be
possible as psql autocompletion won't be extended. And even if it somehow
was,
I wouldn't expect all psql clients to be setup as needed.

"technically" speaking you are correct, usability speaking you are not. We
ran into this discussion previously when dealing with replication. There is
certainly a history to calling functions to do what the grammar (from a
usability perspective) should do and that is not really a good history. It
is just what we are all used to. Looking at what you wrote above as a DBA
or even an average developer: CREATE HYPOTHETICAL INDEX makes much more
sense than the SELECT execution.

JD

P.S. I had to write HYPOTHETICAL 4 times, I kept typing HYPOTECHNICAL :/