On Fri, 22 Mar 2013 14:40:36 +0000 Mateusz Loskot <[email protected]> wrote:
ML> > ML> If you say we can release without the tests added, just with the
ML> > ML> fixes, let's do so.
ML> >
ML> > The "endOfRowSet" fix is perfectly safe to apply, it can't break anything
ML> > because it changes nothing if execute() is called only once and if it's
ML> > called twice or more times, currently it doesn't work at all so anything
ML> > else can only be an improvement.
ML>
ML> I see.
This was indeed correct.
ML> > The "s/unsigned/int/" fix for SQL_LONG is slightly more risky but, still,
ML> > right now you simply get completely wrong values when reading from the
ML> > database (something like 4953.956 when the database contains -0.0013), so
ML> > IMO it would still be better to apply it rather than not do it because
ML> > silently reading wrong data is one of the worst things that can happen in
ML> > a database library.
The above was correct too, in the "more risky" part. It turned out that
this fix did break Firebird tests[1] and that "unsigned" was used here to
work around the incorrect result when we casted 4000000000 (which is
greater than INT_MAX on 32 bit systems) to int and then to unsigned long
which could be 64 bits, yielding a wrong value.
However I don't believe the original fix by Viacheslav (see 4c3cb629)
was the right way to do it, it only hid the problem but didn't solve it.
Using SQL_LONG for "unsigned long" simply can't work on LP64 systems
because SQL_LONG is just not big enough. So the real fix is to use
SQL_INT64 here and this is as simple as creating the column of right type
in the test database and this is what my fix[2] does. And with that fix
applied, the original commit fixing reading doubles still works and doesn't
break the tests any more.
ML> Indeed, we should aim to straighten the numeric types support as much
ML> as we can, within the extent of what we support now, I mean, without
ML> introducing new types yet. So, I'm glad you've got fix for that issue.
Yes, I'm glad too because at one moment I was ready to totally despair...
But finally it's not too bad and, provided that you use the correct column
types, things do seem to work now.
Still, the SQL to C++ types mapping is a huge mess and it clearly should
be the main focus of SOCI 4.0 release.
ML> > ML> Are you OK with releasing your fixes but without tests in 3.2.0?
ML> >
ML> > I prefer to commit them without tests to not committing them at all but
ML> > I'll try to add the tests too. Just please let me know until when do I
have
ML> > to do this.
ML>
ML> Let's say Sunday midnight :)
This means we still have 36 hours for testing, so I'd like to ask
everybody potentially affected by these issues to test the changes at
https://github.com/SOCI/soci/pull/108
Please let me know if you see any unexpected results or have any remarks
or questions. TIA!
VZ
[1] Which I finally managed to run with embedded Firebird here, although
only using a dirty hack of creating a link to libfbembed.so called
libfbclient.so and putting it in LD_LIBRARY_PATH, i.e. in front of the
real libfbclient.so. I'd still like to have a proper fix at CMake level
but at least now I can run the tests which is already good.
[2] https://github.com/vadz/soci/commit/700fb7df7ae4dfd5ea04082a239340c49045c116
pgpU51LvTQl60.pgp
Description: PGP signature
------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_mar
_______________________________________________ soci-users mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/soci-users
