Make Bitmapsets be valid Nodes.

Add a NodeTag field to struct Bitmapset.  This is free because of
alignment considerations on 64-bit hardware.  While it adds some
space on 32-bit machines, we aren't optimizing for that case anymore.
The advantage is that data structures such as Lists of Bitmapsets
are now first-class objects to the Node infrastructure, and don't
require special-case code to handle.

This patch includes removal of one such special case, in indxpath.c:
bms_equal_any() can now be replaced by list_member().  There may be
more existing code that could be simplified, but I didn't look very
hard.  We also get to drop the read_write_ignore annotations on a
couple of RelOptInfo fields.

The outfuncs/readfuncs support is arranged so that nothing changes
in the string representation of a Bitmapset field; therefore, this
doesn't need a catversion bump.

Amit Langote and Tom Lane

Discussion: https://postgr.es/m/109089.1668197158@sss.pgh.pa.us
This commit is contained in:
Tom Lane 2022-11-13 10:22:45 -05:00
parent 9c7eb9d85a
commit 5e1f3b9ebf
12 changed files with 68 additions and 30 deletions

View File

@ -52,6 +52,7 @@ node_headers = \
commands/trigger.h \
executor/tuptable.h \
foreign/fdwapi.h \
nodes/bitmapset.h \
nodes/extensible.h \
nodes/lockoptions.h \
nodes/replnodes.h \

View File

@ -194,6 +194,7 @@ bms_make_singleton(int x)
wordnum = WORDNUM(x);
bitnum = BITNUM(x);
result = (Bitmapset *) palloc0(BITMAPSET_SIZE(wordnum + 1));
result->type = T_Bitmapset;
result->nwords = wordnum + 1;
result->words[wordnum] = ((bitmapword) 1 << bitnum);
return result;
@ -852,6 +853,7 @@ bms_add_range(Bitmapset *a, int lower, int upper)
if (a == NULL)
{
a = (Bitmapset *) palloc0(BITMAPSET_SIZE(uwordnum + 1));
a->type = T_Bitmapset;
a->nwords = uwordnum + 1;
}
else if (uwordnum >= a->nwords)

View File

@ -160,6 +160,12 @@ _copyExtensibleNode(const ExtensibleNode *from)
return newnode;
}
static Bitmapset *
_copyBitmapset(const Bitmapset *from)
{
return bms_copy(from);
}
/*
* copyObjectImpl -- implementation of copyObject(); see nodes/nodes.h

View File

@ -145,6 +145,12 @@ _equalA_Const(const A_Const *a, const A_Const *b)
return true;
}
static bool
_equalBitmapset(const Bitmapset *a, const Bitmapset *b)
{
return bms_equal(a, b);
}
/*
* Lists are handled specially
*/

View File

@ -65,6 +65,7 @@ my @all_input_files = qw(
commands/trigger.h
executor/tuptable.h
foreign/fdwapi.h
nodes/bitmapset.h
nodes/extensible.h
nodes/lockoptions.h
nodes/replnodes.h

View File

@ -314,6 +314,9 @@ _outList(StringInfo str, const List *node)
* converts a bitmap set of integers
*
* Note: the output format is "(b int int ...)", similar to an integer List.
*
* We export this function for use by extensions that define extensible nodes.
* That's somewhat historical, though, because calling outNode() will work.
*/
void
outBitmapset(StringInfo str, const Bitmapset *bms)
@ -844,6 +847,8 @@ outNode(StringInfo str, const void *obj)
_outString(str, (String *) obj);
else if (IsA(obj, BitString))
_outBitString(str, (BitString *) obj);
else if (IsA(obj, Bitmapset))
outBitmapset(str, (Bitmapset *) obj);
else
{
appendStringInfoChar(str, '{');

View File

@ -22,6 +22,7 @@
#include <ctype.h>
#include "common/string.h"
#include "nodes/bitmapset.h"
#include "nodes/pg_list.h"
#include "nodes/readfuncs.h"
#include "nodes/value.h"
@ -347,6 +348,7 @@ nodeRead(const char *token, int tok_len)
* Could be an integer list: (i int int ...)
* or an OID list: (o int int ...)
* or an XID list: (x int int ...)
* or a bitmapset: (b int int ...)
* or a list of nodes/values: (node node ...)
*----------
*/
@ -372,6 +374,7 @@ nodeRead(const char *token, int tok_len)
tok_len, token);
l = lappend_int(l, val);
}
result = (Node *) l;
}
else if (tok_len == 1 && token[0] == 'o')
{
@ -392,6 +395,7 @@ nodeRead(const char *token, int tok_len)
tok_len, token);
l = lappend_oid(l, val);
}
result = (Node *) l;
}
else if (tok_len == 1 && token[0] == 'x')
{
@ -412,6 +416,30 @@ nodeRead(const char *token, int tok_len)
tok_len, token);
l = lappend_xid(l, val);
}
result = (Node *) l;
}
else if (tok_len == 1 && token[0] == 'b')
{
/* Bitmapset -- see also _readBitmapset() */
Bitmapset *bms = NULL;
for (;;)
{
int val;
char *endptr;
token = pg_strtok(&tok_len);
if (token == NULL)
elog(ERROR, "unterminated Bitmapset structure");
if (tok_len == 1 && token[0] == ')')
break;
val = (int) strtol(token, &endptr, 10);
if (endptr != token + tok_len)
elog(ERROR, "unrecognized integer: \"%.*s\"",
tok_len, token);
bms = bms_add_member(bms, val);
}
result = (Node *) bms;
}
else
{
@ -426,8 +454,8 @@ nodeRead(const char *token, int tok_len)
if (token == NULL)
elog(ERROR, "unterminated List structure");
}
result = (Node *) l;
}
result = (Node *) l;
break;
}
case RIGHT_PAREN:

