From 2f459bb26283de4c1227dace89b7e49672561c0d Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Wed, 18 Jun 2025 14:12:20 +0300 Subject: [PATCH 1/4] Move impure-related string allocation to helper methods --- src/common/dsc.h | 5 +++ src/dsql/ExprNodes.cpp | 68 ++--------------------------------------- src/dsql/StmtNodes.cpp | 14 +-------- src/jrd/SysFunction.cpp | 26 +--------------- src/jrd/evl.cpp | 18 ++--------- src/jrd/val.h | 41 +++++++++++++++++++++++++ 6 files changed, 54 insertions(+), 118 deletions(-) diff --git a/src/common/dsc.h b/src/common/dsc.h index b39779b7a3b..e71d4f4716f 100644 --- a/src/common/dsc.h +++ b/src/common/dsc.h @@ -228,6 +228,11 @@ typedef struct dsc return dsc_dtype == dtype_unknown; } + bool hasDynamicLength() const + { + return isText(); // TODO: add isJson(); + } + SSHORT getBlobSubType() const { if (isBlob()) diff --git a/src/dsql/ExprNodes.cpp b/src/dsql/ExprNodes.cpp index 1fccbee3e11..21354d6f5da 100644 --- a/src/dsql/ExprNodes.cpp +++ b/src/dsql/ExprNodes.cpp @@ -3752,25 +3752,8 @@ dsc* CastNode::perform(thread_db* tdbb, impure_value* impure, dsc* value, impure->vlu_desc.dsc_length = length; } - length = impure->vlu_desc.dsc_length; - // Allocate a string block of sufficient size. - - auto string = impure->vlu_string; - - if (string && string->str_length < length) - { - delete string; - string = nullptr; - } - - if (!string) - { - string = impure->vlu_string = FB_NEW_RPT(*tdbb->getDefaultPool(), length) VaryingString(); - string->str_length = length; - } - - impure->vlu_desc.dsc_address = string->str_data; + impure->allocateTextImpureDscAddress(*tdbb->getDefaultPool()); } EVL_validate(tdbb, Item(Item::TYPE_CAST), itemInfo, @@ -7139,29 +7122,7 @@ dsc* FieldNode::execute(thread_db* tdbb, Request* request) const dsc desc = impure->vlu_desc; impure->vlu_desc = format->fmt_desc[fieldId]; - if (impure->vlu_desc.isText()) - { - // Allocate a string block of sufficient size. - VaryingString* string = impure->vlu_string; - - if (string && string->str_length < impure->vlu_desc.dsc_length) - { - delete string; - string = NULL; - } - - if (!string) - { - string = impure->vlu_string = FB_NEW_RPT(*tdbb->getDefaultPool(), - impure->vlu_desc.dsc_length) VaryingString(); - string->str_length = impure->vlu_desc.dsc_length; - } - - impure->vlu_desc.dsc_address = string->str_data; - } - else - impure->vlu_desc.dsc_address = (UCHAR*) &impure->vlu_misc; - + impure->makeImpureDscAddress(*tdbb->getDefaultPool()); MOV_move(tdbb, &desc, &impure->vlu_desc); } @@ -13687,30 +13648,7 @@ dsc* UdfCallNode::execute(thread_db* tdbb, Request* request) const const Parameter* const returnParam = function->getOutputFields()[0]; value->vlu_desc = returnParam->prm_desc; - // If the return data type is any of the string types, allocate space to hold value. - - if (value->vlu_desc.dsc_dtype <= dtype_varying) - { - const USHORT retLength = value->vlu_desc.dsc_length; - VaryingString* string = value->vlu_string; - - if (string && string->str_length < retLength) - { - delete string; - string = NULL; - } - - if (!string) - { - string = FB_NEW_RPT(*tdbb->getDefaultPool(), retLength) VaryingString; - string->str_length = retLength; - value->vlu_string = string; - } - - value->vlu_desc.dsc_address = string->str_data; - } - else - value->vlu_desc.dsc_address = (UCHAR*) &value->vlu_misc; + value->makeImpureDscAddress(*tdbb->getDefaultPool()); if (!impureArea->temp) { diff --git a/src/dsql/StmtNodes.cpp b/src/dsql/StmtNodes.cpp index f1b8f15d739..67727bec6b7 100644 --- a/src/dsql/StmtNodes.cpp +++ b/src/dsql/StmtNodes.cpp @@ -2248,19 +2248,7 @@ const StmtNode* DeclareVariableNode::execute(thread_db* tdbb, Request* request, variable->vlu_desc = varDesc; variable->vlu_desc.clearFlags(); - if (variable->vlu_desc.dsc_dtype <= dtype_varying) - { - if (!variable->vlu_string) - { - const USHORT len = variable->vlu_desc.dsc_length; - variable->vlu_string = FB_NEW_RPT(*tdbb->getDefaultPool(), len) VaryingString(); - variable->vlu_string->str_length = len; - } - - variable->vlu_desc.dsc_address = variable->vlu_string->str_data; - } - else - variable->vlu_desc.dsc_address = (UCHAR*) &variable->vlu_misc; + variable->makeImpureDscAddress(*tdbb->getDefaultPool()); request->req_operation = Request::req_return; } diff --git a/src/jrd/SysFunction.cpp b/src/jrd/SysFunction.cpp index 2f58670f3be..5e4d8cb5baf 100644 --- a/src/jrd/SysFunction.cpp +++ b/src/jrd/SysFunction.cpp @@ -5545,31 +5545,7 @@ dsc* evlMaxMinValue(thread_db* tdbb, const SysFunction* function, const NestValu DataTypeUtil(tdbb).makeFromList(&impure->vlu_desc, function->name, argTypes.getCount(), argTypes.begin()); - if (impure->vlu_desc.isText()) - { - const USHORT length = impure->vlu_desc.dsc_length; - - // Allocate a string block of sufficient size - - auto string = impure->vlu_string; - - if (string && string->str_length < length) - { - delete string; - string = nullptr; - } - - if (!string) - { - string = impure->vlu_string = FB_NEW_RPT(*tdbb->getDefaultPool(), length) VaryingString(); - string->str_length = length; - } - - impure->vlu_desc.dsc_address = string->str_data; - } - else - impure->vlu_desc.dsc_address = (UCHAR*) &impure->vlu_misc; - + impure->makeImpureDscAddress(*tdbb->getDefaultPool()); MOV_move(tdbb, result, &impure->vlu_desc); if (impure->vlu_desc.dsc_dtype == dtype_text) diff --git a/src/jrd/evl.cpp b/src/jrd/evl.cpp index d3fedca597e..ad546b419e0 100644 --- a/src/jrd/evl.cpp +++ b/src/jrd/evl.cpp @@ -573,22 +573,10 @@ void EVL_make_value(thread_db* tdbb, const dsc* desc, impure_value* value, Memor // Allocate a string block of sufficient size. - VaryingString* string = value->vlu_string; + if (!pool) + pool = tdbb->getDefaultPool(); - if (string && string->str_length < length) - { - delete string; - string = NULL; - } - - if (!string) - { - if (!pool) - pool = tdbb->getDefaultPool(); - - string = value->vlu_string = FB_NEW_RPT(*pool, length) VaryingString(); - string->str_length = length; - } + VaryingString* string = value->getAllocateString(from.dsc_length, *pool); value->vlu_desc.dsc_length = length; UCHAR* target = string->str_data; diff --git a/src/jrd/val.h b/src/jrd/val.h index 13814943f1f..4aa7208bc11 100644 --- a/src/jrd/val.h +++ b/src/jrd/val.h @@ -168,6 +168,13 @@ struct impure_value void make_double(const double val); void make_decimal128(const Firebird::Decimal128 val); void make_decimal_fixed(const Firebird::Int128 val, const signed char scale); + + template + VaryingString* getAllocateString(T requeuedLength, MemoryPool& pool) = delete; // Prevent dangerous length shrink + VaryingString* getAllocateString(USHORT requeuedLength, MemoryPool& pool); + + void makeImpureDscAddress(MemoryPool& pool); + void allocateTextImpureDscAddress(MemoryPool& pool); }; // Do not use these methods where dsc_sub_type is not explicitly set to zero. @@ -221,6 +228,40 @@ inline void impure_value::make_decimal_fixed(const Firebird::Int128 val, const s this->vlu_desc.dsc_address = reinterpret_cast(&this->vlu_misc.vlu_int128); } +inline VaryingString* impure_value::getAllocateString(USHORT requeuedLength, MemoryPool& pool) +{ + if (vlu_string && vlu_string->str_length < requeuedLength) + { + delete vlu_string; + vlu_string = nullptr; + } + + if (!vlu_string) + { + vlu_string = FB_NEW_RPT(pool, requeuedLength) VaryingString(); + vlu_string->str_length = requeuedLength; + } + + return vlu_string; +} + +inline void impure_value::makeImpureDscAddress(MemoryPool& pool) +{ + if (vlu_desc.hasDynamicLength()) + { + // If the data type is any of the string types, allocate space to hold value. + allocateTextImpureDscAddress(pool); + } + else + vlu_desc.dsc_address = (UCHAR*) &vlu_misc; +} + +inline void impure_value::allocateTextImpureDscAddress(MemoryPool& pool) +{ + vlu_desc.dsc_address = getAllocateString(vlu_desc.dsc_length, pool)->str_data; +} + + struct impure_value_ex : public impure_value { SINT64 vlux_count; From 4f6204019fb788f73ec3540178a70194902cf6a1 Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Thu, 19 Jun 2025 09:31:40 +0300 Subject: [PATCH 2/4] Use better name for impure string make method --- src/jrd/evl.cpp | 2 +- src/jrd/val.h | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/jrd/evl.cpp b/src/jrd/evl.cpp index ad546b419e0..5cf45ad0605 100644 --- a/src/jrd/evl.cpp +++ b/src/jrd/evl.cpp @@ -576,7 +576,7 @@ void EVL_make_value(thread_db* tdbb, const dsc* desc, impure_value* value, Memor if (!pool) pool = tdbb->getDefaultPool(); - VaryingString* string = value->getAllocateString(from.dsc_length, *pool); + VaryingString* string = value->getString(*pool, length); value->vlu_desc.dsc_length = length; UCHAR* target = string->str_data; diff --git a/src/jrd/val.h b/src/jrd/val.h index 4aa7208bc11..29a15e72e38 100644 --- a/src/jrd/val.h +++ b/src/jrd/val.h @@ -170,8 +170,8 @@ struct impure_value void make_decimal_fixed(const Firebird::Int128 val, const signed char scale); template - VaryingString* getAllocateString(T requeuedLength, MemoryPool& pool) = delete; // Prevent dangerous length shrink - VaryingString* getAllocateString(USHORT requeuedLength, MemoryPool& pool); + VaryingString* getString(MemoryPool& pool, const T length) = delete; // Prevent dangerous length shrink + VaryingString* getString(MemoryPool& pool, const USHORT length); void makeImpureDscAddress(MemoryPool& pool); void allocateTextImpureDscAddress(MemoryPool& pool); @@ -228,9 +228,9 @@ inline void impure_value::make_decimal_fixed(const Firebird::Int128 val, const s this->vlu_desc.dsc_address = reinterpret_cast(&this->vlu_misc.vlu_int128); } -inline VaryingString* impure_value::getAllocateString(USHORT requeuedLength, MemoryPool& pool) +inline VaryingString* impure_value::getString(MemoryPool& pool, const USHORT length) { - if (vlu_string && vlu_string->str_length < requeuedLength) + if (vlu_string && vlu_string->str_length < length) { delete vlu_string; vlu_string = nullptr; @@ -238,8 +238,8 @@ inline VaryingString* impure_value::getAllocateString(USHORT requeuedLength, Mem if (!vlu_string) { - vlu_string = FB_NEW_RPT(pool, requeuedLength) VaryingString(); - vlu_string->str_length = requeuedLength; + vlu_string = FB_NEW_RPT(pool, length) VaryingString(); + vlu_string->str_length = length; } return vlu_string; @@ -258,7 +258,7 @@ inline void impure_value::makeImpureDscAddress(MemoryPool& pool) inline void impure_value::allocateTextImpureDscAddress(MemoryPool& pool) { - vlu_desc.dsc_address = getAllocateString(vlu_desc.dsc_length, pool)->str_data; + vlu_desc.dsc_address = getString(pool, vlu_desc.dsc_length)->str_data; } From 5b9af0e943e2c1b7286e3404da9ca6867f07ed9b Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Thu, 19 Jun 2025 09:36:33 +0300 Subject: [PATCH 3/4] Check dsc_length via type_lengths array --- src/common/dsc.h | 5 ----- src/jrd/val.h | 3 ++- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/common/dsc.h b/src/common/dsc.h index e71d4f4716f..b39779b7a3b 100644 --- a/src/common/dsc.h +++ b/src/common/dsc.h @@ -228,11 +228,6 @@ typedef struct dsc return dsc_dtype == dtype_unknown; } - bool hasDynamicLength() const - { - return isText(); // TODO: add isJson(); - } - SSHORT getBlobSubType() const { if (isBlob()) diff --git a/src/jrd/val.h b/src/jrd/val.h index 29a15e72e38..fe352f2324d 100644 --- a/src/jrd/val.h +++ b/src/jrd/val.h @@ -35,6 +35,7 @@ #include "../jrd/MetaName.h" #include "../jrd/RecordNumber.h" #include "../common/dsc.h" +#include "../jrd/align.h" #define FLAG_BYTES(n) (((n + BITS_PER_LONG) & ~((ULONG)BITS_PER_LONG - 1)) >> 3) @@ -247,7 +248,7 @@ inline VaryingString* impure_value::getString(MemoryPool& pool, const USHORT len inline void impure_value::makeImpureDscAddress(MemoryPool& pool) { - if (vlu_desc.hasDynamicLength()) + if (type_lengths[vlu_desc.dsc_dtype] == 0) { // If the data type is any of the string types, allocate space to hold value. allocateTextImpureDscAddress(pool); From ccc9f51a883bedb08764edc22722b4437ac38641 Mon Sep 17 00:00:00 2001 From: Artyom Abakumov Date: Thu, 19 Jun 2025 09:47:37 +0300 Subject: [PATCH 4/4] Use different names for new impure helper methods --- src/dsql/ExprNodes.cpp | 6 +++--- src/dsql/StmtNodes.cpp | 2 +- src/jrd/SysFunction.cpp | 2 +- src/jrd/val.h | 10 +++++----- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/dsql/ExprNodes.cpp b/src/dsql/ExprNodes.cpp index 21354d6f5da..7529e45166b 100644 --- a/src/dsql/ExprNodes.cpp +++ b/src/dsql/ExprNodes.cpp @@ -3753,7 +3753,7 @@ dsc* CastNode::perform(thread_db* tdbb, impure_value* impure, dsc* value, } // Allocate a string block of sufficient size. - impure->allocateTextImpureDscAddress(*tdbb->getDefaultPool()); + impure->makeTextValueAddress(*tdbb->getDefaultPool()); } EVL_validate(tdbb, Item(Item::TYPE_CAST), itemInfo, @@ -7122,7 +7122,7 @@ dsc* FieldNode::execute(thread_db* tdbb, Request* request) const dsc desc = impure->vlu_desc; impure->vlu_desc = format->fmt_desc[fieldId]; - impure->makeImpureDscAddress(*tdbb->getDefaultPool()); + impure->makeValueAddress(*tdbb->getDefaultPool()); MOV_move(tdbb, &desc, &impure->vlu_desc); } @@ -13648,7 +13648,7 @@ dsc* UdfCallNode::execute(thread_db* tdbb, Request* request) const const Parameter* const returnParam = function->getOutputFields()[0]; value->vlu_desc = returnParam->prm_desc; - value->makeImpureDscAddress(*tdbb->getDefaultPool()); + value->makeValueAddress(*tdbb->getDefaultPool()); if (!impureArea->temp) { diff --git a/src/dsql/StmtNodes.cpp b/src/dsql/StmtNodes.cpp index 67727bec6b7..fe3503544ef 100644 --- a/src/dsql/StmtNodes.cpp +++ b/src/dsql/StmtNodes.cpp @@ -2248,7 +2248,7 @@ const StmtNode* DeclareVariableNode::execute(thread_db* tdbb, Request* request, variable->vlu_desc = varDesc; variable->vlu_desc.clearFlags(); - variable->makeImpureDscAddress(*tdbb->getDefaultPool()); + variable->makeValueAddress(*tdbb->getDefaultPool()); request->req_operation = Request::req_return; } diff --git a/src/jrd/SysFunction.cpp b/src/jrd/SysFunction.cpp index 5e4d8cb5baf..07e908fea13 100644 --- a/src/jrd/SysFunction.cpp +++ b/src/jrd/SysFunction.cpp @@ -5545,7 +5545,7 @@ dsc* evlMaxMinValue(thread_db* tdbb, const SysFunction* function, const NestValu DataTypeUtil(tdbb).makeFromList(&impure->vlu_desc, function->name, argTypes.getCount(), argTypes.begin()); - impure->makeImpureDscAddress(*tdbb->getDefaultPool()); + impure->makeValueAddress(*tdbb->getDefaultPool()); MOV_move(tdbb, result, &impure->vlu_desc); if (impure->vlu_desc.dsc_dtype == dtype_text) diff --git a/src/jrd/val.h b/src/jrd/val.h index fe352f2324d..fc3b6d64c28 100644 --- a/src/jrd/val.h +++ b/src/jrd/val.h @@ -174,8 +174,8 @@ struct impure_value VaryingString* getString(MemoryPool& pool, const T length) = delete; // Prevent dangerous length shrink VaryingString* getString(MemoryPool& pool, const USHORT length); - void makeImpureDscAddress(MemoryPool& pool); - void allocateTextImpureDscAddress(MemoryPool& pool); + void makeValueAddress(MemoryPool& pool); + void makeTextValueAddress(MemoryPool& pool); }; // Do not use these methods where dsc_sub_type is not explicitly set to zero. @@ -246,18 +246,18 @@ inline VaryingString* impure_value::getString(MemoryPool& pool, const USHORT len return vlu_string; } -inline void impure_value::makeImpureDscAddress(MemoryPool& pool) +inline void impure_value::makeValueAddress(MemoryPool& pool) { if (type_lengths[vlu_desc.dsc_dtype] == 0) { // If the data type is any of the string types, allocate space to hold value. - allocateTextImpureDscAddress(pool); + makeTextValueAddress(pool); } else vlu_desc.dsc_address = (UCHAR*) &vlu_misc; } -inline void impure_value::allocateTextImpureDscAddress(MemoryPool& pool) +inline void impure_value::makeTextValueAddress(MemoryPool& pool) { vlu_desc.dsc_address = getString(pool, vlu_desc.dsc_length)->str_data; }