public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 3/8] Move processing of NULLs from BRIN support functions 3+ messages / 3 participants [nested] [flat]
* [PATCH 3/8] Move processing of NULLs from BRIN support functions @ 2020-04-02 00:56 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Tomas Vondra @ 2020-04-02 00:56 UTC (permalink / raw) --- src/backend/access/brin/brin.c | 260 ++++++++++++++--------- src/backend/access/brin/brin_inclusion.c | 44 +--- src/backend/access/brin/brin_minmax.c | 41 +--- src/include/access/brin_internal.h | 3 + 4 files changed, 169 insertions(+), 179 deletions(-) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index caf7b62688..a9c44c0b82 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -35,6 +35,7 @@ #include "storage/freespace.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/datum.h" #include "utils/index_selfuncs.h" #include "utils/memutils.h" #include "utils/rel.h" @@ -77,7 +78,9 @@ static void form_and_insert_tuple(BrinBuildState *state); static void union_tuples(BrinDesc *bdesc, BrinMemTuple *a, BrinTuple *b); static void brin_vacuum_scan(Relation idxrel, BufferAccessStrategy strategy); - +static bool add_values_to_range(Relation idxRel, BrinDesc *bdesc, + BrinMemTuple *dtup, Datum *values, bool *nulls); +static bool check_null_keys(BrinValues *bval, ScanKey *nullkeys, int nnullkeys); /* * BRIN handler function: return IndexAmRoutine with access method parameters @@ -178,7 +181,6 @@ brininsert(Relation idxRel, Datum *values, bool *nulls, OffsetNumber off; BrinTuple *brtup; BrinMemTuple *dtup; - int keyno; CHECK_FOR_INTERRUPTS(); @@ -242,31 +244,7 @@ brininsert(Relation idxRel, Datum *values, bool *nulls, dtup = brin_deform_tuple(bdesc, brtup, NULL); - /* - * Compare the key values of the new tuple to the stored index values; - * our deformed tuple will get updated if the new tuple doesn't fit - * the original range (note this means we can't break out of the loop - * early). Make a note of whether this happens, so that we know to - * insert the modified tuple later. - */ - for (keyno = 0; keyno < bdesc->bd_tupdesc->natts; keyno++) - { - Datum result; - BrinValues *bval; - FmgrInfo *addValue; - - bval = &dtup->bt_columns[keyno]; - addValue = index_getprocinfo(idxRel, keyno + 1, - BRIN_PROCNUM_ADDVALUE); - result = FunctionCall4Coll(addValue, - idxRel->rd_indcollation[keyno], - PointerGetDatum(bdesc), - PointerGetDatum(bval), - values[keyno], - nulls[keyno]); - /* if that returned true, we need to insert the updated tuple */ - need_insert |= DatumGetBool(result); - } + need_insert = add_values_to_range(idxRel, bdesc, dtup, values, nulls); if (!need_insert) { @@ -359,6 +337,7 @@ brinbeginscan(Relation r, int nkeys, int norderbys) return scan; } + /* * Execute the index scan. * @@ -562,69 +541,31 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) (nkeys[attno - 1] <= scan->numberOfKeys)); /* - * First check if there are any IS [NOT] NULL scan keys, and - * if we're violating them. In that case we can terminate - * early, without invoking the support function. + * First check if there are any IS [NOT] NULL scan keys, + * and if we're violating them. In that case we can + * terminate early, without invoking the support function. * * As there may be more keys, we can only detemine mismatch * within this loop. */ - for (keyno = 0; (keyno < nnullkeys[attno - 1]); keyno++) + if (bdesc->bd_info[attno - 1]->oi_regular_nulls && + !check_null_keys(bval, nullkeys[attno - 1], + nnullkeys[attno - 1])) { - ScanKey key = nullkeys[attno - 1][keyno]; - - Assert(key->sk_attno == bval->bv_attno); - - /* interrupt the loop as soon as we find a mismatch */ - if (!addrange) - break; - - /* handle IS NULL/IS NOT NULL tests */ - if (key->sk_flags & SK_ISNULL) - { - /* IS NULL scan key, but range has no NULLs */ - if (key->sk_flags & SK_SEARCHNULL) - { - if (!bval->bv_allnulls && !bval->bv_hasnulls) - addrange = false; - - continue; - } - - /* - * For IS NOT NULL, we can only skip ranges that are - * known to have only nulls. - */ - if (key->sk_flags & SK_SEARCHNOTNULL) - { - if (bval->bv_allnulls) - addrange = false; - - continue; - } - - /* - * Neither IS NULL nor IS NOT NULL was used; assume all - * indexable operators are strict and thus return false - * with NULL value in the scan key. - */ - addrange = false; - } - } - - /* - * If any of the IS [NOT] NULL keys failed, the page range as - * a whole can't pass. So terminate the loop. - */ - if (!addrange) + /* + * If any of the IS [NOT] NULL keys failed, the page + * range as a whole can't pass. So terminate the loop. + */ + addrange = false; break; + } /* - * So either there are no IS [NOT] NULL keys, or all passed. If - * there are no regular scan keys, we're done - the page range - * matches. If there are regular keys, but the page range is - * marked as 'all nulls' it can't possibly pass (we're assuming - * the operators are strict). + * So either there are no IS [NOT] NULL keys, or all passed. + * If there are no regular scan keys, we're done - the page + * range matches. If there are regular keys, but the page + * range is marked as 'all nulls' it can't possibly pass + * (we're assuming the operators are strict). */ /* No regular scan keys - page range as a whole passes. */ @@ -772,7 +713,6 @@ brinbuildCallback(Relation index, { BrinBuildState *state = (BrinBuildState *) brstate; BlockNumber thisblock; - int i; thisblock = ItemPointerGetBlockNumber(tid); @@ -801,25 +741,8 @@ brinbuildCallback(Relation index, } /* Accumulate the current tuple into the running state */ - for (i = 0; i < state->bs_bdesc->bd_tupdesc->natts; i++) - { - FmgrInfo *addValue; - BrinValues *col; - Form_pg_attribute attr = TupleDescAttr(state->bs_bdesc->bd_tupdesc, i); - - col = &state->bs_dtuple->bt_columns[i]; - addValue = index_getprocinfo(index, i + 1, - BRIN_PROCNUM_ADDVALUE); - - /* - * Update dtuple state, if and as necessary. - */ - FunctionCall4Coll(addValue, - attr->attcollation, - PointerGetDatum(state->bs_bdesc), - PointerGetDatum(col), - values[i], isnull[i]); - } + (void) add_values_to_range(index, state->bs_bdesc, state->bs_dtuple, + values, isnull); } /* @@ -1598,6 +1521,39 @@ union_tuples(BrinDesc *bdesc, BrinMemTuple *a, BrinTuple *b) FmgrInfo *unionFn; BrinValues *col_a = &a->bt_columns[keyno]; BrinValues *col_b = &db->bt_columns[keyno]; + BrinOpcInfo *opcinfo = bdesc->bd_info[keyno]; + + if (opcinfo->oi_regular_nulls) + { + /* Adjust "hasnulls". */ + if (!col_a->bv_hasnulls && col_b->bv_hasnulls) + col_a->bv_hasnulls = true; + + /* If there are no values in B, there's nothing left to do. */ + if (col_b->bv_allnulls) + continue; + + /* + * Adjust "allnulls". If A doesn't have values, just copy the + * values from B into A, and we're done. We cannot run the + * operators in this case, because values in A might contain + * garbage. Note we already established that B contains values. + */ + if (col_a->bv_allnulls) + { + int i; + + col_a->bv_allnulls = false; + + for (i = 0; i < opcinfo->oi_nstored; i++) + col_a->bv_values[i] = + datumCopy(col_b->bv_values[i], + opcinfo->oi_typcache[i]->typbyval, + opcinfo->oi_typcache[i]->typlen); + + continue; + } + } unionFn = index_getprocinfo(bdesc->bd_index, keyno + 1, BRIN_PROCNUM_UNION); @@ -1651,3 +1607,103 @@ brin_vacuum_scan(Relation idxrel, BufferAccessStrategy strategy) */ FreeSpaceMapVacuum(idxrel); } + +static bool +add_values_to_range(Relation idxRel, BrinDesc *bdesc, BrinMemTuple *dtup, + Datum *values, bool *nulls) +{ + int keyno; + bool modified = false; + + /* + * Compare the key values of the new tuple to the stored index values; + * our deformed tuple will get updated if the new tuple doesn't fit + * the original range (note this means we can't break out of the loop + * early). Make a note of whether this happens, so that we know to + * insert the modified tuple later. + */ + for (keyno = 0; keyno < bdesc->bd_tupdesc->natts; keyno++) + { + Datum result; + BrinValues *bval; + FmgrInfo *addValue; + + bval = &dtup->bt_columns[keyno]; + + if (bdesc->bd_info[keyno]->oi_regular_nulls && nulls[keyno]) + { + /* + * If the new value is null, we record that we saw it if it's + * the first one; otherwise, there's nothing to do. + */ + if (!bval->bv_hasnulls) + { + bval->bv_hasnulls = true; + modified = true; + } + + continue; + } + + addValue = index_getprocinfo(idxRel, keyno + 1, + BRIN_PROCNUM_ADDVALUE); + result = FunctionCall4Coll(addValue, + idxRel->rd_indcollation[keyno], + PointerGetDatum(bdesc), + PointerGetDatum(bval), + values[keyno], + nulls[keyno]); + /* if that returned true, we need to insert the updated tuple */ + modified |= DatumGetBool(result); + } + + return modified; +} + +static bool +check_null_keys(BrinValues *bval, ScanKey *nullkeys, int nnullkeys) +{ + int keyno; + + /* + * First check if there are any IS [NOT] NULL scan keys, and if we're + * violating them. + */ + for (keyno = 0; keyno < nnullkeys; keyno++) + { + ScanKey key = nullkeys[keyno]; + + Assert(key->sk_attno == bval->bv_attno); + + /* Handle only IS NULL/IS NOT NULL tests */ + if (!(key->sk_flags & SK_ISNULL)) + continue; + + if (key->sk_flags & SK_SEARCHNULL) + { + /* IS NULL scan key, but range has no NULLs */ + if (!bval->bv_allnulls && !bval->bv_hasnulls) + return false; + } + else if (key->sk_flags & SK_SEARCHNOTNULL) + { + /* + * For IS NOT NULL, we can only skip ranges that are known to + * have only nulls. + */ + if (bval->bv_allnulls) + return false; + } + else + { + /* + * Neither IS NULL nor IS NOT NULL was used; assume all indexable + * operators are strict and thus return false with NULL value in + * the scan key. + */ + return false; + } + } + + return true; +} diff --git a/src/backend/access/brin/brin_inclusion.c b/src/backend/access/brin/brin_inclusion.c index 22edc6b46f..59503b6f68 100644 --- a/src/backend/access/brin/brin_inclusion.c +++ b/src/backend/access/brin/brin_inclusion.c @@ -110,6 +110,7 @@ brin_inclusion_opcinfo(PG_FUNCTION_ARGS) */ result = palloc0(MAXALIGN(SizeofBrinOpcInfo(3)) + sizeof(InclusionOpaque)); result->oi_nstored = 3; + result->oi_regular_nulls = true; result->oi_opaque = (InclusionOpaque *) MAXALIGN((char *) result + SizeofBrinOpcInfo(3)); @@ -141,7 +142,7 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS) BrinDesc *bdesc = (BrinDesc *) PG_GETARG_POINTER(0); BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1); Datum newval = PG_GETARG_DATUM(2); - bool isnull = PG_GETARG_BOOL(3); + bool isnull PG_USED_FOR_ASSERTS_ONLY = PG_GETARG_BOOL(3); Oid colloid = PG_GET_COLLATION(); FmgrInfo *finfo; Datum result; @@ -149,18 +150,7 @@ brin_inclusion_add_value(PG_FUNCTION_ARGS) AttrNumber attno; Form_pg_attribute attr; - /* - * If the new value is null, we record that we saw it if it's the first - * one; otherwise, there's nothing to do. - */ - if (isnull) - { - if (column->bv_hasnulls) - PG_RETURN_BOOL(false); - - column->bv_hasnulls = true; - PG_RETURN_BOOL(true); - } + Assert(!isnull); attno = column->bv_attno; attr = TupleDescAttr(bdesc->bd_tupdesc, attno - 1); @@ -510,37 +500,11 @@ brin_inclusion_union(PG_FUNCTION_ARGS) Datum result; Assert(col_a->bv_attno == col_b->bv_attno); - - /* Adjust "hasnulls". */ - if (!col_a->bv_hasnulls && col_b->bv_hasnulls) - col_a->bv_hasnulls = true; - - /* If there are no values in B, there's nothing left to do. */ - if (col_b->bv_allnulls) - PG_RETURN_VOID(); + Assert(!col_a->bv_allnulls && !col_b->bv_allnulls); attno = col_a->bv_attno; attr = TupleDescAttr(bdesc->bd_tupdesc, attno - 1); - /* - * Adjust "allnulls". If A doesn't have values, just copy the values from - * B into A, and we're done. We cannot run the operators in this case, - * because values in A might contain garbage. Note we already established - * that B contains values. - */ - if (col_a->bv_allnulls) - { - col_a->bv_allnulls = false; - col_a->bv_values[INCLUSION_UNION] = - datumCopy(col_b->bv_values[INCLUSION_UNION], - attr->attbyval, attr->attlen); - col_a->bv_values[INCLUSION_UNMERGEABLE] = - col_b->bv_values[INCLUSION_UNMERGEABLE]; - col_a->bv_values[INCLUSION_CONTAINS_EMPTY] = - col_b->bv_values[INCLUSION_CONTAINS_EMPTY]; - PG_RETURN_VOID(); - } - /* If B includes empty elements, mark A similarly, if needed. */ if (!DatumGetBool(col_a->bv_values[INCLUSION_CONTAINS_EMPTY]) && DatumGetBool(col_b->bv_values[INCLUSION_CONTAINS_EMPTY])) diff --git a/src/backend/access/brin/brin_minmax.c b/src/backend/access/brin/brin_minmax.c index 7a7bd21cec..8882eec12c 100644 --- a/src/backend/access/brin/brin_minmax.c +++ b/src/backend/access/brin/brin_minmax.c @@ -48,6 +48,7 @@ brin_minmax_opcinfo(PG_FUNCTION_ARGS) result = palloc0(MAXALIGN(SizeofBrinOpcInfo(2)) + sizeof(MinmaxOpaque)); result->oi_nstored = 2; + result->oi_regular_nulls = true; result->oi_opaque = (MinmaxOpaque *) MAXALIGN((char *) result + SizeofBrinOpcInfo(2)); result->oi_typcache[0] = result->oi_typcache[1] = @@ -69,7 +70,7 @@ brin_minmax_add_value(PG_FUNCTION_ARGS) BrinDesc *bdesc = (BrinDesc *) PG_GETARG_POINTER(0); BrinValues *column = (BrinValues *) PG_GETARG_POINTER(1); Datum newval = PG_GETARG_DATUM(2); - bool isnull = PG_GETARG_DATUM(3); + bool isnull PG_USED_FOR_ASSERTS_ONLY = PG_GETARG_DATUM(3); Oid colloid = PG_GET_COLLATION(); FmgrInfo *cmpFn; Datum compar; @@ -77,18 +78,7 @@ brin_minmax_add_value(PG_FUNCTION_ARGS) Form_pg_attribute attr; AttrNumber attno; - /* - * If the new value is null, we record that we saw it if it's the first - * one; otherwise, there's nothing to do. - */ - if (isnull) - { - if (column->bv_hasnulls) - PG_RETURN_BOOL(false); - - column->bv_hasnulls = true; - PG_RETURN_BOOL(true); - } + Assert(!isnull); attno = column->bv_attno; attr = TupleDescAttr(bdesc->bd_tupdesc, attno - 1); @@ -245,34 +235,11 @@ brin_minmax_union(PG_FUNCTION_ARGS) bool needsadj; Assert(col_a->bv_attno == col_b->bv_attno); - - /* Adjust "hasnulls" */ - if (!col_a->bv_hasnulls && col_b->bv_hasnulls) - col_a->bv_hasnulls = true; - - /* If there are no values in B, there's nothing left to do */ - if (col_b->bv_allnulls) - PG_RETURN_VOID(); + Assert(!col_a->bv_allnulls && !col_b->bv_allnulls); attno = col_a->bv_attno; attr = TupleDescAttr(bdesc->bd_tupdesc, attno - 1); - /* - * Adjust "allnulls". If A doesn't have values, just copy the values from - * B into A, and we're done. We cannot run the operators in this case, - * because values in A might contain garbage. Note we already established - * that B contains values. - */ - if (col_a->bv_allnulls) - { - col_a->bv_allnulls = false; - col_a->bv_values[0] = datumCopy(col_b->bv_values[0], - attr->attbyval, attr->attlen); - col_a->bv_values[1] = datumCopy(col_b->bv_values[1], - attr->attbyval, attr->attlen); - PG_RETURN_VOID(); - } - /* Adjust minimum, if B's min is less than A's min */ finfo = minmax_get_strategy_procinfo(bdesc, attno, attr->atttypid, BTLessStrategyNumber); diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h index 9ffc9100c0..7c4f3da0a0 100644 --- a/src/include/access/brin_internal.h +++ b/src/include/access/brin_internal.h @@ -27,6 +27,9 @@ typedef struct BrinOpcInfo /* Number of columns stored in an index column of this opclass */ uint16 oi_nstored; + /* Regular processing of NULLs in BrinValues? */ + bool oi_regular_nulls; + /* Opaque pointer for the opclass' private use */ void *oi_opaque; -- 2.25.4 --jsivyprvf2oxfjz3 Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0004-BRIN-bloom-indexes-20200807.patch" ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: appendBinaryStringInfo stuff @ 2023-02-10 12:15 Peter Eisentraut <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Peter Eisentraut @ 2023-02-10 12:15 UTC (permalink / raw) To: pgsql-hackers On 19.12.22 07:13, Peter Eisentraut wrote: > Also, the argument type of appendBinaryStringInfo() is char *. There is > some code that uses this function to assemble some kind of packed binary > layout, which requires a bunch of casts because of this. I think > functions taking binary data plus length should take void * instead, > like memcpy() for example. I found a little follow-up for this one: Make the same change to pq_sendbytes(), which is a thin wrapper around appendBinaryStringInfo(). This would allow getting rid of further casts at call sites. From 0aed766e046ba71b01093b44f0cae3a4098f1e5b Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Fri, 10 Feb 2023 13:11:09 +0100 Subject: [PATCH] Change argument type of pq_sendbytes from char * to void * --- src/backend/libpq/pqformat.c | 2 +- src/backend/utils/adt/array_userfuncs.c | 11 +++++------ src/backend/utils/adt/uuid.c | 2 +- src/backend/utils/adt/varbit.c | 2 +- src/include/libpq/pqformat.h | 2 +- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c index b7e2c7b6c9..d712c80b5e 100644 --- a/src/backend/libpq/pqformat.c +++ b/src/backend/libpq/pqformat.c @@ -123,7 +123,7 @@ pq_beginmessage_reuse(StringInfo buf, char msgtype) * -------------------------------- */ void -pq_sendbytes(StringInfo buf, const char *data, int datalen) +pq_sendbytes(StringInfo buf, const void *data, int datalen) { /* use variant that maintains a trailing null-byte, out of caution */ appendBinaryStringInfo(buf, data, datalen); diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c index c8a8d33ca3..80750191d8 100644 --- a/src/backend/utils/adt/array_userfuncs.c +++ b/src/backend/utils/adt/array_userfuncs.c @@ -654,7 +654,7 @@ array_agg_serialize(PG_FUNCTION_ARGS) pq_sendbyte(&buf, state->typalign); /* dnulls */ - pq_sendbytes(&buf, (char *) state->dnulls, sizeof(bool) * state->nelems); + pq_sendbytes(&buf, state->dnulls, sizeof(bool) * state->nelems); /* * dvalues. By agreement with array_agg_deserialize, when the element @@ -664,8 +664,7 @@ array_agg_serialize(PG_FUNCTION_ARGS) * must be sent first). */ if (state->typbyval) - pq_sendbytes(&buf, (char *) state->dvalues, - sizeof(Datum) * state->nelems); + pq_sendbytes(&buf, state->dvalues, sizeof(Datum) * state->nelems); else { SerialIOData *iodata; @@ -1097,7 +1096,7 @@ array_agg_array_serialize(PG_FUNCTION_ARGS) if (state->nullbitmap) { Assert(state->aitems > 0); - pq_sendbytes(&buf, (char *) state->nullbitmap, (state->aitems + 7) / 8); + pq_sendbytes(&buf, state->nullbitmap, (state->aitems + 7) / 8); } /* nitems */ @@ -1107,10 +1106,10 @@ array_agg_array_serialize(PG_FUNCTION_ARGS) pq_sendint32(&buf, state->ndims); /* dims: XXX should we just send ndims elements? */ - pq_sendbytes(&buf, (char *) state->dims, sizeof(state->dims)); + pq_sendbytes(&buf, state->dims, sizeof(state->dims)); /* lbs */ - pq_sendbytes(&buf, (char *) state->lbs, sizeof(state->lbs)); + pq_sendbytes(&buf, state->lbs, sizeof(state->lbs)); result = pq_endtypsend(&buf); diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c index 76ff6c533f..4f7aa768fd 100644 --- a/src/backend/utils/adt/uuid.c +++ b/src/backend/utils/adt/uuid.c @@ -154,7 +154,7 @@ uuid_send(PG_FUNCTION_ARGS) StringInfoData buffer; pq_begintypsend(&buffer); - pq_sendbytes(&buffer, (char *) uuid->data, UUID_LEN); + pq_sendbytes(&buffer, uuid->data, UUID_LEN); PG_RETURN_BYTEA_P(pq_endtypsend(&buffer)); } diff --git a/src/backend/utils/adt/varbit.c b/src/backend/utils/adt/varbit.c index 4fde0c1b14..3dbbd1207f 100644 --- a/src/backend/utils/adt/varbit.c +++ b/src/backend/utils/adt/varbit.c @@ -685,7 +685,7 @@ varbit_send(PG_FUNCTION_ARGS) pq_begintypsend(&buf); pq_sendint32(&buf, VARBITLEN(s)); - pq_sendbytes(&buf, (char *) VARBITS(s), VARBITBYTES(s)); + pq_sendbytes(&buf, VARBITS(s), VARBITBYTES(s)); PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h index c5644c0061..0d2f958af3 100644 --- a/src/include/libpq/pqformat.h +++ b/src/include/libpq/pqformat.h @@ -22,7 +22,7 @@ extern void pq_beginmessage_reuse(StringInfo buf, char msgtype); extern void pq_endmessage(StringInfo buf); extern void pq_endmessage_reuse(StringInfo buf); -extern void pq_sendbytes(StringInfo buf, const char *data, int datalen); +extern void pq_sendbytes(StringInfo buf, const void *data, int datalen); extern void pq_sendcountedtext(StringInfo buf, const char *str, int slen, bool countincludesself); extern void pq_sendtext(StringInfo buf, const char *str, int slen); -- 2.39.1 Attachments: [text/plain] 0001-Change-argument-type-of-pq_sendbytes-from-char-to-vo.patch (4.2K, ../../[email protected]/2-0001-Change-argument-type-of-pq_sendbytes-from-char-to-vo.patch) download | inline diff: From 0aed766e046ba71b01093b44f0cae3a4098f1e5b Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Fri, 10 Feb 2023 13:11:09 +0100 Subject: [PATCH] Change argument type of pq_sendbytes from char * to void * --- src/backend/libpq/pqformat.c | 2 +- src/backend/utils/adt/array_userfuncs.c | 11 +++++------ src/backend/utils/adt/uuid.c | 2 +- src/backend/utils/adt/varbit.c | 2 +- src/include/libpq/pqformat.h | 2 +- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c index b7e2c7b6c9..d712c80b5e 100644 --- a/src/backend/libpq/pqformat.c +++ b/src/backend/libpq/pqformat.c @@ -123,7 +123,7 @@ pq_beginmessage_reuse(StringInfo buf, char msgtype) * -------------------------------- */ void -pq_sendbytes(StringInfo buf, const char *data, int datalen) +pq_sendbytes(StringInfo buf, const void *data, int datalen) { /* use variant that maintains a trailing null-byte, out of caution */ appendBinaryStringInfo(buf, data, datalen); diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c index c8a8d33ca3..80750191d8 100644 --- a/src/backend/utils/adt/array_userfuncs.c +++ b/src/backend/utils/adt/array_userfuncs.c @@ -654,7 +654,7 @@ array_agg_serialize(PG_FUNCTION_ARGS) pq_sendbyte(&buf, state->typalign); /* dnulls */ - pq_sendbytes(&buf, (char *) state->dnulls, sizeof(bool) * state->nelems); + pq_sendbytes(&buf, state->dnulls, sizeof(bool) * state->nelems); /* * dvalues. By agreement with array_agg_deserialize, when the element @@ -664,8 +664,7 @@ array_agg_serialize(PG_FUNCTION_ARGS) * must be sent first). */ if (state->typbyval) - pq_sendbytes(&buf, (char *) state->dvalues, - sizeof(Datum) * state->nelems); + pq_sendbytes(&buf, state->dvalues, sizeof(Datum) * state->nelems); else { SerialIOData *iodata; @@ -1097,7 +1096,7 @@ array_agg_array_serialize(PG_FUNCTION_ARGS) if (state->nullbitmap) { Assert(state->aitems > 0); - pq_sendbytes(&buf, (char *) state->nullbitmap, (state->aitems + 7) / 8); + pq_sendbytes(&buf, state->nullbitmap, (state->aitems + 7) / 8); } /* nitems */ @@ -1107,10 +1106,10 @@ array_agg_array_serialize(PG_FUNCTION_ARGS) pq_sendint32(&buf, state->ndims); /* dims: XXX should we just send ndims elements? */ - pq_sendbytes(&buf, (char *) state->dims, sizeof(state->dims)); + pq_sendbytes(&buf, state->dims, sizeof(state->dims)); /* lbs */ - pq_sendbytes(&buf, (char *) state->lbs, sizeof(state->lbs)); + pq_sendbytes(&buf, state->lbs, sizeof(state->lbs)); result = pq_endtypsend(&buf); diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c index 76ff6c533f..4f7aa768fd 100644 --- a/src/backend/utils/adt/uuid.c +++ b/src/backend/utils/adt/uuid.c @@ -154,7 +154,7 @@ uuid_send(PG_FUNCTION_ARGS) StringInfoData buffer; pq_begintypsend(&buffer); - pq_sendbytes(&buffer, (char *) uuid->data, UUID_LEN); + pq_sendbytes(&buffer, uuid->data, UUID_LEN); PG_RETURN_BYTEA_P(pq_endtypsend(&buffer)); } diff --git a/src/backend/utils/adt/varbit.c b/src/backend/utils/adt/varbit.c index 4fde0c1b14..3dbbd1207f 100644 --- a/src/backend/utils/adt/varbit.c +++ b/src/backend/utils/adt/varbit.c @@ -685,7 +685,7 @@ varbit_send(PG_FUNCTION_ARGS) pq_begintypsend(&buf); pq_sendint32(&buf, VARBITLEN(s)); - pq_sendbytes(&buf, (char *) VARBITS(s), VARBITBYTES(s)); + pq_sendbytes(&buf, VARBITS(s), VARBITBYTES(s)); PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h index c5644c0061..0d2f958af3 100644 --- a/src/include/libpq/pqformat.h +++ b/src/include/libpq/pqformat.h @@ -22,7 +22,7 @@ extern void pq_beginmessage_reuse(StringInfo buf, char msgtype); extern void pq_endmessage(StringInfo buf); extern void pq_endmessage_reuse(StringInfo buf); -extern void pq_sendbytes(StringInfo buf, const char *data, int datalen); +extern void pq_sendbytes(StringInfo buf, const void *data, int datalen); extern void pq_sendcountedtext(StringInfo buf, const char *str, int slen, bool countincludesself); extern void pq_sendtext(StringInfo buf, const char *str, int slen); -- 2.39.1 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: appendBinaryStringInfo stuff @ 2023-02-10 19:08 Corey Huinker <[email protected]> parent: Peter Eisentraut <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Corey Huinker @ 2023-02-10 19:08 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers On Fri, Feb 10, 2023 at 7:16 AM Peter Eisentraut < [email protected]> wrote: > On 19.12.22 07:13, Peter Eisentraut wrote: > > Also, the argument type of appendBinaryStringInfo() is char *. There is > > some code that uses this function to assemble some kind of packed binary > > layout, which requires a bunch of casts because of this. I think > > functions taking binary data plus length should take void * instead, > > like memcpy() for example. > > I found a little follow-up for this one: Make the same change to > pq_sendbytes(), which is a thin wrapper around appendBinaryStringInfo(). > This would allow getting rid of further casts at call sites. > +1 Has all the benefits that 54a177a948b0a773c25c6737d1cc3cc49222a526 had. Passes make check-world. ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-02-10 19:08 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-04-02 00:56 [PATCH 3/8] Move processing of NULLs from BRIN support functions Tomas Vondra <[email protected]> 2023-02-10 12:15 Re: appendBinaryStringInfo stuff Peter Eisentraut <[email protected]> 2023-02-10 19:08 ` Re: appendBinaryStringInfo stuff Corey Huinker <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox