RepoDB icon indicating copy to clipboard operation
RepoDB copied to clipboard

Request: Add Support for Oracle

Open say25 opened this issue 4 years ago • 22 comments

I've stumbled upon this project and started playing with it in a Postgres scenario, though I would get far more benefit by using it in an Oracle scenario at work. Any opposition to adding support for Oracle? Any hints to anyone adding a new supported DB?

say25 avatar Jun 25 '20 03:06 say25

Certainly, this can be done! :) It would take a little time though. As of now, I am waiting for more users to adopt the library before developing any more new major releases. Though, if there are big community support pertaining to this request, then it will definitely urge us to support this most likely ASAP. This can be grabbed also.

mikependon avatar Jun 25 '20 03:06 mikependon

I would need Oracle support as well for company projects. Oracle db certainly has large footprint in enterprise world. I am working for a Telecom.

robertmircea avatar Jul 25 '20 08:07 robertmircea

I am currently taking a look at what can be done to support Oracle, and the first stumbling block so far has been its quoting/casing rules.

In Oracle, Customer is the same as CUSTOMER, which is the same as "CUSTOMER", but it is definitely not "Customer". Most Oracle databases in the wild use SCREAMING_SNAKE_CASE, so CreatedDateUtc from the "getting started" model should be mapped to CREATED_DATE_UTC instead of "CreatedDateUtc" and vice versa by default, but I would have to modify IDbSetting to include this as a supportable option.

orthoxerox avatar Jan 28 '21 13:01 orthoxerox

No need, it is supported by default by the library. The IDbSetting shoukd only contains the DB provider specific setting like (quote character, etc).

Actually, you had explained almost the same scenario as PosrgreSQL.

It is not the responsibility of IDbSetting to manage how the mapping would work to both tables and columns. It should be via the FluentMapper and Table/Column/Map attributes.

mikependon avatar Jan 28 '21 14:01 mikependon

Another thing I wanted to run by you first is using OracleDbType instead of DbType in OracleConvertFieldResolver. Am I correct that this is not a good idea, since the core library uses only DbType to set command parameters and I can't plug in a replacement for ClientTypeToDbTypeResolver?

orthoxerox avatar Jan 29 '21 09:01 orthoxerox

Finally, what's the lowest version of Oracle you want to support? 11g and lower require some... interesting queries to emulate LIMIT/OFFSET clauses.

orthoxerox avatar Jan 29 '21 11:01 orthoxerox

11 would be good.

Another thing I wanted to run by you first is using OracleDbType instead of DbType in OracleConvertFieldResolver. Am I correct that this is not a good idea, since the core library uses only DbType to set command parameters and I can't plug in a replacement for ClientTypeToDbTypeResolver?

This can be solved by the extra attribute something like MySqlTypeMap for the MySql.

mikependon avatar Jan 29 '21 13:01 mikependon

That's what I wanted to hear. I also found MySqlDbTypeToMySqlStringNameResolver that looked like what I was proposing, but it looks like it isn't used anywhere at all.

orthoxerox avatar Jan 29 '21 13:01 orthoxerox

Are your Oracle extension being written inside RepoDB repository or is it something you would like to manage yourself?

mikependon avatar Jan 29 '21 13:01 mikependon

I would like to release a PR for your repo if you accept contributions.

orthoxerox avatar Jan 29 '21 13:01 orthoxerox

Cool. I do not have the environment ready for Oracle, but yes, I do accept the contributions. We can put you as the owner of the package itself, but we will first do the review everything before pushing it as an official package. Please do let me know if that is ready?

mikependon avatar Jan 29 '21 13:01 mikependon

I should be able to show something next week, if I am not inundated with actual work.

orthoxerox avatar Jan 29 '21 19:01 orthoxerox

@orthoxerox - make sure you follow our coding standards to void thorough line-by-line review. @SimonCropp is strict on this one as well 😸

mikependon avatar Jan 29 '21 20:01 mikependon

Unit tests for 12c and higher are green (but I need to write a few more for the trivial changes I did to the hints placement). Tomorrow I hope to deal with the integration tests and push a WIP PR. For 11g I'll write its own IDbHelper and IStatementBuilder (very different limit/offset implementation, no identity columns) and add a separate Initialize11g method to OracleBootstrap.

orthoxerox avatar Feb 04 '21 15:02 orthoxerox

I am bogged down in the integration tests, no PR today. Oh, Oracle, why do you have to come up with stuff like OracleCommand.BindByName being false by default?

orthoxerox avatar Feb 05 '21 21:02 orthoxerox

All right, I think I need some advice here. In DbConnectionExtension.InsertInternalBase the command is invoked like this:

                result = Converter.ToType<TResult>(command.ExecuteScalar());

                // Get explicitly if needed
                if (Equals(result, default(TResult)) == true && dbSetting.IsMultiStatementExecutable == false)
                {
                    result = Converter.ToType<TResult>(connection.GetDbHelper().GetScopeIdentity(connection, transaction));
                }

This is not how Oracle works. You need to add an output parameter to the command, call it with ExecuteNonQuery and get the value from the parameter. Same with InsertAll: instead of enumerating ExecuteReader you get the values from a bunch of output parameters like :Result, :Result_1 and so on. I cannot override this behavior, since it's in a static class. Should I refactor the core library to extract all the ...Base methods into an injectable dependency?

orthoxerox avatar Feb 06 '21 10:02 orthoxerox

Is there a way for you to multi result generated values instead of using the ouput parameters? I would pre-assume, yes. I am abit afraid to change the logic of the core, such approach has mediated multiple RDBMS (TBH).

FYI, it was an ouput parameter historically.

mikependon avatar Feb 06 '21 12:02 mikependon

Oracle is... special. I checked Oracle docs again, and this is the summary:

  • Oracle has no way to obtain the last generated identity like @@IDENTITY in MS SQL, until 12c it had no concept of identity columns at all
  • insert statements in Oracle aren't queries and can only be called using ExecuteNonQuery.
  • insert statements in Oracle have a returning clause that can bind the fields of the inserted row to host variables (command parameters)
  • to insert N rows with M fields with a single call I can do two things:
    • combine N insert statements into an anonymous begin ... end; block (N output parameters needed to collect generated identities)
    • use ODP.NET-specific array bind parameters: instead of N*M command parameters I can use just M parameters, each one an array of N values (1 output parameter needed to collect an array of generated identitites)

Neither approach works with RepoDB's existing code, as neither insert statement nor anonymous blocks are queries in Oracle.

orthoxerox avatar Feb 06 '21 14:02 orthoxerox

Let me as well do a little research for you. I hope to get back to you early next week.

mikependon avatar Feb 06 '21 21:02 mikependon

@orthoxerox as you mentioned and also through this article, only the ExecuteNonQuery() command can be used to extract the generated identity value from Oracle. What a mess actually 😄

RepoDB on the other hand, the base functionality has been rewrote for the SELECT base rather than OUTPUT parameters (as historically).

I am not an Oracle expert, but there may be one trick here. Would the recommendation below feasible and doable in Oracle?

  • When you do insert, declare a temporary OUTPUT VARIABLE
  • Set this OUTPUT VARIABLE after the insertion from the table
  • Select back the value of this OUTPUT VARIABLE

Pseudo-Code:

-- CREATE
CREATE TABLE TABLE_NAME (...);

--TRIGGER
CREATE OR REPLACE TRIGGER TABLE_NAME
    BEFORE INSERT ON TABLE_NAME FOR EACH ROW
BEGIN
    SELECT TABLE_NAME.NEXTVAL INTO :NEW.ID_COLUMN FROM DUAL;
END;

--INSERT
INSERT INTO TABLE_NAME (OTHER_FIELD) VALUES ("Other Field Value");

-- DECLARATION
DECLARE OUTPUT_VARIABLE TYPE;
SET OUTPUT_VARIABLE = (SELECT my_table_seq.CURRVAL);

--SELECT
SELECT OUTPUT_VARIABLE;

If this is feasible, then, you have to do such kind of things if you're inserting by batch as well. For the batch operations of 10 rows, you need to have a variable OUTPUT_VARIABLE1 until OUTPUT_VARIABLE10.

mikependon avatar Feb 12 '21 21:02 mikependon

If you want to execute multiple statements in Oracle, you must wrap them in an anonymous block, which requires... you guessed it, ExecuteNonQuery(). I could use a temp table to persist the generated identity between two Execute... calls instead, but that's me hurtling off the cliffs of insanity.

Let me just take a gander at what can be done to override the standard behavior with minimal changes. I'll release this as a separate PR so you can take a look at the proposal without being blinded by Larry Ellison's... idiosyncratic... brilliance.

orthoxerox avatar Feb 13 '21 10:02 orthoxerox

any update about oracle support ? It will be really neccesary.

Koleshy avatar Sep 19 '21 00:09 Koleshy