Re: The use of atooid() on non-Oid results

Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
> When looking at the report in [0] an API choice in the relevant pg_upgrade code
> path stood out as curious. check_is_install_user() runs this query to ensure
> that only the install user is present in the cluster:

> res = executeQueryOrDie(conn,
> "SELECT COUNT(*) "
> "FROM pg_catalog.pg_roles "
> "WHERE rolname !~ '^pg_'");

> The result is then verified with the following:

> if (cluster == &new_cluster && atooid(PQgetvalue(res, 0, 0)) != 1)
> pg_fatal("Only the install user can be defined in the new cluster.");

> This was changed from atoi() in ee646df59 with no specific comment on why.
> This is not a bug, since atooid() will do the right thing here, but it threw me
> off reading the code and might well confuse others. Is there a reason not to
> change this back to atoi() for code clarity as we're not reading an Oid here?

Hmm ... in principle, you could have more than 2^31 entries in pg_roles,
but not more than 2^32 since they all have to have distinct OIDs. So
I can see the point of avoiding that theoretical overflow hazard. But
personally I'd probably avoid assuming anything about how wide the COUNT()
result could be, and instead writing

... && strcmp(PQgetvalue(res, 0, 0), "1") != 0)

regards, tom lane