Re: Do we want a hashset type?

On Thu, Jun 29, 2023, at 08:54, jian he wrote:
> Anyway, this time, I added another macro,which seems to simplify the code.
>
> #define SET_DATA_PTR(a) \
> (((char *) (a->data)) + CEIL_DIV(a->capacity, 8))
>
> it passed all the tests on my local machine.

Hmm, this is interesting. There is a bug in your second patch,
that the tests catch, so it's really surprising if they pass on your machine.

Can you try to run `make clean && make && make install && make installcheck`?

I would guess you forgot to recompile or reinstall.

This is the bug in 0002-marco-SET_DATA_PTR-to-quicly-access-hashset-data-reg.patch:

@@ -411,7 +411,7 @@ int4hashset_union(PG_FUNCTION_ARGS)
     int4hashset_t  *seta = PG_GETARG_INT4HASHSET_COPY(0);
     int4hashset_t  *setb = PG_GETARG_INT4HASHSET(1);
     char           *bitmap = setb->data;
-    int32           *values = (int32 *) (bitmap + CEIL_DIV(setb->capacity, 8));
+    int32           *values = (int32 *) SET_DATA_PTR(seta);

You accidentally replaced `setb` with `seta`.

I renamed the macro to HASHSET_GET_VALUES and changed it slightly,
also added a HASHSET_GET_BITMAP for completeness:

#define HASHSET_GET_BITMAP(set) ((set)->data)
#define HASHSET_GET_VALUES(set) ((int32 *) ((set)->data + CEIL_DIV((set)->capacity, 8)))

Instead of your version:

#define SET_DATA_PTR(a) \
        (((char *) (a->data)) + CEIL_DIV(a->capacity, 8))

Changes:
* Parenthesize macro parameters.
* Prefix the macro names with "HASHSET_" to avoid potential conflicts.
* "GET_VALUES" more clearly communicates that it's the values we're extracting.

New patch attached.

Other changes in same commit:

* Add original friends-of-friends graph query to new benchmark/ directory
* Add table of content to README
* Update docs: Explain null semantics and add function examples
* Simplify empty hashset handling, remove unused includes

/Joel