Re: connectivity::ORowSetValue cleanup and behaviour

2012-02-15 Thread Lionel Elie Mamane
On Wed, Feb 15, 2012 at 01:55:34PM +0100, Lionel Elie Mamane wrote:

> 2) Signed integers and unsigned integers never compare equal. But an
>unsigned integer compares equal to the corresponding float...

>Errr... They obviously should?

I misread the code... Need coffee. integers never compare equal to
floats, they don't even compare equal to the corresponding integer of
different size? E.g. 5 as a 16-bit integer is not equal to 5 as a
32-bit integer.

I'm suddenly more shy about changing that... The intended semantics
seem different, but OTOH,
frm::OListBoxModel::translateDbColumnToControlValue clearly expects
wider semantics (more things are equal). Dunno about other callers.

Mainly, I see no reason why floats of different subtypes should
compare equal, but not integers.

Thinking about it.

-- 
Lionel
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice


connectivity::ORowSetValue cleanup and behaviour

2012-02-15 Thread Lionel Elie Mamane
Hi,

The connectivity contains a ORowSetValue class, to hold a database
value, along with its type. "Obviously" this class is used all over
the place in our database-related code.

I'm itching to change it in non-trivial ways, and I've hit some
"WTFs", so I thought I'd check if someone knows a good reason for some
of baroque choices made (which would mean my naive "clean" way might
introduce other bugs).

1) Variable-length strings and fixed-length strings never compare
   equal. That's what started all this, because of a user-visible bug
   consequence.

   Errr... They obviously should?

2) Signed integers and unsigned integers never compare equal. But an
   unsigned integer compares equal to the corresponding float...

   Errr... They obviously should?

3) Unsigned integers are stored weirdly, that is as the next-bigger
   size signed integer. Does anybody have *any* clue as to why this
   could possibly be a good idea?

   Oh, and unsigned 64 bit integer? Just stored as a *string*,
   because, you know, we don't have a bigger signed integer at hand.

The data part of the class looks like:

class OOO_DLLPUBLIC_DBTOOLS ORowSetValue
{
union
{
sal_Boolm_bBool;
sal_Int8m_nInt8;
sal_Int16   m_nInt16;
sal_Int32   m_nInt32;
rtl_uString*m_pString;

void*   m_pValue;   // can contains double, etc
} m_aValue;

sal_Int32   m_eTypeKind;// the database type
sal_Boolm_bNull : 1;// value is null
sal_Boolm_bBound: 1;// is bound
sal_Boolm_bModified : 1;// value was changed
sal_Boolm_bSigned   : 1;// value is signed
   }

m_eTypeKind is a tag that says what type the value is: fixed-length
string, variable-length string, 8/16/32/64 bit (un)signed integer,
date, time, timestamp, decimal number, boolean,  See
offapi/com/sun/star/sdbc/DataType.hdl


My first impulse was to reinterpret_cast<>() unsigned integers into
the *same* size signed integer and back, but actually cleaner to do:

class OOO_DLLPUBLIC_DBTOOLS ORowSetValue
{
union
{
sal_Boolm_bBool;
sal_Int8m_nInt8;
sal_Int16   m_nInt16;
sal_Int32   m_nInt32;
sal_uInt8   m_nuInt8;
sal_uInt16  m_nuInt16;
sal_uInt32  m_nuInt32;
rtl_uString*m_pString;

void*   m_pValue;   // can contains double, etc
} m_aValue;

sal_Int32   m_eTypeKind;// the database type
sal_Boolm_bNull : 1;// value is null
sal_Boolm_bBound: 1;// is bound
sal_Boolm_bModified : 1;// value was changed
sal_Boolm_bSigned   : 1;// value is signed
   }

That much shouldn't change the storage size of that class.

I'm tempted to actually go further and just make the class bigger, so
that we don't have to dynamically allocate storage for double, 64 bit
ints, etc, which (to me at least) seems like a bigger overhead than the
increase in memory:

class OOO_DLLPUBLIC_DBTOOLS ORowSetValue
{
union
{
sal_Boolm_bBool;
sal_Int8m_nInt8;
sal_Int16   m_nInt16;
sal_Int32   m_nInt32;
sal_Int64   m_nInt64;
sal_uInt8   m_nuInt8;
sal_uInt16  m_nuInt16;
sal_uInt32  m_nuInt32;
sal_uInt64  m_nuInt64;
rtl_uString*m_pString;
float   m_fFloat;
double  m_fDouble;

void*   m_pValue;   // for complex types
} m_aValue;

sal_Int32   m_eTypeKind;// the database type
sal_Boolm_bNull : 1;// value is null
sal_Boolm_bBound: 1;// is bound
sal_Boolm_bModified : 1;// value was changed
sal_Boolm_bSigned   : 1;// value is signed
   }

And since e.g. ::com::sun::star::util::Date and Time are structs of
three or four 16-bit integers, let's stick them in the union, too.


Any opinions, vetos, insights? Am I misguided about the "minimal
memory amount taken vs dynamic allocation overhead" balance?

-- 
Lionel
___
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice