Re: [PATCH] Expose port->authn_id to extensions and triggers

From: Jacob Champion <jchampion(at)timescale(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Michael Paquier <michael(at)paquier(dot)xyz>, "sfrost(at)snowman(dot)net" <sfrost(at)snowman(dot)net>, "rjuju123(at)gmail(dot)com" <rjuju123(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>, "andres(at)anarazel(dot)de" <andres(at)anarazel(dot)de>
Subject: Re: [PATCH] Expose port->authn_id to extensions and triggers
Date: 2022-06-08 23:53:08
Message-ID: CAAWbhmhfzt+hYiTXi3f0V_E5E+E_mmzd_x+p9zjs_NvBpJ36pA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jun 7, 2022 at 5:45 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> Perhaps that is
> ill-founded, but I don't think "should be serialized" is necessarily
> something that everybody is going to have the same view on, or even
> know what it means.

Using this thread as an example, once it was decided that the parallel
workers needed the additional info, the need for serialization
followed directly from that. I don't get the feeling that developers
are going to jump through the hoops of writing serialization logic for
a new field in the struct just by accident; they should know why
they're writing that code, and hopefully it would be easy for
reviewers to catch a patch that showed up with pointless
serialization.

> Also, I don't think we want to end up with a situation where we have a
> struct that contains wildly unrelated things that all need to be
> serialized. If the definition of the struct is "stuff we should
> serialize and send to the worker," well then maybe the transaction
> snapshot ought to go in there! Well, no. I mean, we already have a
> separate place for that, but suppose somehow we didn't. It doesn't
> belong here, because yes the things in this struct get serialized, but
> it's not only any old thing that needs serializing, it's more specific
> than that.

I completely agree with you here -- the name should not be so generic
that it's just a catch-all for any serialized fields that exist.

> I guess what this boils down to is that I really want this thing to
> have a meaningful name by means of which a future developer can make a
> guess as to whether some field they're adding ought to go in there. I
> theorize that SharedPort is not too great because (a) Port is already
> a bad name and (b) how am I supposed to know whether my stuff ought to
> be shared or not? I like something like ClientConnectionInfo better
> because it seems to describe what the stuff in the struct *is* rather
> than what we *do* with it.

I think having both would be useful in this case -- what the stuff is,
so that it's clear what doesn't belong in it, and what we do with it,
so it's clear that you have to write serialization code if you add new
things. The nature of the struct is such that I think you _have_ to
figure out whether or not your stuff ought to be shared before you
have any business adding it.

But I don't have any better ideas for how to achieve both. I'm fine
with your suggestion of ClientConnectionInfo, if that sounds good to
others; the doc comment can clarify why it differs from Port? Or add
one of the Shared-/Gang-/Group- prefixes to it, maybe?

Thanks,
--Jacob

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-06-09 00:00:31 Re: PGDOCS - "System Catalogs" table-of-contents page structure
Previous Message Peter Geoghegan 2022-06-08 23:43:48 Re: BTMaxItemSize seems to be subtly incorrect