Possible type mismatch for t_comm parameter in C version of TRF5_1?

[ksyx]

While running dbt5-run I encountered exceptions in TradeResult, but it appears that the error message of exception thrown in

dbt5/src/transactions/pgsql/DBConnection.cpp

Lines 218 to 220 in 5575431

	<< PQresultErrorMessage(res) << endl;
	rollback();
	throw msg.str().c_str();

has already been freed in the catch block at

dbt5/src/BrokerageHouse/BrokerageHouse.cpp

Lines 197 to 203 in 5575431

	} catch (const char *str) {
	pid_t pid = syscall(SYS_gettid);
	ostringstream msg;
	msg << time(NULL) << " " << pid << " "
	<< szTransactionName[pMessage->TxnType] << endl;
	pThrParam->pBrokerageHouse->logErrorMessage(msg.str());
	iRet = CBaseTxnErr::EXPECTED_ROLLBACK;

, leading str to be a garbage value.
Therefore I ran TestTxn with gdb and got the followings:

(gdb) p sql
$1 = 0x7fffe8009de0 "SELECT * FROM TradeResultFrame5(4300000001,19.54,'CMPT','2024-5-22 0:53:15',200000008728315,27.14)"
(gdb) printf "%s", (const char *)PQresultErrorMessage(res)
ERROR:  numeric field overflow
DETAIL:  A field with precision 10, scale 2 must round to an absolute value less than 10^8.
CONTEXT:  SQL statement "UPDATE trade
SET t_comm = $1
  , t_dts = $2
  , t_st_id = $3
  , t_trade_price = $4
WHERE t_id = $5"

This statement appears to be TRF5_1 and upon checking the implementation I found that all values related with comm_amount is processed using float8/double, and there is not another mention of FLOAT4OID in the repo. After changing it to FLOAT8OID the exception is no longer thrown, but I cannot figure out a reason, and I am also wondering whether there are any rationale behind using FLOAT4 specifically for this parameter? Thanks.

dbt5/scripts/pgsql/dbt5-pgsql-create-tables.in

Line 235 in 5575431

CREATE DOMAIN VALUE_T AS NUMERIC(10,2) NULL;

dbt5/scripts/pgsql/dbt5-pgsql-create-tables.in

Line 392 in 5575431

t_comm VALUE_T NOT NULL CHECK (t_comm >= 0),

dbt5/storedproc/pgsql/c/trade_result.c

Lines 208 to 214 in 5575431

	#define SQLTRF5_1 \
	"UPDATE trade\n" \
	"SET t_comm = $1\n" \
	" , t_dts = $2\n" \
	" , t_st_id = $3\n" \
	" , t_trade_price = $4\n" \
	"WHERE t_id = $5"

dbt5/storedproc/pgsql/c/trade_result.c

Line 1654 in 5575431

double comm_amount;

dbt5/storedproc/pgsql/c/trade_result.c

Lines 1667 to 1668 in 5575431

	comm_amount = DatumGetFloat8(DirectFunctionCall1(
	numeric_float8_no_overflow, PointerGetDatum(comm_amount_num)));

dbt5/storedproc/pgsql/c/trade_result.c

Line 367 in 5575431

{ SQLTRF5_1, 5, { FLOAT4OID, TIMESTAMPOID, TEXTOID, FLOAT8OID, INT8OID } },

[markwkm]

Thanks for catching that. The old C user defined functions need some TLC. :)

I believe float4 was used because S_PRICE_T was defined in a way that float4 should have been big enough. I have seen in other publications that vendors are increasing the size of S_PRICE_T, and I tried to do the same in a couple places. So I think it's correct to make sure we are using float8 instead....

[ksyx]

Thanks for confirming! Another thing I spotted, as in the beginning of this issue, is that the string pointed to msg.str().c_str() seemed to be freed while the exception is being thrown, and this makes the str value in the catch block unable to be used. Fixing this can be helpful as logging str in error message can save some effort making it to run with a debugger. So is it a good idea to create a new issue for this? Thanks

[markwkm]

Yeah, please go ahead and submit another patch for fixing that logging bit, if you have one ready. Thanks!

[markwkm]

I think this has been addressed in f240d86 and e1ce4a7.