Clarify that css::sdbc::XClob::getSubString takes 1-based pos
... and fix connectivity::firebird::Clob::getSubString.
XClob::getSubString is modelled after JDBC counterpart, that also
takes 1-based position argument. This is also described in multiple
places in our code:
- OPreparedStatement::setClob
in connectivity/source/drivers/firebird/PreparedStatement.cxx
- OSingleSelectQueryComposer::setConditionByColumn
in dbaccess/source/core/api/SingleSelectQueryComposer.cxx
- ORowSetValue::getString
in connectivity/source/commontools/FValue.cxx
However in some places 0-based value was used (fixed here).
To clarify, the mention that the pos argument is 1-based is put
to the corresponding IDL file.
Also the code in connectivity::firebird::Clob::getSubString had
multiple issues:
- no checks of arguments;
- possibility to throw "nPosition out of range" when just-read
segment has enough data;
- wrong start position in case when nPosition is not aligned to
segment boundary.
This change fixes these, and simplifies the implementation.
Change-Id: I119a62dd7f02c9569ce36395ed8cc1a98c931fcf
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/121884
Tested-by: Jenkins
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
diff --git a/connectivity/source/drivers/firebird/Clob.cxx b/connectivity/source/drivers/firebird/Clob.cxx
index 5825a5f..707020b 100644
--- a/connectivity/source/drivers/firebird/Clob.cxx
+++ b/connectivity/source/drivers/firebird/Clob.cxx
@@ -71,55 +71,51 @@ sal_Int64 SAL_CALL Clob::length()
OUString SAL_CALL Clob::getSubString(sal_Int64 nPosition,
sal_Int32 nLength)
{
if (nPosition < 1) // XClob is indexed from 1
throw lang::IllegalArgumentException("nPosition < 1", *this, 0);
--nPosition; // make 0-based
if (nLength < 0)
throw lang::IllegalArgumentException("nLength < 0", *this, 0);
MutexGuard aGuard(m_aMutex);
checkDisposed(Clob_BASE::rBHelper.bDisposed);
// TODO do not reset position if it is not necessary
m_aBlob->closeInput(); // reset position
OUStringBuffer sSegmentBuffer;
sal_Int64 nActPos = 1;
sal_Int32 nActLen = 0;
std::vector<char> aSegmentBytes;
// skip irrelevant parts
while( nActPos < nPosition )
for (;;)
{
bool bLastRead = m_aBlob->readOneSegment( aSegmentBytes );
if( bLastRead )
throw lang::IllegalArgumentException("nPosition out of range", *this, 0);
// TODO: handle possible case of split UTF-8 character
OUString sSegment(aSegmentBytes.data(), aSegmentBytes.size(), RTL_TEXTENCODING_UTF8);
sal_Int32 nStrLen = sSegment.getLength();
nActPos += nStrLen;
if( nActPos > nPosition )
// skip irrelevant parts
if (sSegment.getLength() < nPosition)
{
sal_Int32 nCharsToCopy = static_cast<sal_Int32>(nActPos - nPosition);
if( nCharsToCopy > nLength )
nCharsToCopy = nLength;
// append relevant part of first segment
sSegmentBuffer.append( sSegment.subView(0, nCharsToCopy) );
nActLen += sSegmentBuffer.getLength();
if (bLastRead)
throw lang::IllegalArgumentException("nPosition out of range", *this, 0);
nPosition -= sSegment.getLength();
continue;
}
}
// read nLength characters
while( nActLen < nLength )
{
bool bLastRead = m_aBlob->readOneSegment( aSegmentBytes );
// Getting here for the first time, nPosition may be > 0, meaning copy start offset.
// This also handles sSegment.getLength() == nPosition case, including nLength == 0.
const sal_Int32 nCharsToCopy = std::min<sal_Int32>(sSegment.getLength() - nPosition,
nLength - sSegmentBuffer.getLength());
sSegmentBuffer.append(sSegment.subView(nPosition, nCharsToCopy));
if (sSegmentBuffer.getLength() == nLength)
return sSegmentBuffer.makeStringAndClear();
OUString sSegment(aSegmentBytes.data(), aSegmentBytes.size(), RTL_TEXTENCODING_UTF8);
sal_Int32 nStrLen = sSegment.getLength();
if( nActLen + nStrLen > nLength )
sSegmentBuffer.append(sSegment.subView(0, nLength - nActLen));
else
sSegmentBuffer.append(sSegment);
nActLen += nStrLen;
assert(sSegmentBuffer.getLength() < nLength);
if( bLastRead && nActLen < nLength )
if (bLastRead)
throw lang::IllegalArgumentException("out of range", *this, 0);
}
return sSegmentBuffer.makeStringAndClear();
nPosition = 0; // No offset after first append
}
}
uno::Reference< XInputStream > SAL_CALL Clob::getCharacterStream()
diff --git a/connectivity/source/drivers/firebird/DatabaseMetaData.cxx b/connectivity/source/drivers/firebird/DatabaseMetaData.cxx
index fde8809..eff8097 100644
--- a/connectivity/source/drivers/firebird/DatabaseMetaData.cxx
+++ b/connectivity/source/drivers/firebird/DatabaseMetaData.cxx
@@ -1417,7 +1417,7 @@ uno::Reference< XResultSet > SAL_CALL ODatabaseMetaData::getTables(
uno::Reference< XClob > xClob = xRow->getClob(4);
if (xClob.is())
{
aCurrentRow[5] = new ORowSetValueDecorator(xClob->getSubString(0, xClob->length()));
aCurrentRow[5] = new ORowSetValueDecorator(xClob->getSubString(1, xClob->length()));
}
}
diff --git a/connectivity/source/drivers/firebird/ResultSet.cxx b/connectivity/source/drivers/firebird/ResultSet.cxx
index 17e87cf8..892e510 100644
--- a/connectivity/source/drivers/firebird/ResultSet.cxx
+++ b/connectivity/source/drivers/firebird/ResultSet.cxx
@@ -611,7 +611,7 @@ OUString OResultSet::retrieveValue(const sal_Int32 nColumnIndex, const ISC_SHORT
else if(aSqlType == SQL_BLOB && aSqlSubType == static_cast<short>(BlobSubtype::Clob) )
{
uno::Reference<XClob> xClob = getClob(nColumnIndex);
return xClob->getSubString( 0, xClob->length() );
return xClob->getSubString( 1, xClob->length() );
}
else
{
diff --git a/offapi/com/sun/star/sdbc/XClob.idl b/offapi/com/sun/star/sdbc/XClob.idl
index bc0d2bc..045dfc9 100644
--- a/offapi/com/sun/star/sdbc/XClob.idl
+++ b/offapi/com/sun/star/sdbc/XClob.idl
@@ -117,7 +117,7 @@ published interface XClob: com::sun::star::uno::XInterface
consecutive characters.
</p>
@param pos
the starting position
the starting position, 1-based
@param length
the length of the substring
@returns
