On Wed, Oct 21, 2020 at 11:23 PM Thomas Munro <thomas.mu...@gmail.com> wrote:
>
> On Wed, Jun 17, 2020 at 10:32 PM Magnus Hagander <mag...@hagander.net> wrote:
> > In looking at this I realize we also have exactly one thing referred to as 
> > "blacklist" in our codebase, which is the "enum blacklist" (and then a 
> > small internal variable in pgindent). AFAICT, it's not actually exposed to 
> > userspace anywhere, so we could probably make the attached change to 
> > blocklist at no "cost" (the only thing changed is the name of the hash 
> > table, and we definitely change things like that in normal releases with no 
> > specific thought on backwards compat).
>
> +1
>
> Hmm, can we find a more descriptive name for this mechanism?  What
> about calling it the "uncommitted enum table"?  See attached.

Thanks for picking this one up again!

Agreed, that's a much better choice.

The term itself is a bit of a mouthful, but it does describe what it
is in a much more clear way than what the old term did anyway.

Maybe consider just calling it "uncomitted enums", which would as a
bonus have it not end up talking about uncommittedenumtablespace which
gets hits on searches for tablespace.. Though I'm not sure that's
important.

I'm +1 to the change with or without that adjustment.

As for the code, I note that:
+       /* Set up the enum table if not already done in this transaction */

forgets to say it's *uncomitted* enum table -- which is the important
part of I believe.

And
+ * Test if the given enum value is in the table of blocked enums.

should probably talk about uncommitted rather than blocked?

-- 
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/


Reply via email to