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.kaganski@collabora.com>
Tested-by: Jenkins

4 files changed, 97 insertions, 129 deletions

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 19e2ad36cfb8..c80f5b8453d4 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)) {}