On 08.03.23 22:40, Andrew Dunstan wrote:
> There are probably some fairly easy opportunities to reduce the number
> of non-terminals introduced here (e.g. I think json_aggregate_func could
> possibly be expanded in place without introducing
> json_object_aggregate_constructor and json_array_aggregate_constructor).
> I'm going to make an attempt at that, at least to pick some low hanging
> fruit. But in the end I think we are going to be left with a significant
> expansion of the grammar rules, more or less forced on us by the way the
> SQL Standards Committee rather profligately invents new ways of
> contorting the grammar.
I browsed these patches, and I agree that the grammar is the thing that
sticks out as something that could be tightened up a bit. Try to reduce
the number of different symbols, and check that the keywords are all in
alphabetical order.
There are also various bits of code that are commented out, in some
cases because they can't be implemented, in some cases without
explanation. I think these should all be removed. Otherwise, whoever
needs to touch this code next would be under some sort of obligation to
keep the commented-out code up to date with surrounding changes, which
would be awkward. We can find better ways to explain missing
functionality and room for improvement.
Also, perhaps we can find better names for the new test files. Like,
what does "sqljson.sql" mean, as opposed to, say, "json.sql"? Maybe
something like "json_functions", "json_expressions", etc. would be
clearer. (Starting it with "json" would also group the files better.)
> These both seem like things not worth holding up progress for, and I
> think it would be good to get these patches committed as soon as
> possible. My intention is to commit them (after some grammar
> adjustments) plus their documentation in the next few days.
If possible, the documentation for each incremental part should be part
of that patch, not a separate all-in-one patch.