From 39365be9d43055c752a9b23e93b51989164b992c Mon Sep 17 00:00:00 2001 From: Mats Kindahl Date: Fri, 18 Oct 2024 14:06:01 +0200 Subject: [PATCH] Add assert for fetch_att and store_att_byval These two functions throw an error if you pass in a strange combination of typbyval and typlen, so we assert on bad values to get a backtrace. --- src/utils.h | 10 ++++++++++ tsl/src/compression/algorithms/datum_serialize.c | 8 +++++++- tsl/src/hypercore/arrow_array.c | 3 ++- tsl/src/nodes/decompress_chunk/pred_vector_array.c | 7 +++---- 4 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/utils.h b/src/utils.h index a01415b4e2c..3ad6dbe847e 100644 --- a/src/utils.h +++ b/src/utils.h @@ -188,6 +188,16 @@ extern TSDLLEXPORT List *ts_get_reloptions(Oid relid); .value = 0, .isnull = true \ } +static inline Datum +ts_fetch_att(const void *T, bool attbyval, int attlen) +{ + /* Length should be set to something sensible, otherwise an error will be + * raised by fetch_att, so we assert this here to get a stack for + * violations. */ + Assert(!attbyval || (attlen > 0 && attlen <= 8)); + return fetch_att(T, attbyval, attlen); +} + static inline int64 int64_min(int64 a, int64 b) { diff --git a/tsl/src/compression/algorithms/datum_serialize.c b/tsl/src/compression/algorithms/datum_serialize.c index 18d79e76634..0e51bb435ee 100644 --- a/tsl/src/compression/algorithms/datum_serialize.c +++ b/tsl/src/compression/algorithms/datum_serialize.c @@ -19,6 +19,7 @@ #include #include "datum_serialize.h" +#include "src/utils.h" #include typedef struct DatumSerializer @@ -167,6 +168,11 @@ datum_to_bytes_and_advance(DatumSerializer *serializer, char *start, Size *max_s start = align_and_zero(start, serializer->type_align, max_size); data_length = serializer->type_len; check_allowed_data_len(data_length, *max_size); + + /* Data length should be set to something sensible, otherwise an error + * will be raised inside store_att_byval, so we assert here to get a + * stack. */ + Assert(data_length > 0 && data_length <= 8); store_att_byval(start, datum, data_length); } else if (serializer->type_len == -1) @@ -322,7 +328,7 @@ bytes_to_datum_and_advance(DatumDeserializer *deserializer, const char **ptr) CheckCompressedData((VARATT_IS_1B(*ptr) && VARSIZE_1B(*ptr) >= VARHDRSZ_SHORT) || (VARSIZE_4B(*ptr) > VARHDRSZ)); } - res = fetch_att(*ptr, deserializer->type_by_val, deserializer->type_len); + res = ts_fetch_att(*ptr, deserializer->type_by_val, deserializer->type_len); *ptr = att_addlength_pointer(*ptr, deserializer->type_len, *ptr); return res; } diff --git a/tsl/src/hypercore/arrow_array.c b/tsl/src/hypercore/arrow_array.c index 66b81419fb7..74adca3871d 100644 --- a/tsl/src/hypercore/arrow_array.c +++ b/tsl/src/hypercore/arrow_array.c @@ -13,6 +13,7 @@ #include "arrow_array.h" #include "compression/arrow_c_data_interface.h" #include "compression/compression.h" +#include "src/utils.h" #define TYPLEN_VARLEN (-1) @@ -470,7 +471,7 @@ arrow_get_datum_fixlen(const ArrowArray *array, Oid typid, int16 typlen, uint16 /* In order to handle fixed-length values of arbitrary size that are byref * and byval, we use fetch_all() rather than rolling our own. This is * taken from utils/adt/rangetypes.c */ - Datum datum = fetch_att(&values[index * typlen], apriv->typbyval, typlen); + Datum datum = ts_fetch_att(&values[index * typlen], apriv->typbyval, typlen); TS_DEBUG_LOG("retrieved fixlen value %s row %u from offset %u" " in memory context %s", diff --git a/tsl/src/nodes/decompress_chunk/pred_vector_array.c b/tsl/src/nodes/decompress_chunk/pred_vector_array.c index bd0cc1c193e..40b3220b8e2 100644 --- a/tsl/src/nodes/decompress_chunk/pred_vector_array.c +++ b/tsl/src/nodes/decompress_chunk/pred_vector_array.c @@ -7,10 +7,9 @@ #include #include "compression/arrow_c_data_interface.h" - -#include "vector_predicates.h" - #include "compression/compression.h" +#include "src/utils.h" +#include "vector_predicates.h" /* * Vectorized implementation of ScalarArrayOpExpr. Applies scalar_predicate for @@ -77,7 +76,7 @@ vector_array_predicate(VectorPredicate *vector_const_predicate, bool is_or, } return; } - Datum constvalue = fetch_att(array_data, typbyval, typlen); + Datum constvalue = ts_fetch_att(array_data, typbyval, typlen); array_data = att_addlength_pointer(array_data, typlen, array_data); array_data = (const char *) att_align_nominal(array_data, typalign);