View File

@ -194,6 +194,10 @@ nullable_string(const char *token, int length)
/*
* _readBitmapset
*
* Note: this code is used in contexts where we know that a Bitmapset
* is expected. There is equivalent code in nodeRead() that can read a
* Bitmapset when we come across one in other contexts.
*/
static Bitmapset *
_readBitmapset(void)
@ -234,7 +238,8 @@ _readBitmapset(void)
}
/*
* for use by extensions which define extensible nodes
* We export this function for use by extensions that define extensible nodes.
* That's somewhat historical, though, because calling nodeRead() will work.
*/
Bitmapset *
readBitmapset(void)

View File

@ -99,7 +99,6 @@ static void get_join_index_paths(PlannerInfo *root, RelOptInfo *rel,
List **considered_relids);
static bool eclass_already_used(EquivalenceClass *parent_ec, Relids oldrelids,
List *indexjoinclauses);
static bool bms_equal_any(Relids relids, List *relids_list);
static void get_index_paths(PlannerInfo *root, RelOptInfo *rel,
IndexOptInfo *index, IndexClauseSet *clauses,
List **bitindexpaths);
@ -370,8 +369,8 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
Path *path = (Path *) lfirst(lc);
Relids required_outer = PATH_REQ_OUTER(path);
if (!bms_equal_any(required_outer, all_path_outers))
all_path_outers = lappend(all_path_outers, required_outer);
all_path_outers = list_append_unique(all_path_outers,
required_outer);
}
/* Now, for each distinct parameterization set ... */
@ -517,7 +516,7 @@ consider_index_join_outer_rels(PlannerInfo *root, RelOptInfo *rel,
int num_considered_relids;
/* If we already tried its relids set, no need to do so again */
if (bms_equal_any(clause_relids, *considered_relids))
if (list_member(*considered_relids, clause_relids))
continue;
/*
@ -612,7 +611,7 @@ get_join_index_paths(PlannerInfo *root, RelOptInfo *rel,
int indexcol;
/* If we already considered this relids set, don't repeat the work */
if (bms_equal_any(relids, *considered_relids))
if (list_member(*considered_relids, relids))
return;
/* Identify indexclauses usable with this relids set */
@ -694,25 +693,6 @@ eclass_already_used(EquivalenceClass *parent_ec, Relids oldrelids,
return false;
}
/*
* bms_equal_any
* True if relids is bms_equal to any member of relids_list
*
* Perhaps this should be in bitmapset.c someday.
*/
static bool
bms_equal_any(Relids relids, List *relids_list)
{
ListCell *lc;
foreach(lc, relids_list)
{
if (bms_equal(relids, (Relids) lfirst(lc)))
return true;
}
return false;
}
/*
* get_index_paths

View File

@ -20,6 +20,8 @@
#ifndef BITMAPSET_H
#define BITMAPSET_H
#include "nodes/nodes.h"
/*
* Forward decl to save including pg_list.h
*/
@ -48,6 +50,9 @@ typedef int32 signedbitmapword; /* must be the matching signed type */
typedef struct Bitmapset
{
pg_node_attr(custom_copy_equal, special_read_write)
NodeTag type;
int nwords; /* number of words in array */
bitmapword words[FLEXIBLE_ARRAY_MEMBER]; /* really [nwords] */
} Bitmapset;

View File

@ -13,6 +13,7 @@ node_support_input_i = [
'commands/trigger.h',
'executor/tuptable.h',
'foreign/fdwapi.h',
'nodes/bitmapset.h',
'nodes/extensible.h',
'nodes/lockoptions.h',
'nodes/replnodes.h',

View File

@ -911,13 +911,11 @@ typedef struct RelOptInfo
/*
* cache space for remembering if we have proven this relation unique
*
* can't print unique_for_rels/non_unique_for_rels; BMSes aren't Nodes
*/
/* known unique for these other relid set(s) */
List *unique_for_rels pg_node_attr(read_write_ignore);
List *unique_for_rels;
/* known not unique for these set(s) */
List *non_unique_for_rels pg_node_attr(read_write_ignore);
List *non_unique_for_rels;
/*
* used by various scans and joins: