exasol / pyexasol

Exasol Python driver with low overhead, fast HTTP transport and compression
MIT License
72 stars 39 forks source link

✨ Make websocket types of dbapi2 compatibilty shim PEP-249 compliant #146

Open JoepvandenHoven-Bluemine opened 10 months ago

JoepvandenHoven-Bluemine commented 10 months ago

Summary

In the current implementation the dbapi2 types for the websocket driver are not 100% compliant.

Details

PEP-249 requires that each type_code matches one of the available dbapi2 types. This is not currently the case for BOOL, CHAR and GEOMETRY.

The function Binary(string) is also missing, though the best it could do is throw a NotImplemented exception.

Background & Context

If possible it would be best to comply with the standard as much a possible. Obviously this is difficult for BOOL and GEOMETRY which don't have a particularly good fit right now.

At the very least CHAR should be added, it is mentioned in PEP-249 as an example of a STRING type.

Adding a Binary(string) function even one that raises an exception, can't really hurt. It would prevent issues when someone imports functions that the PEP-249 standard claims should exist.

Not sure what is best for BOOL, perhaps pretend it's a number? I think int(bool) is least likely to cause issues. Converting to string may cause confusion if someone tries to use it in an if statement (since all non-empty strings are truthy).

For GEOMETRY I'd say it's string based, querying one from Exasol returns a string and inserting a string causes it to be converted to a GEOMETRY type.

References

https://peps.python.org/pep-0249/#type-objects-and-constructors

Nicoretti commented 10 months ago

Hi @JoepvandenHoven-Bluemine ,

Thank you for creating this issue (your points are valid). To align our expectations and understanding, I will try to clarify our current position and also try to better understand our point of view.

Currently, the DBAPI2 websocket driver implementation is specifically designed to provide a websocket-based SQLAlchemy dialect. Its primary purpose, up to this point, has not been to act as a full-fledged DBAPI2 driver for external use. With this in mind:

We highly value your input on this topic, as it will guide us in assessing user interest in the development of a standalone websocket DBAPI2 compliant driver project.

Furthermore, to ensure we are completely aligned, the DBAPI2 abstraction under discussion is this one.

Best, Nico

JoepvandenHoven-Bluemine commented 10 months ago

Hi @Nicoretti,

What I was doing was using sqlalchemy to reflect the results of some query. To avoid having to completely reinvent the wheel for every single dialect I used the 'dbapi' property of the dialect to write a generic method that gives reasonable results if the connection is (mostly) PEP-249 compliant.

This always requires a bit of custom logic to get perfect, and most implementation seem to take the PEP-249 standard with a huge grain of salt when it comes to the type support. Anyway I noticed that exasol fell through the generic method because in this case 'TypeCode.Bool' didn't match any of the available types.

This may have been a bit of a strange use-case, then again I can't think of a different way to utilise the type_code part of the PEP-249 standard. Seems to me that it was written precisely to give a 'good enough' generic way of reflecting the column types of a query. So I hope this helps clarify how PEP-249 compliance might be useful.

Cheers, Joep

Nicoretti commented 10 months ago

@JoepvandenHoven-Bluemine, thank you for your valuable feedback and input. I'll ensure the information is forwarded to PM. However, I'm unable to guarantee if or when it will be incorporated into our schedule. If you're an active customer and this issue is critical for your operations, I recommend utilizing our official support channels for more immediate assistance.

On a related note, while reaching out to colleagues, I learned about another DBAPI2-compliant websocket-based driver available here ( :information_source: Not sure in which state it is in though).

JoepvandenHoven-Bluemine commented 9 months ago

If it helps I can make a pull request with the changes I proposed when I have some time. It's not something mission critical because it's easy enough to work around, but if we can avoid a workaround by making the driver more DBAPI2 compliant I'd say that's better.

Nicoretti commented 9 months ago

@JoepvandenHoven-Bluemine sure that definitely would save some time. So by any means, if you are willing and find some time for it, send us a PR.

Nicoretti commented 4 months ago

Hi @JoepvandenHoven-Bluemine,

Just a quick update: the shim for dbapi2 compliance will be transferred to the pyexasol project, therefore I have moved the ticket here. (see also PR #145)

Best, Nico