[Oledb-dev] A major rewrite of the Postgres OLE DB Provider.

[Jeremy Lea]

Hi,

On Sat, Dec 16, 2006 at 10:39:39AM +0200, Shachar Shemesh wrote:
> Jeremy Lea wrote:
> >  - Runs multiple queries (using PQexec) if you pass a command with no
> >    parameters and IID_NULL for the return record set.
> >   
> You should be aware that there is a security advantage to NOT being able
> to do that, in fact.
> 
> On the plus side, it does allow servers older than 7.4 to work with the
> provider, under certain cases. On the don't-care side, so what?

The security differences are really minor...  If you're letting user's
inject SQL, then you better be handling it., no matter if they can run
multiple statements.

Earlier versions are not going to work, because you run a query straight
after connection to get the pg_types, which uses binary returns.  They
only way is to go to having the CopyData functions check binary vs text
and do the right thing in both cases.  And to turn off lots of
functionality if they provider is < 7.4.
 
> >  - Proper type handling.
> >   
> I'd love an explanation in what, you think, was missing.

The current design doesn't do DBTYPE to pg_type oid mapping properly,
doesn't handle a number of corner cases as required by the OLE DB docs
(DBTYPE_NUMERIC colinfo comes to mind).  The code is really messy in
it's handling of bytea's BYREF stuff, and the "_text" type is completely
wrong.  It doesn't pass out a SAFEARRAY.  There were a lot more things,
besides the weird C callback design., but I forgotten a lot of them -
when I started looking at why my parameter passing wasn't working, I
quickly realised that the code would have to be redone from scratch... 

> >  - Much cleaner code (IMHO), no more STL or exceptions.
> >   
> A matter of taste. I think this design breaks every OOD instruction out
> there (read - no switch-case on child classes).

Better than throwing exceptions in functions which aren't declared to
throw...  Dropping the STL in favour of ATL classes reduced the code
size by about half and having fewer classes really makes it easier to
understand.

> >  - Parsing of parameters on stored procedures.  This was a hack,
> Yes, but the hack is in the OLE-DB's specs. Quite a number of programs
> don't work without it.
> >  which
> >    was being done at the wrong place in the code.
> >   
> I'd love it if you elaborated on that.

The spec says it must be done in GetParameterInfo, you're also doing it
in Execute.  Once the user has set up an Accessor, you can't play with
it.

> >  - Support for some types (_text, utinyint, varcharci)
> >  - TIMESTAMP WITH TIMEZONE is reported in GMT not local, since there is
> >    no OLE DB type for this (so you can't go backwards).
> >   
> If you cannot support data-types that are implemented in PG, and have a
> reasonable representation in OLE DB, what makes this type support "proper"?
> 
> Also, if I understand my very brief passing over the code correctly, if
> I have two coloumns, one of type timestamp and one of type timestamptz,
> and I read both in, and then send a parametrized update for both to the
> same values I read, one of them will change.

No, that's what the current code does - it converts from GMT to local
time and doesn't convert back.  Well sort of - I don't think it supports
writing timestamps? (I can't see at the moment how this works - I
presume ADO just sends these as text stings?  I'd have to instrument the
patched old provider to figure out...)

As for the future of the project...  I needed code to work.  It now
works.  The provider, unpatched does not for what I need. 

Regards,
  -Jeremy