On Mon, May 19, 2025 at 04:17:26PM -0500, Nathan Bossart wrote:
> On Mon, May 19, 2025 at 10:37:29AM +0800, jian he wrote:
>> I saw REGROLEOID in foreign_expr_walker,
>> I'm wondering whether REGDATABASEOID is reachable within foreign_expr_walker.
>>
>> not familiar with this area, also there are no coverage tests for
>> other reg*Const,
>> per
>> https://coverage.postgresql.org/contrib/postgres_fdw/deparse.c.gcov.html
>
> Oh, good catch. At first glance, it looks like we could just add another
> case block for REGDATABASEOID, but looking closer at is_shippable(), I see
> that it's just testing for objectId < FirstGenbkiObjectId. That seems like
> it might be a problem because we don't consider any databases to be pinned,
> and template0, template1, and postgres start with OIDs of 4, 1, and 5,
> respectively. However, I'd expect that to also be a problem for the public
> schema, but foreign_expr_walker() doesn't appear to have any special
> handling for that. Hm...
For now, I've just added another case block for REGDATABASEOID to match the
others. If there are problems with non-pinned objects being considered
shippable, it's not really the fault of this patch. Also, from reading
around [0], I get the idea that "shippability" might just mean that the
same object _probably_ exists on the remote server. Plus, there seems to
be very few use-cases for shipping reg* values in the first place. But
even after reading lots of threads, code, and docs, I'm still not sure I
fully grasp all the details here.
[0] https://postgr.es/m/flat/1423433.1652722406%40sss.pgh.pa.us
--
nathan
