connectivity::ORowSetValue cleanup and behaviour

[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_Bool        m_bBool;
            sal_Int8        m_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_Bool            m_bNull     : 1;    // value is null
        sal_Bool            m_bBound    : 1;    // is bound
        sal_Bool            m_bModified : 1;    // value was changed
        sal_Bool            m_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_Bool        m_bBool;
            sal_Int8        m_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_Bool            m_bNull     : 1;    // value is null
        sal_Bool            m_bBound    : 1;    // is bound
        sal_Bool            m_bModified : 1;    // value was changed
        sal_Bool            m_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_Bool        m_bBool;
            sal_Int8        m_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_Bool            m_bNull     : 1;    // value is null
        sal_Bool            m_bBound    : 1;    // is bound
        sal_Bool            m_bModified : 1;    // value was changed
        sal_Bool            m_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