Re: [PATCH] reindexdb: Fix schema name lookup in parallel mode

Hi Koshi,

I tested v1 on a fresh build and I think the approach is right.
Details below.

Environment
-----------
- master at c074737f687f604b64f6ee830fccbbe854719823 (the patch
says base f1eafd911dd, but it applies cleanly with git apply on
current master, no offsets or fuzz)
- Debian bookworm container, gcc, non-root user
- ./configure --enable-tap-tests --enable-debug --enable-cassert
--prefix=/tmp/pginst

Test database: schemas "TestSchema" (needs quoting), plainschema,
emptyschema (no tables) and "16384", each with one table with a
primary key.

Before the patch (master), reindexdb -j 2 vs -j 1
-------------------------------------------------
-S '"TestSchema"' -j 2 rc=0, nothing reindexed (silent)
-S '"TestSchema"' -j 1 rc=0, reindexed
-S TestSchema -j 2 rc=0, silent
-S TestSchema -j 1 ERROR: schema "testschema" does not exist
-S nosuch -j 2 rc=0, silent
-S nosuch -j 1 ERROR: schema "nosuch" does not exist
-S emptyschema -S nosuch -j 2 rc=0, silent
-S emptyschema -S nosuch -j 1 ERROR: schema "nosuch" does not
exist
-S emptyschema -j 2/-j 1 rc=0 in both
-S '"16384"' -j 2 rc=0, silent (-j 1 reindexes it)

So the report is reproducible: in parallel mode, quoted
mixed-case names and missing schemas are silently ignored.
vacuumdb -n nosuch fails with -j 1 and -j 2 alike.

After the patch
---------------
-S '"TestSchema"' -j 2 reindexed (matches -j 1)
-S TestSchema -j 2 ERROR: schema "testschema" does not exist
-S nosuch -j 2 ERROR: schema "nosuch" does not exist
-S emptyschema -S nosuch -j 2 ERROR: schema "nosuch" does not
exist
-S emptyschema -j 2 rc=0
-S '"16384"' -j 2 reindexed
plainschema unchanged

All the cases above now match -j 1, apart from the error text (see
below) and the '-' case.

TAP
---
make -C src/bin/scripts check
Files=13, Tests=379, Result: PASS
t/090_reindexdb.pl alone: Tests=71, PASS

With only the reindexdb.c hunk reverted and the new tests kept,
t/090_reindexdb.pl fails 4 of 71 (tests 58, 62-64): the case
folding test, the quoted-name test and both checks of the
missing-schema test. So the tests do exercise the fix.

Code review
-----------
Quoting: appendStringLiteralConn() escapes the argument as a string
literal, and the ::pg_catalog.regnamespace cast then parses it as
an identifier. So there is no injection issue, and embedded
quotes or double quotes go through the identifier parser, not
through our own code. This is the same scheme vacuumdb uses.

Error messages: the error comes from the catalog query, so it
includes the query text:

reindexdb: error: query failed: ERROR: schema "nosuch" does not
exist
LINE 6: AND ns.oid IN ('nosuch'::pg_catalog.regnamespace)
reindexdb: detail: Query was: SELECT c.relname, ns.nspname ...

That is noisier than the serial "processing of database failed"
message, but it is identical in style to what -t nosuch gives in
both serial and parallel mode today, and to vacuumdb, so I would
not hold the patch for it.

Consistency: -t already uses regclass, vacuumdb uses regnamespace,
so after this patch -S is the same as -t and -n in this respect.
Failing before any REINDEX is run (the lookup happens when the
list is built) is also what -t does.

Nits
----
1. The new quoted-name test uses '"s1"', which is all lower case,
so it never exercises a name that needs the quotes to keep its
case, which is the case from the original report. A mixed-case
schema (e.g. "S1x", passed as -S '"S1x"') would cover it.
2. The comment says "makes a nonexistent schema an error, even if
there are no tables". Fine, but the same lookup is also what
makes -S TestSchema an error when only "TestSchema" exists; a
short mention could help.

regnamespace accepting OIDs and '-'
-----------------------------------
From an operations angle I agree with Takeshi that this is fine.
- '-S -' with -j 2 is accepted and matches nothing (rc=0). In
serial mode it is a syntax error ("REINDEX SCHEMA -"). Nobody
will rely on either behaviour, and vacuumdb -n - already
succeeds the same way (I checked with -j 1).
- A numeric argument is taken as an OID. A schema literally
named "16384" has to be written -S '"16384"' with -j 2, as in
serial mode, and that works after the patch. The unquoted
form in serial mode is a syntax error, which is the more
common mistake, and nobody types OIDs by hand.
- The real risk for operators is the opposite: scripts that
passed -S with the wrong case and -j N and were silently doing
nothing. After the patch they fail loudly, which is what they
want, but it is worth a line in the release notes.

If someone wants the exact REINDEX SCHEMA grammar (U& names,
comments), the only way is to not use the catalog query at all, and
that is a much bigger change. I do not think it is worth it.

Verdict
-------
The patch applies, builds without warnings that I saw, fixes the
reported behaviour and the tests fail without it. I would mark it
Ready for Committer, with nit 1 as a suggestion (it can be fixed
by the committer or in a v2, it does not change the code).

Regards,
Joao Rito

Em seg., 9 de nov. de 2026 às 22:29, Koshi Shibagaki (Fujitsu)
<shibagaki(dot)koshi(at)fujitsu(dot)com> escreveu:
>
> Hi,
>
> Thanks for reviewing the patch.
>
> I agree with your comment that accepting numeric OIDs and '-' is reasonable here,
> given the vacuumdb precedent. I'll keep the regnamespace approach.
>
> Regards,
>
> Koshi Shibagaki
> FUJITSU LIMITED
> https://www.fujitsu.com/
>
> ________________________________________
> 差出人: Takeshi Ideriha <iderihatakeshi(at)gmail(dot)com>
> 送信日時: 2026年10月21日 15:52
> 宛先: Shibagaki, Koshi/柴垣 向志; 'pgsql-hackers(at)lists(dot)postgresql(dot)org'
> 件名: Re: [PATCH] reindexdb: Fix schema name lookup in parallel mode
>
> Hi,
>
> On 2026/10/13 22:07, Koshi Shibagaki (Fujitsu) wrote:
> > reindexdb currently interprets --schema differently with -j 2 than
> > without it.
> >
> > For example, in a database testdb containing:
> >
> > CREATE SCHEMA testschema;
> > CREATE TABLE testschema.t (id integer PRIMARY KEY);
> >
> > these commands behave differently:
> >
> > reindexdb -d testdb -S TESTSCHEMA
> > reindexdb -d testdb -S TESTSCHEMA -j 2
> >
> > Without -j, TESTSCHEMA is folded to testschema and its index is reindexed.
> > With -j 2, the command succeeds without reindexing anything. The same
> > problem occurs with -S '"testschema"' (with double-quotes). Also, a nonexistent
> > schema causes an error in serial mode but is silently ignored in parallel mode.
> >
> > Serial mode sends REINDEX SCHEMA to the server. Parallel mode instead
> > lists tables by comparing the supplied strings directly with
> > pg_namespace.nspname.
> >
> > The attached v1 patch resolves schema names through regnamespace and
> > filters by namespace OID, as vacuumdb does. This handles case folding and
> > quoted names, and reports an error for a missing schema. Existing empty
> > schemas still succeed. TAP coverage checks case folding, quoted names,
> > and a missing schema specified alongside an empty one.
> >
> > The regnamespace input syntax also differs from REINDEX SCHEMA: it accepts numeric
> > OIDs and '-', but does not recognize SQL comments or U&-quoted identifiers. vacuumdb
> > uses regnamaspace in both serial and parallel modes. Would this approach be acceptable for
> > reindexdb, or should its parallel mode follow REINDEX SCHEMA's input syntax more closely?
>
> +1 for using regnamespace. The fix looks good to me.
>
> I believe accepting schema OID and '-' is not harmful. '-' just matches
> nothing and nothing happens. Vacuumdb and the vacuum command have
> different syntax. Vacuumdb also accepts numeric schema OIDs but the
> vacuum command does not even have a schema option. Even if there is a
> schema named "16384", we need to specify it as -S '"16384"' regardless
> of the -j option for the new reindexdb.
>
> Regards,
> Takeshi Ideriha