Thread: Redundant syscache access in get_rel_sync_entry()

Redundant syscache access in get_rel_sync_entry()

From
"cca5507"
Date:
Hi,
in func get_rel_sync_entry() we access the same tuple in pg_class three times:
    Oid         schemaId = get_rel_namespace(relid);
    bool am_partition = get_rel_relispartition(relid);
    char relkind = get_rel_relkind(relid);
Why not just merge into one?

--
Regards,
ChangAo Chen

Re: Redundant syscache access in get_rel_sync_entry()

From
Ashutosh Bapat
Date:


On Thu, Jul 11, 2024 at 11:38 AM cca5507 <cca5507@qq.com> wrote:
Hi,
in func get_rel_sync_entry() we access the same tuple in pg_class three times:
    Oid         schemaId = get_rel_namespace(relid);
    bool am_partition = get_rel_relispartition(relid);
    char relkind = get_rel_relkind(relid);
Why not just merge into one?

I think it's just convenient. We do that at multiple places; not exactly these functions but functions which fetch relation attributes from cached tuples. Given that the tuple is cached and local to the backend, it's not too expensive.  But if there are multiple places which do something like this, we may want to create more function get_rel_* function which return multiple properties in one function call. I see get_rel_namspace() and get_rel_name() called together at many places.

--
Best Wishes,
Ashutosh Bapat

Re: Redundant syscache access in get_rel_sync_entry()

From
"cca5507"
Date:
------------------ Original ------------------
From: "Ashutosh Bapat" <ashutosh.bapat.oss@gmail.com>;
Date: Thu, Jul 11, 2024 09:40 PM
To: "cca5507"<cca5507@qq.com>;
Cc: "pgsql-hackers"<pgsql-hackers@lists.postgresql.org>;
Subject: Re: Redundant syscache access in get_rel_sync_entry()
I think it's just convenient. We do that at multiple places; not exactly these functions but functions which fetch relation attributes from cached tuples. Given that the tuple is cached and local to the backend, it's not too expensive.  But if there are multiple places which do something like this, we may want to create more function get_rel_* function which return multiple properties in one function call. I see get_rel_namspace() and get_rel_name() called together at many places.

Agreed

Thank you for reply

--
Regards,
ChangAo Chen

Re: Redundant syscache access in get_rel_sync_entry()

From
Michael Paquier
Date:
On Thu, Jul 11, 2024 at 07:10:58PM +0530, Ashutosh Bapat wrote:
> I think it's just convenient. We do that at multiple places; not exactly
> these functions but functions which fetch relation attributes from cached
> tuples. Given that the tuple is cached and local to the backend, it's not
> too expensive.  But if there are multiple places which do something like
> this, we may want to create more function get_rel_* function which return
> multiple properties in one function call. I see get_rel_namspace() and
> get_rel_name() called together at many places.

That's not worth the complications based on the level of caching.
This code is fine as-is.
--
Michael

Attachment

Re: Redundant syscache access in get_rel_sync_entry()

From
Tom Lane
Date:
Michael Paquier <michael@paquier.xyz> writes:
> On Thu, Jul 11, 2024 at 07:10:58PM +0530, Ashutosh Bapat wrote:
>> I think it's just convenient. We do that at multiple places; not exactly
>> these functions but functions which fetch relation attributes from cached
>> tuples. Given that the tuple is cached and local to the backend, it's not
>> too expensive.  But if there are multiple places which do something like
>> this, we may want to create more function get_rel_* function which return
>> multiple properties in one function call. I see get_rel_namspace() and
>> get_rel_name() called together at many places.

> That's not worth the complications based on the level of caching.
> This code is fine as-is.

I could get behind creating such functions if there were a
demonstrable performance win, but in places that are not hot-spots
that's unlikely to be demonstrable.

            regards, tom lane