On Mon, Mar 15, 2021 at 09:30:43AM -0700, Mark Dilger wrote: > > In the docs, 0001, "Fow now, the only dependency handled currently", > > "Fow now" is misspelled, and "For now" seems redundant when used with > "currently". > > > In the docs, 0002, "For now only dependency on collations are supported." > > "dependency" is singular, "are" is conjugated for plural. > > > In the docs, 0002, you forgot to update doc/src/sgml/ref/reindexdb.sgml with > the documentation for the --outdated switch.
Thanks, I'll fix those and do a full round a doc / comment proofreading. > In the tests, you check that REINDEX (OUTDATED) doesn't do anything crazy, > but you are not really testing the functionality so far as I can see, as you > don't have any tests which cause the collation to be outdated. Am I right > about that? I wonder if you could modify DefineCollation. In addition to > the providers "icu" and "libc" that it currently accepts, I wonder if it > might accept "test" or similar, and then you could create a test in > src/test/modules that compiles a "test" provider, creates a database with > indexes dependent on something from that provider, stops the database, > updates the test collation, ...? Indeed the tests in create_index.sql (and similarly in 090_reindexdb.pl) check that REINDEX (OUTDATED) will ignore non outdated indexes as expected. But there are also the tests in collate.icu.utf8.out which will fake outdated collations (that's the original tests for the collation tracking patches) and then check that outdated indexes are reindexed with both REINDEX and REINDEX (OUDATED). So I think that all cases are covered. Do you want to have more test cases?
