Horton

Thread

Commits

  1. Make Bitmapsets be valid Nodes.

  1. Making Bitmapsets be valid Nodes

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-11T20:05:58Z

    Per the discussion at [1], it seems like it'd be a good idea to make
Bitmapsets into full-fledged, tagged Nodes, so that we could do things
like print or copy lists of them without special-case logic.  The
extra space for the NodeTag is basically free due to alignment
considerations, at least on 64-bit hardware.

Attached is a cleaned-up version of Amit's patch v24-0003 at [2].
I fixed the problems with not always tagging Bitmapsets, and changed
the outfuncs/readfuncs logic so that Bitmapsets still print exactly
as they did before (thus, this doesn't require a catversion bump).

As proof of concept, I removed the read_write_ignore labels from
RelOptInfo's unique_for_rels and non_unique_for_rels fields, and
got nice-looking debug printout:

      :unique_for_rels ((b 1))
      :non_unique_for_rels <>

I also removed some special-case code from indxpath.c because
list_member() can do the same thing now.  (There might be other
places that can be simplified; I didn't look very hard.)

It'd be possible to make Bitmapset fields be (mostly) not special cases
in the copy/equal/out/read support.  But I chose to leave that alone,
because it'd add a little runtime overhead for indirecting through the
generic support functions while not really saving any code space.

Barring objections, I'd like to go ahead and push this.

			regards, tom lane

[1] https://www.postgresql.org/message-id/94353655-c177-1f55-7afb-b2090de33341%40enterprisedb.com
[2] https://www.postgresql.org/message-id/CA%2BHiwqEYCLRZ2Boq_uK0pjLn_9b8XL-LmwKj7HN5kJOivUkYLg%40mail.gmail.com

  2. Re: Making Bitmapsets be valid Nodes

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> — 2022-11-13T12:22:33Z

    On 11.11.22 21:05, Tom Lane wrote:
> Per the discussion at [1], it seems like it'd be a good idea to make
> Bitmapsets into full-fledged, tagged Nodes, so that we could do things
> like print or copy lists of them without special-case logic.  The
> extra space for the NodeTag is basically free due to alignment
> considerations, at least on 64-bit hardware.
> 
> Attached is a cleaned-up version of Amit's patch v24-0003 at [2].
> I fixed the problems with not always tagging Bitmapsets, and changed
> the outfuncs/readfuncs logic so that Bitmapsets still print exactly
> as they did before (thus, this doesn't require a catversion bump).

This looks good to me.

  3. Re: Making Bitmapsets be valid Nodes

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-11-13T15:23:28Z

    Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> On 11.11.22 21:05, Tom Lane wrote:
>> Attached is a cleaned-up version of Amit's patch v24-0003 at [2].
>> I fixed the problems with not always tagging Bitmapsets, and changed
>> the outfuncs/readfuncs logic so that Bitmapsets still print exactly
>> as they did before (thus, this doesn't require a catversion bump).

> This looks good to me.

Pushed, thanks for looking.

			regards, tom lane

  4. Re: Making Bitmapsets be valid Nodes

    Amit Langote <amitlangote09@gmail.com> — 2022-11-14T07:26:07Z

    On Mon, Nov 14, 2022 at 12:23 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> > On 11.11.22 21:05, Tom Lane wrote:
> >> Attached is a cleaned-up version of Amit's patch v24-0003 at [2].
> >> I fixed the problems with not always tagging Bitmapsets, and changed
> >> the outfuncs/readfuncs logic so that Bitmapsets still print exactly
> >> as they did before (thus, this doesn't require a catversion bump).
>
> > This looks good to me.
>
> Pushed, thanks for looking.

Thanks a lot for this.

I agree that it may not be worthwhile to add an extra function call by
changing COPY_BITMAPSET_FIELD, etc. that is currently emitted by
gen_node_support.pl for any Bitmapset * / Relid struct members to
COPY_NODE_FIELD, etc.

-- 
Thanks, Amit Langote
EDB: http://www.enterprisedb.com