Alvaro Herrera <alvhe...@alvh.no-ip.org> writes: > On 2022-Oct-06, Amit Langote wrote: >> Actually, List of Bitmapsets turned out to be something that doesn't >> just-work with our Node infrastructure, which I found out thanks to >> -DWRITE_READ_PARSE_PLAN_TREES. So, I had to go ahead and add >> first-class support for copy/equal/write/read support for Bitmapsets,
> Hmm, is this related to what Tom posted as part of his 0004 in > https://postgr.es/m/2901865.1667685...@sss.pgh.pa.us It could be. For some reason I thought that Amit had withdrawn his proposal to make Bitmapsets be Nodes. But if it's still live, then the data structure I invented in my 0004 could plausibly be replaced by a List of Bitmapsets. The code I was using that for would rather have fixed-size arrays of Bitmapsets than variable-size Lists, mainly because it always knows ab initio what the max length of the array will be. But I don't think that the preference is so strong that it justifies a private data structure. The main thing I was wondering about in connection with that was whether to assume that there could be other future applications of the logic to perform multi-bitmapset union, intersection, etc. If so, then I'd be inclined to choose different naming and put those functions in or near to bitmapset.c. It doesn't look like Amit's code needs anything like that, but maybe somebody has an idea about other applications? Anyway, I concur with Peter's upthread comment that making Bitmapsets be Nodes is probably justifiable all by itself. The lack of a Node tag in them now is just because in a 32-bit world it seemed like unnecessary bloat. But on 64-bit machines it's free, and we aren't optimizing for 32-bit any more. I do not like the details of v24-0003 at all though, because it seems to envision that a "node Bitmapset" is a different thing from a raw Bitmapset. That can only lead to bugs --- why would we not make it the case that every Bitmapset is properly labeled with the node tag? Also, although I'm on board with making Bitmapsets be Nodes, I don't think I'm on board with changing their dump format. Planner node dumps would get enormously bulkier and less readable if we changed things like :relids (b 1 2) to :relids {BITMAPSET (b 1 2) } or whatever the output would look like as the patch stands. So that needs a bit more effort, but it's surely manageable. regards, tom lane
