fsprojects / SQLProvider

A general F# SQL database erasing type provider, supporting LINQ queries, schema exploration, individuals, CRUD operations and much more besides.
https://fsprojects.github.io/SQLProvider
Other
567 stars 144 forks source link

Firebird can't be used with case sensitive (quoted) table names #431

Open jlogar opened 7 years ago

jlogar commented 7 years ago

Description

I have a DB that uses quoted (camel case) table names. I couldn't find a way to make the provider use correct names so all my queries fail.

Repro steps

Do a query on a table that was created using quotes. Something like "Assistant". Query using sth like:

    let ctx = sql.GetDataContext()
    let a = query {
             for ass in ctx.Dbo.Assistant do
             select ass
             }

Expected behavior

Rows should be returned.

Actual behavior

An exception occurs because the provider tries to use an unquoted table name so upper case is used by Firebird (eg. ASSISTANT). The query fails with:

SQL error code = -204
Table unknown
ASSISTANT

Related information

jlogar commented 7 years ago

taking a peek at the code, it would probably make sense to replace occurrences of: Table.Name.Replace("[","\"").Replace("]","\"").Replace("``","\"") with a separate function that would optionally (SqlDataProvider static parameter?) quote the table name.

thanks to @emmanueltouzery for pointing this out

Thorium commented 7 years ago

There is a case sensitivity static parameter that may or may not help.

ODBC code has an option to change the quote char, but that is currently the only one.

pezipink commented 7 years ago

would suggest we simply re-use the existing static parameter, shame it is named OdbcQuoteCharacter ...

Thorium commented 7 years ago

Are you stating that Firebird is having multiple different quote chars? Or that the current one used is wrong?

pezipink commented 7 years ago

ping @gibranrosa

jlogar commented 7 years ago

I didn't notice the param in the docs before. Checked it out (http://fsprojects.github.io/SQLProvider/core/odbc.html) and it seems appropriate (apart from the naming :)). Would the approach I mention above and using the OdbcQuoteCharacter be the way to go? I am really new to F# so might take me a while...

gibranrosa commented 6 years ago

Hi! Sorry for the long delay! :/ @Thorium I haven't considered the case for quoted identifiers, but it is the double quote the correct one. @jlogar This approach, I think, is the way to go, and be sure to test the case for Unquoted identifiers after the modification, which would be the OdbcQuoteCharacter being null, I suggest.

jlogar commented 6 years ago

OK, so I finally got around to this again... The first thing I'm trying to do is to run Firebird specific tests so that I can add a test for the setting as the next step. I do that by running fsi .\tests\SqlProvider.Tests\scripts\FirebirdTests.fsx. The tests fail out of the box. Is that something that should be fixed first? (it seems like the provided DB has records that should not be there and some keys are failing on insert)

gibranrosa commented 6 years ago

Hi @jlogar , can you send the resulting errors?

jlogar commented 6 years ago

Sure. Here's the first:

Executing SQL: EXEC ADD_JOB_HISTORY(101, 13. 01. 1993 00:00:00, 24. 07. 1998 00:00:00, "IT_PROG", 60) - params
FirebirdSql.Data.FirebirdClient.FbException (0x80004005): violation of PRIMARY or UNIQUE KEY constraint "INTEG_133" on table "JOB_HISTORY"
Problematic key value is ("EMPLOYEE_ID" = 101, "START_DATE" = '1993-01-13')
At procedure 'ADD_JOB_HISTORY' line: 11, col: 3 ---> violation of PRIMARY or UNIQUE KEY constraint "INTEG_133" on table "JOB_HISTORY"
Problematic key value is ("EMPLOYEE_ID" = 101, "START_DATE" = '1993-01-13')
At procedure 'ADD_JOB_HISTORY' line: 11, col: 3
   at FirebirdSql.Data.FirebirdClient.FbCommand.ExecuteNonQuery()
   at FSharp.Data.Sql.Providers.Firebird.executeSprocCommand(IDbCommand com, QueryParameter[] inputParams, QueryParameter[] retCols, Object[] values) in C:\_tlaka\oss\SQLProvider\src\SQLProvider\Providers.Firebird.fs:line 403
   at FSharp.Data.Sql.Providers.FirebirdProvider.FSharp-Data-Sql-Common-ISqlProvider-ExecuteSprocCommand(IDbCommand com, QueryParameter[] definition, QueryParameter[] retCols, Object[] values) in C:\_tlaka\oss\SQLProvider\src\SQLProvider\Providers.Firebird.fs:line 591
   at FSharp.Data.Sql.Runtime.SqlDataContext.FSharp-Data-Sql-Common-ISqlDataContext-CallSproc(RunTimeSprocDefinition def, QueryParameter[] retCols, Object[] values) in C:\_tlaka\oss\SQLProvider\src\SQLProvider\SqlRuntime.DataContext.fs:line 125
   at <StartupCode$FSI_0001>.$FSI_0001.main@() in C:\_tlaka\oss\SQLProvider\tests\SqlProvider.Tests\scripts\FirebirdTests.fsx:line 174

