https://bugs.documentfoundation.org/show_bug.cgi?id=90520

--- Comment #24 from Lionel Elie Mamane <[email protected]> ---
I'd be happy to review/approve a well-tested patch along the lines of the
discussion below. Put me as reviewer on gerrit, and feel free to ping me every
week by email if I don't respond.

(In reply to grongo2 from comment #3)
> OTools::getStringValue tries to read a column with more than 2048 bytes by
> iterating over SQLGetData, but uses only pcbValue to detect when stopping,
> instead of looking also at the function return value.

Good point. Looking also at the function return value would make the code more
robust and would be a welcome improvement.

> Further, it iterates even with fixed-length column, while ODBC allows
> iterating only over variable lenght columns (eg VARCHAR).

Not 100% sure what you mean by "fixed-length column", but I note that
https://learn.microsoft.com/en-us/sql/odbc/reference/syntax/sqlgetdata-function#retrieving-variable-length-data-in-parts
explicitly allows it for SQL_CHAR, SQL_WCHAR and SQL_BINARY.

In the context of this bug, you say the data being read is SQL_CHAR so the
iteration seems legit.

However, there does seem to be a codepath where indeed LibreOffice will try to
do that iteration on SQL_DECIMAL and SQL_NUMERIC: in OResultSet.cxx, method
OResultSet::fillColumn

Plus anytime any code (even user-written LibreOffice Basic, Python or other API
binding) calls getString on a non-string column but only on a
DatabaseMetadataResultSet, not on a regular ResultSet.

> So a workaround would be resetting pcbValue to zero just before calling
> SQLGetData.

While I'm generally wary of driver-specific work-arounds, something like that
could indeed be seen as a general robustification of the code, so I could be
tempted to approve it. One could discuss whether one should set it to some
other value than zero, such as e.g. a constant not allowed to be written there
by the driver (such as a constant chosen to be different than all valid "Length
and Indicator Values", or one valid only for SQLPutData and not SQLGetData).

> A better fix should be check SQLGetData return value and act accordingly,
> but perhaps LO does things this way as a workaround to other driver bugs ?

In the absence of a comment in the code that specifically says so, any such
putative driver bug is lost to time immemorial, and could have been fixed since
then. I wouldn't let that keep us from moving forward with that better fix.

(In reply to grongo2 from comment #22)
> To tell the truth, the handling of this bug is disheartening: eight year ago
> I reported it, carrying out a detailed analysis and suggesting a fix.
> After that, no action, only a "please try with the latest release", even if
> the source code remains unchanged.

> Anyway, the bug is still present.

You mean in the same eight years, Oracle has (also) not fixed their bug?

> Reproducing is simple:

Well, no. It requires an Oracle database, which is not quite an "apt-get
install" away, and I expect requires agreeing to a bizarre funky anti-social
agreement forbidding use for any purpose on any machine one wants and/or
sharing it with one's neighbour and/or improving it and/or sharing one's
improvements? The first download page I could find
https://www.oracle.com/database/technologies/xe-downloads.html also doesn't
have any download for any source code (so I cannot compile/install it), nor for
binaries for my main work machine (GNU/Linux on ppc64le).

-- 
You are receiving this mail because:
You are the assignee for the bug.

Reply via email to