On 23.02.23 07:36, Jim Jones wrote:
On 23.02.23 02:52, Peter Smith wrote:
Here are my review comments for patch v17-0001.
======
src/test/regress/sql/xml.sql
The blank line(s) which previously separated the xmlserialize tests
from the xml IS [NOT] DOCUMENT tests are now missing...
v18 adds a new line in the xml.sql file to separate the xmlserialize
test cases from the rest.
In kwlist.h you have
PG_KEYWORD("indent", INDENT, UNRESERVED_KEYWORD, AS_LABEL)
but you can actually make it BARE_LABEL, which is preferable.
More importantly, you need to add the new keyword to the
bare_label_keyword production in gram.y. I thought we had some tooling
in the build system to catch this kind of omission, but it's apparently
not working right now.
Elsewhere, let's rename the xmlformat() C function to xmlserialize() (or
maybe something like xmlserialize_indent()), so the association is clearer.
