[Oledb-dev] FillDataOffsets, ulColumnSize and bytea columns

[Victor Snezhko]

Shachar Shemesh <psql at shemesh.biz> writes:

>>I am working on bytea support, and have some news.
>>PgRS.h:FillDataOffsets() line filling ulColumnSize was not quite
>>misplaced as we thought earlier
>>
> Surely you mean "as *I* thought earlier...." :-)

OK, me :)

>>switch (pColCur->wType)
>>{
>>case DBTYPE_STR:
>>	break;
>>case DBTYPE_WSTR:
>>case DBTYPE_BSTR:
>>	break;
>>default:
>>	cbCol = pColCur->ulColumnSize;
>>	break;
>>DataConvert tries to allocate that much
>>memory and obviously returns E_OUTOFMEMORY (BTW, I believe that
>>problem reports in our bug tracking database that contain complaints
>>about out of memory errors are caused by this behaviour).
>>  
> Another excellent catch! Well done!

I just need this stuff to work :)

>>+    m_sizes_filled=true;
>>  
>>
> Is this just a performance thing?

No, GetColumnInfo may be called before CPgVirtualArray::operator[]. In
this case absense of this check will cause filling ulColumnSize with
garbage. That was the cause I asked to remove ulColumnSize assignment
two weeks earlier.

>>+        m_sizes_filled=false;
>>  
>>
> You forgot to initialize it in CPgVirtualArray's constructor.

OK, patch updated, see attachment.

> This all brings us to:
>
>>+                if (rpInfo[i].wType==(DBTYPE_BYTES|DBTYPE_BYREF))
>>  
>>
> This is the line I feel most uncomfertable with. It is, simply and
> plainly, wrong. The reason behind the entire type system was to
> eliminate special casing specific types. This makes adding new types
> difficult, and is just asking for bugs.
>
>>+                    rpInfo[i].ulColumnSize=m_sizes[i];
>>  
>>
> Is there a reason this line should not apply to all data types? Is there
> any data type for which this line is wrong? I fully realize that ATL
> will probably ignore it for WSTR and STR, but is that a problem?

But changing column info on the fly is a horrible hack! We are forced
to do this because we have no other means to pass column length to
ATL. Look at the definition of ulColumnSize in MSDN [1]: 

====
ulColumnSize - The maximum possible length of a value in the
column ... For columns that use a variable-length data type, this is
one of the following: 

    * The maximum length of the column in characters, for DBTYPE_STR
and DBTYPE_WSTR, or in bytes, for DBTYPE_BYTES and DBTYPE_VARNUMERIC,
if one is defined. For example, a CHAR(5) column in an SQL table has a
maximum length of 5.
    * The maximum length of the data type in characters (for
DBTYPE_STR and DBTYPE_WSTR) or in bytes (for DBTYPE_BYTES and
DBTYPE_VARNUMERIC) if the column does not have a defined length. 
    * ~0 (bitwise, the value is not 0; all bits are set to 1) if
neither the column nor the data type has a defined maximum length.

For data types that do not have a length, this is set to ~0.
====

We don't have maximum length for bytea columns, so ulColumnSize should
be ~0. If consumer tries to access ulColumnSize and see the length of
the last value transmitted (instead of ~0), it may become
confused. Like, e.g., erroneously deciding that column has maximum
length and preallocating memory for values.

So, by inserting DBTYPE_ARRAY check I tried to limit consequences of
the hack.

We should find a way to change ulColumnSize back to ~0 at the end
of IRowsetImpl::GetData. I don't see the way to do it so far. Maybe
just because it's late here now. :)

> Does m_sizes[i] contain the right value for, e.g., "DBTYPE_DATE"? If
> not, why?

m_sizes contain the right values for all types that have fixed length,
and these values are equal to corresponding to ulColumnSize's, so
there is no need to assign.

[1] http://msdn.microsoft.com/library/en-us/oledb/htm/oledbicolumnsinfo__getcolumninfo.asp  

-- 
WBR, Victor V. Snezhko
E-mail: snezhko at indorsoft.ru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: oledb-bytea-read.diff
Type: text/x-patch
Size: 1889 bytes
Desc: not available
Url : http://pgfoundry.org/pipermail/oledb-devel/attachments/20060505/5da35beb/attachment.bin