Redefining a typedef is a C11 feature:
In file included from src/hnsw.c:10:
src/hnsw.h:147:5: warning: redefinition of typedef 'HnswElementData' is a C11 feature [-Wtypedef-redefinition]
} HnswElementData;
^
src/hnsw.h:118:32: note: previous definition is here
typedef struct HnswElementData HnswElementData;
^
src/hnsw.h:163:5: warning: redefinition of typedef 'HnswNeighborArray' is a C11 feature [-Wtypedef-redefinition]
} HnswNeighborArray;
^
src/hnsw.h:119:34: note: previous definition is here
typedef struct HnswNeighborArray HnswNeighborArray;
^
2 warnings generated.
I got these warnings when I built PostgreSQL with "CC=clang
CFLAGS=-std=gnu99"; other similar options would surely produce the
warnings too.
The original motivation was to eliminate the superfluous HnswPtrAccess
call from AddToVisited. The caller has to call HnswPtrAccess() anyway,
so it makes sense to pass the HnswElement rather than HnswElementPtr
to AddToVisited(). But then I realized that we can use the
pointer-variant even with shared memory, because the visited-hash is
backend-private, and the addresses where the elements are mapped to in
shared memory are stable within the backend.
These places called PageIndexTupleOverwrite(), with the new tuple
pointing directly to the original page. The PageIndexTupleOverwrite()
call is unnecessary in these cases, as we have already modified the
tuple on the page directly. Moreover, PageIndexTupleOverWrite() will
call memcpy with same src and dst arguments, which is undefined
behavior.
It's OK to modify the pages on disk directly, and we don't need
critical sections, because we either use the generic xlog functions
which create a temporary copy of the page, or we are building a new
index so if we crash the whole index is invisible and will be dropped
anyway.
ivfflat.h and hnsw.h have references to pairingheap_node, so they need
to include lib/pairingheap.h. It happened to work, because
lib/pairingheap.h was being included indirectly through
nodes/execnodes.h, but let's be explicit.
Remove the include from hnswbuild.c, because there are no calls to
pairingheap functions in that file. Instead, add the includes to
hnswutils.c and ivfscan.c, which do have such calls. They are not
strictly necessary again because of the indirect include from hnsw.h
and ivfflat.h, but let's be explicit while we're messing with this.
It was just used to pass heap/index relations to
HnswParallelScanAndInsert. I think it was copied from nbtsort.c, which
is more complicated. I don't think we need a struct like this.
(That said, I actually think that we should have a state object that
would hold fields like 'heap', 'index', 'procinfo', 'collation'
etc. Passing that object around would simplify the signatures of many
functions. But that's a different story).
