On Thu, Jun 29, 2023 at 4:43 PM Joel Jacobson <j...@compiler.org> wrote: > > 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
more like a C questions in this context does #define HASHSET_GET_VALUES(set) ((int32 *) ((set)->data + CEIL_DIV((set)->capacity, 8))) define first, then define struct int4hashset_t. Is this normally ok? Also does #define HASHSET_GET_VALUES(set) ((int32 *) ((set)->data + CEIL_DIV((set)->capacity, 8))) remove (int32 *) will make it generic? then when you use it, you can cast whatever type you like?
