Hi Terrence,
Ocke would be better to answer (most of) your questions, since a) I'm no
active Base developer anymore :) and b) the ODBC driver is his domain.
But since he's on vacation currently, let me jump in.
> (*) I am not satisfied with my analysis of the impact of the change.
> For me to make any progress on this, I think I shall have to start
> over with {OpenGrok, considering the confusion I got into in my
> first attempt at analysis. Perhaps someone here "just knows" that
> the change looks right or wrong? Of course, having found the bug
> at two places, I have to fear that there is a third place which I
> have not found.
Chances are good there's no third place. First, grok says "fraction" is
used nowhere else, and second, connectivity/source/drivers/odbc(base)
are the only two folders which are that close to the ODBC API that they
use this time.
> (*) "Contributing Patches"
> <http://wiki.services.openoffice.org/wiki/Contributing_Patches>
> assumes a working copy from cvs. I lack that, so submitted a diff
> of from the files that I have. Is this of any use?
Well, as long as a "patch -p... -i <patch_file>" succeedes, any patch
file is of use :)
Your current patch file doesn't allow for this, since the file names
therein are "*.cxx_<some_suffix>", which means manual work (typing the
proper file names) when applying the patch. Possible, but annoying.
If you do not have a VCS, on top of which you do your patches, try
copying the source module which you intend to patch aside (say, cp -a
connectivity connectivity.org) and "diff -ru" the original to your
patched version.
Another problem with your patch is that it is not complete: There is no
DateTimeToTimestamp which takes a TIMESTAMP_STRUCT and delivers a
DateTime, only the other way 'round. So, what you probably want to do is
to introduce a TimestampToDateTime method in the OTools class, which
does the proper calculation.
> (*) Along the way, a couple of things in the cod have raised my
> eyebrows a little bit. Are they candidates for fixes?
>
> - The last case the switch structure ending at
> connectivity/source/drivers/odbcbase/OTools.cxx line 394 lacks
> a break statement.
Worth fixing. Just pushed that fix to CWS dba33f.
> - Line 248 of the same file assigns a literal constant to
> _nColumnSize. I think it would be better to use
> SQL_TIMESTAMP_LEN defined at unixODBC/inc/sql.h line 55.
Looks less than optimal. The same probably holds for the other two cases
above the one in question, right? However, I'd like to hear Ocke's
opinion on this before tampering with it.
> Where did the multiplcation by 1000 come from? And is there some
> context in which it is the right thing to do? Which leads to further
> questions ...
>
> Such specifications of ODBC that I have found on the net agree that
> ODBC expresses "fraction" as a number of nanoseconds. But these
> specifications all apply to ODBC 3 or 3.5 or 3.51. Did older version
> of ODBC do it differently? Does some particular (incorrect) ODBC
> driver do it differently? Does anybody care?
>
> Is there a standard set of backends against which to test ODBC
> functionality? Is there even a standard set of test of ODBC
> functionality?
For the standard set of tests - I would delegate this to our QA folks.
In general, I assume there might be backends where the 1000 makes sense,
otherwise the issue would probably have popped up much earlier. So, to
be on the safest possible side, one would have to introduce a setting to
the driver which controls how the DateTime->TIMESTAMP_STRUCT conversion
is done (we have lots of such settings, to care for different backend
behaviors. Not nice at all, but necessary, often enough).
I *think* this might be overkill, assuming that date/time fields
continue to work after the patch, with some set of backends (the usual
MS suspects come to my mind, plus MySQL, and the like). However, I again
would like to hear Ocke's opinion on that.
Thanks & Ciao
Frank
--
- Frank Schönheit, Software Engineer frank.schoenh...@sun.com -
- Sun Microsystems http://www.sun.com/staroffice -
- OpenOffice.org Impress http://graphics.openoffice.org -
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@dba.openoffice.org
For additional commands, e-mail: dev-h...@dba.openoffice.org
