Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Oracle #1

Closed
wants to merge 17 commits into from
Closed

Add support for Oracle #1

wants to merge 17 commits into from

Conversation

cdonnellytx
Copy link

Added support for Oracle to Dapper.Database -- barely.

The problem

While this addresses a number of problems that we were having with our Dapper fork (vauto/Dapper#1) by virtue of there being more overrides (and fixing the caching bug in Dapper.Contrib as well!), there's still a number of problems caused by Oracle itself:

Guid

Oracle's drivers (unmanaged, managed full framework, managed .NET Core) all reject DbType.Guid as an input type.

Specifying an alternate type is problematic for two main reasons:

  1. Dapper's IDbConnection.Execute(...) extension methods resolve DbType via the typeMap and not type handlers, so the only way to change it for Oracle is to change it globally within SqlMapper. While fortunately this can be done via the SqlMapper.AddTypeMap method, this change effectively prevents any other databases from working properly, which means as a general-purpose solution for users using multiple database types in the same process, it's a non-starter.
  2. The type to replace it with is not as obvious as I first thought.
    • We chose the route of mapping Guid to byte[] via ToByteArray() and Guid(byte[]) respectively. This results in the bytes being "flipped" due to Microsoft choosing to make Guid little-endian. However, per this StackOverflow article, big-endian storage may be preferred. In fact, the tests in Dapper.Database convert Guid to Binary in big-endian format.
    • Finally, others may prefer storing it as a String, although this is obviously less efficient from a storage perspective (36 bytes in UTF-8, plus string length overhead if using a varchar column instead of char).

Bind variables

Despite most databases not supporting the SQL Server bind variable format (@foo), most .NET adapters seem to have gone out of their way to work with bind variables named this way. All the tests in the Dapper.Database project show this. Even Npgsql (the PostgreSQL driver) does this.

However, there is one major, glaring exception -- ODP.NET. No implementation of the Oracle driver makes any attempt to pre-parse SQL, so the bind variables are passed all the way down to Oracle as specified. This results in unclear errors like ORA-00936: missing expression, ORA-00923: FROM keyword not found where expected, and many more.

This means most of the tests in Dapper.Database fail simply on account of using the at-sign to denote bind variables, despite working with everything else.

The solution

Unfortunately there is no good solution that will please everyone. This appears to be what @mgravell was referring to in DapperLib/Dapper#637:

Reverted; sorry, closing this down since we can't support it natively in the lib due to the incompatibility between backends

and again in DapperLib/Dapper#633:

The RAW(16) gets very messy because of the problem of endianness; there are
two choices in the wild, and different providers have gone different ways -
there is no choice we can make that will work reliably, and we don't have
the info available to choose between mixed-endian vs big-endian. Supporting
something string-based sounds achievable, but sub-optimal - very vexingly.

So there really isn't a solution that will work for everyone. The basis of all the solutions is very similar, but simply won't work in a simple, turnkey fashion.

I am going to close out this branch and simply make an in-house library that implements Oracle how we (vAuto) use it by overriding the adapter resolution by hooking into SqlMapperExtensions.GetDatabaseType. If there is a way to release this as a globally available library, it will require a nontrivial amount of Oracle-specific code, which is probably not what the maintainers of Dapper or Dapper.Database want.

@cdonnellytx
Copy link
Author

cdonnellytx commented Aug 22, 2018

I just realized the SqlMapperExtensions.GetDatabaseType returns a string, not an ISqlAdapter, so I still need to submit a minimal PR upstream.

Also adding test suite SQL file in Oracle syntax.
* Now overriding Exists to use SELECT 1 FROM DUAL because Oracle needs a FROM.
* Adding a wrapper for OracleCommand (and by extension, OracleConnection) that parses out '@' binds and replaces them with ':' binds ODP.NET can digest.
The ODP.NET driver apparently half-supports Guids -- for EF only.
When it does, it always does little-endian...
Oracle supports using bind variables for its pagination offset/size,
so I am modifying Dapper.Database to support this.

There are probably other database engines that support it, but I don't know which ones, so I am not having the others bind their parameters...
Apparently they fixed something in 12.2 to handle the bind variable names I've chosen...
I hate to introduce ReSharper stuff into someone else's project, but in this case the tests flat-out fail if run in parallel,
if only because you have multiple runtimes initializing the same databases with the same DDL.
Oracle does it that way, and from what I can tell, all the other drivers do, too.
@cdonnellytx cdonnellytx deleted the oracle branch August 29, 2018 17:03
cdonnellytx pushed a commit that referenced this pull request Sep 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant