From 0b81f68def794831e0d8096f1b73c8cfb57b8f94 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Tue, 11 Oct 2022 14:57:31 +0300 Subject: [PATCH] Use memcpy instead of memcpy_s to avoid pointless limits in columnar (#6419) DESCRIPTION: Raises memory limits in columnar from 256MB to 1GB for reads and writes This doesn't completely fix #5918 but at least increases the buffer limits that might cause throwing an error when reading from or writing into into columnar storage. A way better approach to fix this is documented in #6420. Replacing memcpy_s with memcpy is quite safe in those places since we anyway make sure to allocate enough amount of memory before writing into related buffers. --- src/backend/columnar/columnar_metadata.c | 20 ++++++++++++-------- src/backend/columnar/columnar_tableam.c | 9 +++++++-- src/backend/columnar/columnar_writer.c | 24 +++++++++++++++++------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/backend/columnar/columnar_metadata.c b/src/backend/columnar/columnar_metadata.c index 34939710d..8154beb46 100644 --- a/src/backend/columnar/columnar_metadata.c +++ b/src/backend/columnar/columnar_metadata.c @@ -1650,6 +1650,9 @@ create_estate_for_relation(Relation rel) /* * DatumToBytea serializes a datum into a bytea value. + * + * Since we don't want to limit datum size to RSIZE_MAX unnecessarily, + * we use memcpy instead of memcpy_s several places in this function. */ static bytea * DatumToBytea(Datum value, Form_pg_attribute attrForm) @@ -1666,19 +1669,16 @@ DatumToBytea(Datum value, Form_pg_attribute attrForm) Datum tmp; store_att_byval(&tmp, value, attrForm->attlen); - memcpy_s(VARDATA(result), datumLength + VARHDRSZ, - &tmp, attrForm->attlen); + memcpy(VARDATA(result), &tmp, attrForm->attlen); /* IGNORE-BANNED */ } else { - memcpy_s(VARDATA(result), datumLength + VARHDRSZ, - DatumGetPointer(value), attrForm->attlen); + memcpy(VARDATA(result), DatumGetPointer(value), attrForm->attlen); /* IGNORE-BANNED */ } } else { - memcpy_s(VARDATA(result), datumLength + VARHDRSZ, - DatumGetPointer(value), datumLength); + memcpy(VARDATA(result), DatumGetPointer(value), datumLength); /* IGNORE-BANNED */ } return result; @@ -1697,8 +1697,12 @@ ByteaToDatum(bytea *bytes, Form_pg_attribute attrForm) * after the byteaDatum is freed. */ char *binaryDataCopy = palloc0(VARSIZE_ANY_EXHDR(bytes)); - memcpy_s(binaryDataCopy, VARSIZE_ANY_EXHDR(bytes), - VARDATA_ANY(bytes), VARSIZE_ANY_EXHDR(bytes)); + + /* + * We use IGNORE-BANNED here since we don't want to limit datum size to + * RSIZE_MAX unnecessarily. + */ + memcpy(binaryDataCopy, VARDATA_ANY(bytes), VARSIZE_ANY_EXHDR(bytes)); /* IGNORE-BANNED */ return fetch_att(binaryDataCopy, attrForm->attbyval, attrForm->attlen); } diff --git a/src/backend/columnar/columnar_tableam.c b/src/backend/columnar/columnar_tableam.c index 4f1815103..07175dcbe 100644 --- a/src/backend/columnar/columnar_tableam.c +++ b/src/backend/columnar/columnar_tableam.c @@ -2576,8 +2576,13 @@ detoast_values(TupleDesc tupleDesc, Datum *orig_values, bool *isnull) if (values == orig_values) { values = palloc(sizeof(Datum) * natts); - memcpy_s(values, sizeof(Datum) * natts, - orig_values, sizeof(Datum) * natts); + + /* + * We use IGNORE-BANNED here since we don't want to limit + * size of the buffer that holds the datum array to RSIZE_MAX + * unnecessarily. + */ + memcpy(values, orig_values, sizeof(Datum) * natts); /* IGNORE-BANNED */ } /* will be freed when per-tuple context is reset */ diff --git a/src/backend/columnar/columnar_writer.c b/src/backend/columnar/columnar_writer.c index 1db407b6b..8e35b59b1 100644 --- a/src/backend/columnar/columnar_writer.c +++ b/src/backend/columnar/columnar_writer.c @@ -531,6 +531,9 @@ SerializeBoolArray(bool *boolArray, uint32 boolArrayLength) /* * SerializeSingleDatum serializes the given datum value and appends it to the * provided string info buffer. + * + * Since we don't want to limit datum buffer size to RSIZE_MAX unnecessarily, + * we use memcpy instead of memcpy_s several places in this function. */ static void SerializeSingleDatum(StringInfo datumBuffer, Datum datum, bool datumTypeByValue, @@ -552,15 +555,13 @@ SerializeSingleDatum(StringInfo datumBuffer, Datum datum, bool datumTypeByValue, } else { - memcpy_s(currentDatumDataPointer, datumBuffer->maxlen - datumBuffer->len, - DatumGetPointer(datum), datumTypeLength); + memcpy(currentDatumDataPointer, DatumGetPointer(datum), datumTypeLength); /* IGNORE-BANNED */ } } else { Assert(!datumTypeByValue); - memcpy_s(currentDatumDataPointer, datumBuffer->maxlen - datumBuffer->len, - DatumGetPointer(datum), datumLength); + memcpy(currentDatumDataPointer, DatumGetPointer(datum), datumLength); /* IGNORE-BANNED */ } datumBuffer->len += datumLengthAligned; @@ -714,7 +715,12 @@ DatumCopy(Datum datum, bool datumTypeByValue, int datumTypeLength) { uint32 datumLength = att_addlength_datum(0, datumTypeLength, datum); char *datumData = palloc0(datumLength); - memcpy_s(datumData, datumLength, DatumGetPointer(datum), datumLength); + + /* + * We use IGNORE-BANNED here since we don't want to limit datum size to + * RSIZE_MAX unnecessarily. + */ + memcpy(datumData, DatumGetPointer(datum), datumLength); /* IGNORE-BANNED */ datumCopy = PointerGetDatum(datumData); } @@ -737,8 +743,12 @@ CopyStringInfo(StringInfo sourceString) targetString->data = palloc0(sourceString->len); targetString->len = sourceString->len; targetString->maxlen = sourceString->len; - memcpy_s(targetString->data, sourceString->len, - sourceString->data, sourceString->len); + + /* + * We use IGNORE-BANNED here since we don't want to limit string + * buffer size to RSIZE_MAX unnecessarily. + */ + memcpy(targetString->data, sourceString->data, sourceString->len); /* IGNORE-BANNED */ } return targetString;