Re: Removing support for < 7.4 servers

On 03/12/2014 10:10 AM, Michael Paquier wrote:
> On Fri, Mar 7, 2014 at 10:25 PM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
>> Yeah, around that I time I guess. I believe the packagers would prefer for
>> the driver release some weeks before the server release, so that they have
>> time to package it and release together with the server.
>
> OK, I have created a fork on github here
> (https://github.com/michaelpq/psqlodbc) with all the cleanup related
> code in a dedicated branch called cleanup-9.4.

Ahh, that's the way I like my patches:

  49 files changed, 273 insertions(+), 2571 deletions(-)

:-)

> I might rebase this
> branch based on latest HEAD of odbc from time to time to avoid any
> conflicts when applying the cleanup patches.

Yeah, please do. I find it easier to work with branches that are rebased
over current master, rather than merged.

> For the time being I have done the following:
> - Removal of protocol older than 7.4 (including ksqo, schema_support).
> Thanks Heikki for the patch. It would have been actually hard to
> remove the ksqo things, I don't know much the history of Postgres <
> 7.4.

Now that we don't need the Protocol setting for the actual protocol
version, I think we should invent a new setting for the "rollback"
behavior. It's currently stored as a prefix of the protocol, which IMHO
never made much sense, but now even less. Accept it in the Protocol
setting for backwards-compatibility, but document the new field as the
primary setting and use that in the examples and GUI.

> - Changed PRECISION field to use COLUMN_SIZE when getting the
> information of a type (patch attached as it is simple). I have let
> COLUMN_PRECISION as it is as COLUMNS_COLUMN_SIZE is kind of strange,
> only changed the type name reported.

I didn't quite understand this, but in general, I think we should
consistently use the new attribute names.

Also note that the driver manager actually does some mapping of
deprecated ODBC function calls to the new functions, so that the driver
only needs to provide the ODBC 3.X functions. For example, if the driver
doesn't provide the deprecated SQLGetStmtAttr function, and the
application calls it, the driver manager will call SQLGetStmtOption
instead (or just handle the call internally without calling the driver,
depending on the attribute). So once we drop support for compiling the
driver in ODBC 2.X mode, we could completely drop many of the interface
functions. before we actually do that, though, it would be good to have
regression test coverage of the deprecated functions with different
attributes, to make sure they get mapped correctly and still work if
used in an application.

> This makes me think that we should add some regression tests for
> SQLGetTypeInfo, SQLColumns, SQLSpecialColumns & friends. Thoughts?

Yeah, absolutely. Would be good to run a code coverage report (e.g lcov)
to see which SQL interface functions are not currently covered by the
regression tests.

> I noticed as well the following things in the code that may need some
> more thinking:
> - Reorganization of the code organized by versions (I am thinking that
> grouping those APIs makes sense if we drop the ODBCVER ifdef blocks
> and protocols older than 7.4). Following files are impacted:
> odbcapi*.c info*.c, pgapi*.c

Agreed.

> - Some code has been added for support of SQL_INTERVAL in commit
> 6e45dcb, but it is disabled with flag PG_INTERVAL_AS_SQL_INTERVAL.
> Should we do something about that? And what are the applications that
> faced problems with that?

No idea.

> - I am not sure that odbc.sql and odbc-drop.sql are of any use. Should
> we remove them?

Probably. The script still works, but I'm not sure how useful it is.

> - Is inouealc.c in the code tree for conveniency?

I believe the purpose is to find memory leaks. I'm not sure how useful
that is nowadays; it's probably easier to just run the regression tests
under valgrind.

- Heikki