dubious code in dbaccess/source/core/api/KeySet.cxx?

[Lionel Elie Mamane]

On Wed, Oct 02, 2013 at 06:28:39PM +0200, Stephan Bergmann wrote:
> OKeySet::deleteRows in dbaccess/source/core/api/KeySet.cxx contains the loop

>>    for(;pBegin != pEnd;++pBegin)
>>    {
>>        aSql.append(sCon + aOr);
>>    }

> that appends the exact same text to aSql (pEnd-pBegin) number of
> times, which---at least to an SQL-ignoramus---looks dubious.  (On
> the other hand, the code appears to be effectively like that since
> the beginning.)

It is fine. Thanks for bringing it up, though. This function
(OKeySet::deleteRows) deletes all rows in its first argument.

sCon contains the PARAMETRISED condition to select (locate) just one
row, e.g.:

 ( columnName1 = ? AND columnName2 = ? AND columnName3 = ? )

The "?" are parameter placeholders that are filled in later. To locate
one row, they need to be replaced by actual values.

For the sake of the example, assume we need to delete four
rows. Thus, our SQL statement needs to look like:

 DELETE FROM tableName WHERE
 ( columnName1 = ? AND columnName2 = ? AND columnName3 = ? )
 OR
 ( columnName1 = ? AND columnName2 = ? AND columnName3 = ? )
 OR
 ( columnName1 = ? AND columnName2 = ? AND columnName3 = ? )
 OR
 ( columnName1 = ? AND columnName2 = ? AND columnName3 = ? )


Which is correctly the result of the loop you quote.


Continuing the function, there is then a loop:

    for(;pBegin != pEnd;++pBegin)
    {
             for(sal_uInt16 j = 0;aKeyIter != aKeyEnd;++aKeyIter,++j,++aPosIter)
            {
                setParameter(i++,xParameter,*aKeyIter,aPosIter->second.nType,aPosIter->second.nScale);
            }
    }


Which for each row (first for loop) fills in the parameters for each
key column (that's the second for loop) of that row.


I've added some comments to the code.


If you see any other code that looks dubious, feel free to email me
again.

-- 
Lionel