Can be fixed by:

-ctx.Procedures.AddJobHistory.Invoke(101, DateTime(1993, 1, 13), DateTime(1998, 7, 24), "IT_PROG", 60)
+ctx.Procedures.AddJobHistory.Invoke(101, DateTime(1998, 7, 25), DateTime(2001, 4, 1), "IT_PROG", 60)

After that change errors occur in the threading test:

Executing SQL: UPDATE COUNTRIES SET COUNTRY_NAME = @param0 WHERE COUNTRY_ID = @pk0; - params @param0 - "Argentina113"; @pk0 - "AR";
Executing SQL: UPDATE COUNTRIES SET REGION_ID = @param0 WHERE COUNTRY_ID = @pk0; - params @param0 - 113; @pk0 - "BR";
System.AggregateException: One or more errors occurred. ---> FirebirdSql.Data.FirebirdClient.FbException: lock conflict on no wait transaction
deadlock
update conflicts with concurrent update
concurrent transaction number is 3042 ---> FirebirdSql.Data.Common.IscException: lock conflict on no wait transaction
deadlock
update conflicts with concurrent update
concurrent transaction number is 3042
   at FirebirdSql.Data.Client.Managed.Version10.GdsDatabase.ProcessResponse(IResponse response)
   at FirebirdSql.Data.Client.Managed.Version10.GdsDatabase.ReadResponse()
   at FirebirdSql.Data.Client.Managed.Version10.GdsDatabase.ReadGenericResponse()
   at FirebirdSql.Data.Client.Managed.Version12.GdsStatement.Execute()
   at FirebirdSql.Data.FirebirdClient.FbCommand.ExecuteCommand(CommandBehavior behavior, Boolean returnsSet)
   at FirebirdSql.Data.FirebirdClient.FbCommand.ExecuteNonQuery()
   --- End of inner exception stack trace ---
   at FirebirdSql.Data.FirebirdClient.FbCommand.ExecuteNonQuery()
   at <StartupCode$FSharp-Data-SqlProvider>.$Providers.Firebird.FSharp-Data-Sql-Common-ISqlProvider-ProcessUpdates@945-8.Invoke(SqlEntity e) in C:\_tlaka\oss\SQLProvider\src\SQLProvider\Providers.Firebird.fs:line 956
   at Microsoft.FSharp.Collections.SeqModule.Iterate[T](FSharpFunc`2 action, IEnumerable`1 source)
   at FSharp.Data.Sql.Providers.FirebirdProvider.FSharp-Data-Sql-Common-ISqlProvider-ProcessUpdates(IDbConnection con, ConcurrentDictionary`2 entities, TransactionOptions transactionOptions) in C:\_tlaka\oss\SQLProvider\src\SQLProvider\Providers.Firebird.fs:line 944
   at <StartupCode$FSharp-Data-SqlProvider>.$SqlRuntime.DataContext.f@1-44(SqlDataContext __, IDbConnection con, Unit unitVar0) in C:\_tlaka\oss\SQLProvider\src\SQLProvider\SqlRuntime.DataContext.fs:line 88
   at FSharp.Data.Sql.Runtime.SqlDataContext.FSharp-Data-Sql-Common-ISqlDataContext-SubmitPendingChanges() in C:\_tlaka\oss\SQLProvider\src\SQLProvider\SqlRuntime.DataContext.fs:line 87
   at System.Threading.Tasks.Task.InnerInvoke()
   at System.Threading.Tasks.Task.Execute()
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout)
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks)
   at <StartupCode$FSI_0001>.$FSI_0001.main@() in C:\_tlaka\oss\SQLProvider\tests\SqlProvider.Tests\scripts\FirebirdTests.fsx:line 221
gibranrosa commented 6 years ago

Please put a Delete command filtering by the problematic primary key, probably I push the database after the Insert command :D . The multithreading errors maybe due to the Firebird lock mechanism, so we could get rid of them by changing the UPDATE commands in a way to NOT update the same record in different threads. Please tell me if you can't get it working.

jlogar commented 6 years ago

OK, added the delete part. How do you want the PRs organized? I'm not really sure how to avoid the concurrency issue? I'd avoid any locking and with a 100 iterations there's not enough rows to begin with if we'd pick a different country each time.

Thorium commented 6 years ago

About locking, are you talking about some demodata or SQLProvider itself? Typically databases are not accepting multiple parallel SQL-clause executions inside the same pooled connection. This should be already handled by SQLProvider. So if you want to make parallel threads, create a new database connection for each thread.

jlogar commented 6 years ago

Not really sure. Didn't go too far analyzing that part. What I see is a bunch of threads trying to update the same record. They do HR.GetDataContext() for each. Not sure what that means as far as connections are concerned. Probably best for @gibranrosa to say since he knows what the code actually tests.

gibranrosa commented 6 years ago

Actually I've copied the Tests from the Mysql provider. The test with error has two Seq.maps that query for some record and update it. For every of the 300 threads, only one record is queried and updated, so if 2 threads query at the same time, they have 2 copies of the same record, one thread update his copy and if the second thread change his copy, as soon as the second thread tries to commit, we have a deadlock. I don't know if that case is handled by the other databases, @Thorium, or we should change the test to avoid ourselves the deadlock?

jlogar commented 6 years ago

OK, I think I got the quotes working. I added a method called getQuotedTableName that is used whenever a table is referenced (not the alias). Also replaced all the duplicated quotations with .Name.Replace(..)... with the method.

Still clueless about the threading issues though.

Thorium commented 6 years ago

Please make PR.... :-)

Thorium commented 6 years ago

I think that the threading issue could be solved by changing the DataReaderWithCommand, to kind of solution I modified MySql-version recently, __.GetTables(con,cs) method

    Sql.connect con (fun con ->
        let executeSql createCommand sql (con:IDbConnection) = 
            use com : IDbCommand = createCommand sql con   
            use reader = com.ExecuteReader()
            [ while reader.Read() do
                let table ={ Schema = reader.GetString(0); Name = reader.GetString(1); Type=reader.GetString(2) }
                yield tableLookup.GetOrAdd(table.FullName,table) ]
        executeSql MySql.createCommand (sprintf "select TABLE_SCHEMA, TABLE_NAME, TABLE_TYPE from INFORMATION_SCHEMA.TABLES where %s = '%s'" caseChane dbName) con)

The idea here is that executeSql is local function and not from Utils, and for that reason com and reader cannot be disposed before exiting the scope. MySql did work previously correctly but some recent versions of MySql.Data.dll started to dispose the command if Utils class executeSql was being used.

gibranrosa commented 6 years ago

Hi @Thorium ! I was digging into it, I managed to exclude the DataReaderWithCommand with your suggestion, but the threading issue remains, as it seems to be an issue with how the Firebird or the Firebird library deal with this. So I have a question, in any other database system, when there is two independent transactions, within two different connections, updating the same record at the same moment, this is handled smoothly in the case of this tests? Since I have copied them from Mysql.

Edit: And if this would take some investigation too, I could manage to get some Mysql instance to dig into it, of course.

Thorium commented 6 years ago

I have tried with other providers, and this do work.

Some details:

So executing this in parallel should work without any other thread interfering any thread between query and submit:

let someOperation() =
   use t = new Transactions.TransactionScope(Transactions.TransactionScopeAsyncFlowOption.Enabled)
   let ctx = TypeProviderConnection.GetDataContext connectionstring
   let items = query { ctx... }
   items |> Seq.iter(fun x -> x.Thing <- "updated")
   ctx.SubmitUpdates()

Edit: Also be aware of the transaction IsolationLevel you are using.