On Tue, Jan 30, 2024 at 07:51:45PM +0800, Tender Wang wrote:
> I refactor previes patch based on your suggestion, please review the new
> attached patch.
Thanks for the new patch. I've found the logic to be basically OK,
minus edits with the structure, the routine names and some comments.
Unfortunately, the set of regression tests was too shy because this
only tested for the case of duplicated tokens when overridding
mappings, while missing:
- ADD MAPPING that failed on a constraint failure.
- DROP MAPPING that failed with an deletion failure.
- The case of IF EXISTS with tokens supported or not supported by a
configuration's parser. That was not related to the report of this
patch, but as we've discussed it is very easy to miss the historical
distinction between the way this clause is handled.
In short, there was zero coverage for these code paths. See
DropConfigurationMapping() that was completely red, for one:
https://coverage.postgresql.org/src/backend/commands/tsearchcmds.c.gcov.html
This should be marked as covered pretty soon. I was also hesitating
about the addition of a test with REPLACE, actually..
--
Michael
