Re: extensible options syntax for replication parser?

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.