From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: The use of atooid() on non-Oid results |
Date: | 2023-03-16 19:17:15 |
Message-ID: | 639E50A3-E445-44D2-AFD2-8B294DB5554D@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On 16 Mar 2023, at 15:58, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> 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)
Yeah, that makes sense. I'll go ahead with that solution instead and possibly
a brief addition to the comment.
--
Daniel Gustafsson
From | Date | Subject | |
---|---|---|---|
Next Message | Greg Stark | 2023-03-16 20:00:28 | Re: Experiments with Postgres and SSL |
Previous Message | Jim Jones | 2023-03-16 19:07:06 | Re: [PATCH] Add pretty-printed XML output option |