On Wed, 2013-09-11 at 11:20 +0200, Lionel Elie Mamane wrote:
> On Tue, Sep 10, 2013 at 09:56:50PM +0100, Andrzej Hunt wrote:
>
> > The definition of XRowLocate::findColumn, and its implementations, all
> > seem to be slightly confused.
>
> > XRowLocate doesn't specify what should happen what happens if
> > findColumn is called for an invalid column name.
>
> You mean XColumnLocate, not XRowLocate.
Ooops ;)
>
> > In the case of JDBC drivers: the JDBC specification states that an
> > SQLException should be thrown in this case. (...)
>
> > Is it OK if I rewrite the specification of findColumn to mirror that
> > of the JDBC specification,
>
> Yes. SDB(C)(X) is (from my understanding) a C++-ized copy of (an older
> version of) JDBC anyway, so just align.
>
> > perhaps with a note that some drivers may still return invalid
> > values instead?
>
> Something like:
>
> Version of the specification before LibreOffice 4.2 left unspecified
> what happened when passed an invalid column name. As a result, some
> drivers written against older specification may return a special
> invalid numeric value, such as a negative value, zero or a number
> greater than the number of columns.
>
> > (Of course I'd also upgrade all the drivers to reflect the new
> > specification
Done!
>
> Great, thanks. If you could also review what callers of findColumn
> within LibreOffice do, that would be even greater.
Most of the callers reuse the value in other methods (mostly
getXXX(...)) which will themselves have to throw an SQLException (column
index out of range or similar) -- so essentially all that's happening is
that the exception is now being thrown earlier.
In one case the value is used to directly access members of an array (in
connectivity/source/drivers/file/FStatement.cxx) so throwing an
Exception will make that safer (especially since file/FResultSet
returned number_of_columns+1 for findColumn).
>
> > -- so this note would only apply to any external drivers -- are
> > there any?)
>
> I don't know of any truly external; in some way, the MySQL one counts
> as "external" since people take binaries and try to use them with
> multiple OpenOffice.org, Apache OpenOffice and LibreOffice versions.
>
Good to know. In any case the code in LO which deals with external
drivers should be able to handle invalid values and/or exceptions so
this doesn't seem worth worrying about.
One thing I've not yet done is update
ucbhelper/source/provider/propertyvalueset.cxx -- ucbhelper isn't linked
against connectivity at the moment so can't make use of the same
dbexception helper stuff -- I could make it throw the SQL Exception
directly as in mysqlc, or link it against connectivity? No idea which is
preferred.
Cheers,
Andrzej
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice
