connectivity/source/drivers/firebird/PreparedStatement.cxx | 174 ++++++-------
connectivity/source/drivers/firebird/ResultSet.cxx | 44 ---
connectivity/source/drivers/firebird/ResultSet.hxx | 6
connectivity/source/drivers/firebird/Util.hxx | 2
dbaccess/qa/unit/hsql_binary_import.cxx | 12
dbaccess/qa/unit/tdf126268.cxx | 6
dbaccess/source/filter/hsqldb/rowinputbinary.cxx | 10
7 files changed, 118 insertions(+), 136 deletions(-)
New commits:
commit f1121362f3dd4287577dd7952c6f00a524a81111
Author: Mike Kaganski <mike.kagan...@collabora.com>
AuthorDate: Sun Jul 21 13:26:49 2024 +0500
Commit: Xisco Fauli <xiscofa...@libreoffice.org>
CommitDate: Tue Jul 23 09:00:25 2024 +0200
tdf#156530: fix OPreparedStatement::setString
The problem was, that the function only considered the target XSQLVAR's
sqltype, ignoring sqlsubtype. This led to a situation when assignment
to NUMERIC or DECIMAL was treated as assignment to an underlying type,
like SQL_LONG, performed without taking scale into account.
Use ColumnTypeInfo and its getSdbcType, which provides the correct type,
and add missing cases to setString. Fix setObjectWithInfo to make sure
that the resulting number is correct.
This also fixes export of NUMERIC/DECIMAL in HSQL migration. Previously
it miscalculated the position of decimal separator, which accidentally
went unnoticed in the existing unit test, because it was compensated by
broken handling in Firebird SDBC for the specific numbers in the test.
OResultSet::makeNumericString was also fixed; it didn't handle properly
the case of zero fractional part: initial number 1200 with scale 2 was
converted to a string "12.000" instead of expected "12.00".
Change-Id: I5adac59737d21f91c782fe867d4827fb880fd62a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170812
Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com>
Tested-by: Jenkins
Signed-off-by: Xisco Fauli <xiscofa...@libreoffice.org>
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/170858
diff --git a/connectivity/source/drivers/firebird/PreparedStatement.cxx
b/connectivity/source/drivers/firebird/PreparedStatement.cxx
index ce951d639bae..f761365fb5bb 100644
--- a/connectivity/source/drivers/firebird/PreparedStatement.cxx
+++ b/connectivity/source/drivers/firebird/PreparedStatement.cxx
@@ -193,41 +193,36 @@ void SAL_CALL OPreparedStatement::setString(sal_Int32
nParameterIndex,
checkParameterIndex(nParameterIndex);
setParameterNull(nParameterIndex, false);
- OString str = OUStringToOString(sInput , RTL_TEXTENCODING_UTF8 );
-
XSQLVAR* pVar = m_pInSqlda->sqlvar + (nParameterIndex - 1);
+ ColumnTypeInfo columnType(*pVar);
- int dtype = (pVar->sqltype & ~1); // drop flag bit for now
-
- if (str.getLength() > pVar->sqllen)
- str = str.copy(0, pVar->sqllen);
-
- switch (dtype) {
- case SQL_VARYING:
+ switch (auto sdbcType = columnType.getSdbcType()) {
+ case DataType::VARCHAR:
+ case DataType::CHAR:
{
- const sal_Int32 max_varchar_len = 0xFFFF;
- // First 2 bytes indicate string size
- if (str.getLength() > max_varchar_len)
+ OString str = OUStringToOString(sInput, RTL_TEXTENCODING_UTF8);
+ const ISC_SHORT nLength = std::min(str.getLength(),
static_cast<sal_Int32>(pVar->sqllen));
+ int offset = 0;
+ if (sdbcType == DataType::VARCHAR)
{
- str = str.copy(0, max_varchar_len);
+ // First 2 bytes indicate string size
+ static_assert(sizeof(nLength) == 2, "must match dest memcpy len");
+ memcpy(pVar->sqldata, &nLength, 2);
+ offset = 2;
}
- const sal_uInt16 nLength = str.getLength();
- static_assert(sizeof(nLength) == 2, "must match dest memcpy len");
- memcpy(pVar->sqldata, &nLength, 2);
// Actual data
- memcpy(pVar->sqldata + 2, str.getStr(), str.getLength());
+ memcpy(pVar->sqldata + offset, str.getStr(), nLength);
+ if (sdbcType == DataType::CHAR)
+ {
+ // Fill remainder with spaces
+ memset(pVar->sqldata + offset + nLength, ' ', pVar->sqllen -
nLength);
+ }
break;
}
- case SQL_TEXT:
- memcpy(pVar->sqldata, str.getStr(), str.getLength());
- // Fill remainder with spaces
- memset(pVar->sqldata + str.getLength(), ' ', pVar->sqllen -
str.getLength());
- break;
- case SQL_BLOB: // Clob
- assert( pVar->sqlsubtype == static_cast<short>(BlobSubtype::Clob) );
+ case DataType::CLOB:
setClob(nParameterIndex, sInput );
break;
- case SQL_SHORT:
+ case DataType::SMALLINT:
{
sal_Int32 int32Value = sInput.toInt32();
if ( (int32Value < std::numeric_limits<sal_Int16>::min()) ||
@@ -241,31 +236,38 @@ void SAL_CALL OPreparedStatement::setString(sal_Int32
nParameterIndex,
setShort(nParameterIndex, int32Value);
break;
}
- case SQL_LONG:
+ case DataType::INTEGER:
{
sal_Int32 int32Value = sInput.toInt32();
setInt(nParameterIndex, int32Value);
break;
}
- case SQL_INT64:
+ case DataType::BIGINT:
{
sal_Int64 int64Value = sInput.toInt64();
setLong(nParameterIndex, int64Value);
break;
}
- case SQL_FLOAT:
+ case DataType::FLOAT:
{
float floatValue = sInput.toFloat();
setFloat(nParameterIndex, floatValue);
break;
}
- case SQL_BOOLEAN:
+ case DataType::DOUBLE:
+ setDouble(nParameterIndex, sInput.toDouble());
+ break;
+ case DataType::NUMERIC:
+ case DataType::DECIMAL:
+ return setObjectWithInfo(nParameterIndex, Any{ sInput }, sdbcType,
columnType.getScale());
+ break;
+ case DataType::BOOLEAN:
{
bool boolValue = sInput.toBoolean();
setBoolean(nParameterIndex, boolValue);
break;
}
- case SQL_NULL:
+ case DataType::SQLNULL:
{
// See
https://www.firebirdsql.org/file/documentation/html/en/refdocs/fblangref25/firebird-25-language-reference.html#fblangref25-datatypes-special-sqlnull
pVar->sqldata = nullptr;
@@ -359,32 +361,59 @@ namespace {
* the information of where is the fractional part from a
* string representation of a number. (e.g. 54.654 -> 54654)
*/
-sal_Int64 toNumericWithoutDecimalPlace(const OUString& sSource)
+sal_Int64 toNumericWithoutDecimalPlace(const Any& x, sal_Int32 scale)
{
- OUString sNumber(sSource);
-
- // cut off leading 0 eventually ( eg. 0.567 -> .567)
- (void)sSource.startsWith("0", &sNumber);
+ if (double value = 0; x >>= value)
+ return static_cast<sal_Int64>(value * pow10Integer(scale) + 0.5);
- sal_Int32 nDotIndex = sNumber.indexOf('.');
+ // Can't use conversion of string to double, because it could be not
representable in double
- if( nDotIndex < 0)
+ OUString s;
+ x >>= s;
+ std::u16string_view num(o3tl::trim(s));
+ size_t end = num.starts_with('-') ? 1 : 0;
+ for (bool seenDot = false; end < num.size(); ++end)
{
- return sNumber.toInt64(); // no dot -> it's an integer
+ if (num[end] == '.')
+ {
+ if (seenDot)
+ break;
+ seenDot = true;
+ }
+ else if (!rtl::isAsciiDigit(num[end]))
+ break;
}
- else
+ num = num.substr(0, end);
+
+ // fill in the number with nulls in fractional part.
+ // We need this because e.g. 0.450 != 0.045 despite
+ // their scale is equal
+ OUStringBuffer buffer(num);
+ if (auto dotPos = num.find('.'); dotPos != std::u16string_view::npos) //
there is a dot
{
- // remove dot
- OUStringBuffer sBuffer(15);
- if(nDotIndex > 0)
+ scale -= num.substr(dotPos + 1).size();
+ buffer.remove(dotPos, 1);
+ if (scale < 0)
{
- sBuffer.append(sNumber.subView(0, nDotIndex));
+ assert(buffer.getLength() >= -scale);
+ buffer.truncate(buffer.getLength() + scale);
+ scale = 0;
}
- sBuffer.append(sNumber.subView(nDotIndex + 1));
- return o3tl::toInt64(sBuffer);
}
+ for (sal_Int32 i = 0; i < scale; ++i)
+ buffer.append('0');
+
+ return OUString::unacquired(buffer).toInt64();
}
+double toDouble(const Any& x)
+{
+ if (double value = 0; x >>= value)
+ return value;
+ OUString s;
+ x >>= s;
+ return s.toDouble();
+}
}
//----- XParameters -----------------------------------------------------------
@@ -806,57 +835,20 @@ void SAL_CALL OPreparedStatement::setObjectWithInfo(
sal_Int32 parameterIndex, c
if(sqlType == DataType::DECIMAL || sqlType == DataType::NUMERIC)
{
- double dbValue =0.0;
- OUString sValue;
- if( x >>= dbValue )
- {
- // truncate and round to 'scale' number of decimal places
- sValue = OUString::number( std::floor((dbValue *
pow10Integer(scale)) + .5) / pow10Integer(scale) );
- }
- else
- {
- x >>= sValue;
- }
-
- // fill in the number with nulls in fractional part.
- // We need this because e.g. 0.450 != 0.045 despite
- // their scale is equal
- OUStringBuffer sBuffer(15);
- sBuffer.append(sValue);
- if(sValue.indexOf('.') != -1) // there is a dot
- {
- for(sal_Int32 i=sValue.subView(sValue.indexOf('.')+1).size();
i<scale;i++)
- {
- sBuffer.append('0');
- }
- }
- else
- {
- for (sal_Int32 i=0; i<scale; i++)
- {
- sBuffer.append('0');
- }
- }
-
- sValue = sBuffer.makeStringAndClear();
switch(dType)
{
case SQL_SHORT:
- setValue< sal_Int16 >(parameterIndex,
- static_cast<sal_Int16>(
toNumericWithoutDecimalPlace(sValue) ),
- dType);
- break;
+ return setValue(parameterIndex,
+
static_cast<sal_Int16>(toNumericWithoutDecimalPlace(x, scale)),
+ dType);
case SQL_LONG:
- case SQL_DOUBLE:
- setValue< sal_Int32 >(parameterIndex,
- static_cast<sal_Int32>(
toNumericWithoutDecimalPlace(sValue) ),
- dType);
- break;
+ return setValue(parameterIndex,
+
static_cast<sal_Int32>(toNumericWithoutDecimalPlace(x, scale)),
+ dType);
case SQL_INT64:
- setValue< sal_Int64 >(parameterIndex,
- toNumericWithoutDecimalPlace(sValue),
- dType);
- break;
+ return setValue(parameterIndex,
toNumericWithoutDecimalPlace(x, scale), dType);
+ case SQL_DOUBLE:
+ return setValue(parameterIndex, toDouble(x), dType);
default:
SAL_WARN("connectivity.firebird",
"No Firebird sql type found for numeric or decimal
types");
diff --git a/connectivity/source/drivers/firebird/ResultSet.cxx
b/connectivity/source/drivers/firebird/ResultSet.cxx
index 3402de55ee9b..ce6b875554da 100644
--- a/connectivity/source/drivers/firebird/ResultSet.cxx
+++ b/connectivity/source/drivers/firebird/ResultSet.cxx
@@ -363,7 +363,7 @@ bool OResultSet::isNull(const sal_Int32 nColumnIndex)
return false;
}
-template <typename T>
+template <typename T> requires std::is_integral_v<T>
OUString OResultSet::makeNumericString(const sal_Int32 nColumnIndex)
{
// minus because firebird stores scale as a negative number
@@ -377,40 +377,14 @@ OUString OResultSet::makeNumericString(const sal_Int32
nColumnIndex)
OUStringBuffer sRetBuffer;
T nAllDigits =
*reinterpret_cast<T*>(m_pSqlda->sqlvar[nColumnIndex-1].sqldata);
- sal_Int64 nDecimalCountExp = pow10Integer(nDecimalCount);
-
- if(nAllDigits < 0)
- {
- sRetBuffer.append('-');
- nAllDigits = -nAllDigits; // abs
- }
-
- sRetBuffer.append(static_cast<sal_Int64>(nAllDigits / nDecimalCountExp) );
- if( nDecimalCount > 0)
- {
- sRetBuffer.append('.');
-
- sal_Int64 nFractionalPart = nAllDigits % nDecimalCountExp;
-
- int iCount = 0; // digit count
- sal_Int64 nFracTemp = nFractionalPart;
- while(nFracTemp>0)
- {
- nFracTemp /= 10;
- iCount++;
- }
-
- int nMissingNulls = nDecimalCount - iCount;
-
- // append nulls after dot and before nFractionalPart
- for(int i=0; i<nMissingNulls; i++)
- {
- sRetBuffer.append('0');
- }
-
- // the rest
- sRetBuffer.append(nFractionalPart);
- }
+ sRetBuffer.append(static_cast<sal_Int64>(nAllDigits));
+ sal_Int32 insertionPos = nAllDigits < 0 ? 1 : 0; // consider leading minus
+ int nMissingNulls = nDecimalCount - (sRetBuffer.getLength() -
insertionPos) + 1;
+ for (int i = 0; i < nMissingNulls; ++i)
+ sRetBuffer.insert(insertionPos, '0');
+
+ if (nDecimalCount)
+ sRetBuffer.insert(sRetBuffer.getLength() - nDecimalCount, '.');
return sRetBuffer.makeStringAndClear();
}
diff --git a/connectivity/source/drivers/firebird/ResultSet.hxx
b/connectivity/source/drivers/firebird/ResultSet.hxx
index fdae21dfbaaf..8cad4276e732 100644
--- a/connectivity/source/drivers/firebird/ResultSet.hxx
+++ b/connectivity/source/drivers/firebird/ResultSet.hxx
@@ -35,6 +35,8 @@
#include <com/sun/star/sdbc/XRow.hpp>
#include <com/sun/star/sdbc/XResultSetMetaDataSupplier.hpp>
+#include <type_traits>
+
namespace connectivity::firebird
{
/*
@@ -86,8 +88,8 @@ namespace connectivity::firebird
bool isNull(const sal_Int32 nColumnIndex);
- template <typename T> OUString makeNumericString(
- const sal_Int32 nColumnIndex);
+ template <typename T> requires std::is_integral_v<T>
+ OUString makeNumericString(const sal_Int32 nColumnIndex);
template <typename T> T retrieveValue(const sal_Int32
nColumnIndex,
const ISC_SHORT nType);
diff --git a/connectivity/source/drivers/firebird/Util.hxx
b/connectivity/source/drivers/firebird/Util.hxx
index 4cbf49879465..3e0e806bd5de 100644
--- a/connectivity/source/drivers/firebird/Util.hxx
+++ b/connectivity/source/drivers/firebird/Util.hxx
@@ -63,7 +63,7 @@ public:
, m_aSubType(aSubType)
, m_nScale(nScale)
, m_sCharsetName(std::move(sCharset)) {}
- explicit ColumnTypeInfo(const XSQLVAR& var, OUString sCharset)
+ explicit ColumnTypeInfo(const XSQLVAR& var, OUString sCharset = {})
: ColumnTypeInfo(var.sqltype, var.sqlsubtype, -var.sqlscale,
std::move(sCharset)) {}
explicit ColumnTypeInfo(const XSQLDA* pXSQLDA, sal_Int32 column,
OUString sCharset = {})
: ColumnTypeInfo(pXSQLDA->sqlvar[column-1],
std::move(sCharset)) {}
diff --git a/dbaccess/qa/unit/hsql_binary_import.cxx
b/dbaccess/qa/unit/hsql_binary_import.cxx
index 2e8664aeccb4..4a0f6dd525b6 100644
--- a/dbaccess/qa/unit/hsql_binary_import.cxx
+++ b/dbaccess/qa/unit/hsql_binary_import.cxx
@@ -87,6 +87,18 @@ void HsqlBinaryImportTest::testBinaryImport()
CPPUNIT_ASSERT_EQUAL(sal_uInt16{ 2 }, date.Month);
CPPUNIT_ASSERT_EQUAL(sal_Int16{ 1998 }, date.Year);
+ // assert third row
+ CPPUNIT_ASSERT(xRes->next());
+ CPPUNIT_ASSERT_EQUAL(sal_Int16(3), xRow->getShort(1)); // ID
+ CPPUNIT_ASSERT_EQUAL(u"12.00"_ustr, xRow->getString(2)); // numeric
+ CPPUNIT_ASSERT_EQUAL(u"mind-reading"_ustr, xRow->getString(3)); // varchar
+ CPPUNIT_ASSERT(xRow->getBoolean(4)); // boolean
+
+ date = xRow->getDate(5);
+ CPPUNIT_ASSERT_EQUAL(sal_uInt16{ 20 }, date.Day);
+ CPPUNIT_ASSERT_EQUAL(sal_uInt16{ 5 }, date.Month);
+ CPPUNIT_ASSERT_EQUAL(sal_Int16{ 1967 }, date.Year);
+
if (!oldValue)
{
std::shared_ptr<comphelper::ConfigurationChanges> xChanges(
diff --git a/dbaccess/qa/unit/tdf126268.cxx b/dbaccess/qa/unit/tdf126268.cxx
index c82299aef022..ab8ff19211b2 100644
--- a/dbaccess/qa/unit/tdf126268.cxx
+++ b/dbaccess/qa/unit/tdf126268.cxx
@@ -45,9 +45,9 @@ struct expect_t
}
const expect_t expect[] = {
- { 1, u"0.00"_ustr }, { 2, u"25.00"_ustr }, { 3, u"26.00"_ustr },
- { 4, u"30.4"_ustr }, { 5, u"45.8"_ustr }, { 6, u"-25.00"_ustr },
- { 7, u"-26.00"_ustr }, { 8, u"-30.4"_ustr }, { 9, u"-45.8"_ustr },
+ { 1, u"0.0"_ustr }, { 2, u"25.0"_ustr }, { 3, u"26.0"_ustr },
+ { 4, u"30.4"_ustr }, { 5, u"45.8"_ustr }, { 6, u"-25.0"_ustr },
+ { 7, u"-26.0"_ustr }, { 8, u"-30.4"_ustr }, { 9, u"-45.8"_ustr },
};
void Tdf126268Test::testNumbers()
diff --git a/dbaccess/source/filter/hsqldb/rowinputbinary.cxx
b/dbaccess/source/filter/hsqldb/rowinputbinary.cxx
index 7aa11ed5f49c..68946a16457b 100644
--- a/dbaccess/source/filter/hsqldb/rowinputbinary.cxx
+++ b/dbaccess/source/filter/hsqldb/rowinputbinary.cxx
@@ -115,16 +115,18 @@ OUString
lcl_makeStringFromBigint(std::vector<sal_uInt8>&& aBytes)
return sRet.makeStringAndClear();
}
-OUString lcl_putDot(std::u16string_view sNum, sal_Int32 nScale)
+OUString lcl_putDot(const OUString& sNum, sal_Int32 nScale)
{
// e.g. sNum = "0", nScale = 2 -> "0.00"
+ if (nScale <= 0)
+ return sNum;
+
OUStringBuffer sBuf{ sNum };
- sal_Int32 nNullsToAppend = nScale - sNum.size() + 1;
+ sal_Int32 nNullsToAppend = nScale - sNum.getLength() + 1;
for (sal_Int32 i = 0; i < nNullsToAppend; ++i)
sBuf.insert(0, "0");
- if (nScale > 0)
- sBuf.insert(sBuf.getLength() - 1 - nScale, ".");
+ sBuf.insert(sBuf.getLength() - nScale, ".");
return sBuf.makeStringAndClear();
}
}
