Thread

1. Parser Hook

   Jim Mlodgenski <jimmy76@gmail.com> — 2021-02-22T16:20:54Z

   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.
   
   -- Jim
   -- Amazon Web Services
   
   [0] -
   https://www.postgresql.org/message-id/flat/CAGBW59d5SjLyJLt-jwNv%2BoP6esbD8SCB%3D%3D%3D11WVe5%3DdOHLQ5wQ%40mail.gmail.com
   [1] -
   https://www.postgresql.org/message-id/flat/27516.1180053940%40sss.pgh.pa.us
   

2. Re: Parser Hook

   Andres Freund <andres@anarazel.de> — 2021-02-22T20:52:42Z

   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
   
   
   
   

3. Re: Parser Hook

   Jim Mlodgenski <jimmy76@gmail.com> — 2021-03-15T15:48:58Z

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

4. Re: Parser Hook

   Julien Rouhaud <rjuju123@gmail.com> — 2021-03-15T16:43:36Z

   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.
   
   
   
   

5. Re: Parser Hook

   Joel Jacobson <joel@compiler.org> — 2021-03-15T16:58:34Z

   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

6. Re: Parser Hook

   Jim Mlodgenski <jimmy76@gmail.com> — 2021-03-15T17:02:20Z

   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.
   

7. Re: Parser Hook

   Jim Mlodgenski <jimmy76@gmail.com> — 2021-03-15T17:05:25Z

   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(...)
   

8. Re: Parser Hook

   Pavel Stehule <pavel.stehule@gmail.com> — 2021-03-15T17:05:52Z

   > 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
   

9. Re: Parser Hook

   Julien Rouhaud <rjuju123@gmail.com> — 2021-03-15T17:18:57Z

   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.
   
   
   
   

10. Re: Parser Hook

    Pavel Stehule <pavel.stehule@gmail.com> — 2021-03-15T17:41:36Z

    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.
    

11. Re: Parser Hook

    Julien Rouhaud <rjuju123@gmail.com> — 2021-03-15T17:54:47Z

    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?
    
    
    
    

12. Re: Parser Hook

    Pavel Stehule <pavel.stehule@gmail.com> — 2021-03-15T18:02:39Z

    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
    

13. Re: Parser Hook

    Joshua D. Drake <jd@commandprompt.com> — 2021-03-15T18:39:55Z

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