Re: extensible options syntax for replication parser? - Mailing list pgsql-hackers

Hello Robert,

My 0.02 €:

> It seems to me that we're making the same mistake with the replication
> parser that we've made in various placesin the regular parser: using a
> syntax for options that requires that every potential option be a
> keyword, and every potential option requires modification of the
> grammar. In particular, the syntax for the BASE_BACKUP command has
> accreted a whole lot of cruft already and I think that trend is likely
> to continue. I don't think that trying to keep people from adding
> options is a good strategy,

Indeed.

> so instead I'd like to have a better way to do it.

> Attached is v1 of a patch to refactor things so that parts of the 
> BASE_BACKUP and CREATE_REPLICATION_SLOT are replaced with a flexible 
> options syntax.

Patch applies cleanly, however compilation fails on:

   repl_gram.y:271:106: error: expected ‘;’ before ‘}’

Getting rid of "ident_or_keyword", some day, would be a relief.

For boolean options, you are generating (EXPORT_SNAPSHOT TRUE) where
(EXPORT_SNAPSHOT) would do.

Maybe allowing (!EXPORT_SNAPSHOT) for (FOO FALSE) would be nice, if it is 
not allowed yet. That would also allow to get rid of FOO/NOFOO variants if 
any for FOO & !FOO, so one keyword is enough for a concept.

> There are some debatable decisions here, so I'd be happy to get some 
> feedback on whether to go further with this, or less far, or maybe even 
> just abandon the idea altogether. I doubt the last one is the right 
> course, though: ISTM something's got to be done about the BASE_BACKUP 
> case, at least.

ISTM that it would be better to generalize the approach to all commands 
which accept options, so that the syntax is homogeneous.

> If people agree with the basic approach, I'll write docs. The
> intention is that we'd continue to support the existing syntax for the
> existing options, but the client tools would be adjusted to use the
> new syntax if the server's new enough, and any new options would be
> supported only through the new syntax.

Yes.

> At some point in the distant future we could retire the old syntax, when 
> we've stopped caring about compatibility with pre-14 versions.

Just wondering: ISTM that the patch implies that dumping a v14 db 
generates the new syntax, which makes sense. Now I see 4 use cases wrt to 
version.

  #  source    target   comment
  1  v < 14    v < 14   probably the dump would use one of the older version
  2  v < 14    v >= 14  upgrade
  3  v >= 14   v < 14   downgrade: oops, the output uses the new syntax
  4  v >= 14   v >= 14  ok

Both cross version usages may be legitimate. In particular, 3 (oops, 
hardware issue, I have to move the db to a server where pg has not been 
upgraded) seems not possible because the generated syntax uses the new 
approach. Should/could there be some option to tell "please generate vXXX 
syntax" to allow that?

-- 
Fabien.