SQLProvider icon indicating copy to clipboard operation
SQLProvider copied to clipboard

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

Open jlogar opened this issue 7 years ago • 20 comments

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

  • Firebird 2.5.6
  • Win 10
  • .NET 4.6

jlogar avatar Jun 28 '17 16:06 jlogar

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

jlogar avatar Jun 30 '17 07:06 jlogar

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.

Thorium avatar Jun 30 '17 07:06 Thorium

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

pezipink avatar Jun 30 '17 11:06 pezipink

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

Thorium avatar Jun 30 '17 12:06 Thorium

ping @gibranrosa

pezipink avatar Jun 30 '17 12:06 pezipink

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...

jlogar avatar Jul 02 '17 06:07 jlogar

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.

gibranrosa avatar Jul 31 '17 17:07 gibranrosa

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)

jlogar avatar Sep 06 '17 08:09 jlogar

Hi @jlogar , can you send the resulting errors?

gibranrosa avatar Sep 06 '17 18:09 gibranrosa

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

jlogar avatar Sep 07 '17 18:09 jlogar

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.

gibranrosa avatar Sep 08 '17 13:09 gibranrosa

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.

jlogar avatar Sep 10 '17 15:09 jlogar

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.

Thorium avatar Sep 10 '17 15:09 Thorium

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.

jlogar avatar Sep 10 '17 20:09 jlogar

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?

gibranrosa avatar Sep 12 '17 19:09 gibranrosa

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.

jlogar avatar Sep 24 '17 17:09 jlogar

Please make PR.... :-)

Thorium avatar Sep 24 '17 18:09 Thorium

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.

Thorium avatar Oct 21 '17 04:10 Thorium

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.

gibranrosa avatar Apr 30 '18 15:04 gibranrosa

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

Some details:

  • We shouldn't execute parallel queries inside same ado db connection, as connection pooling doesn't handle parallel execution well. So if you create two select-clauses in parallel, they should use separate DB connections. For only selects, there are no locking of any kind, so deadlocks shouldn't appear.
  • If you use update operations, db.SubmitUpdates, we will create a transaction from System.Transactions library. The default transaction scope is Required. https://msdn.microsoft.com/en-us/library/system.transactions.transactionscopeoption(v=vs.110).aspx It means if there is an explicit transaction ongoing by the user, we will commit/join that transaction, and the updates will be tied to that. So in that sense, if you capsulate your operation to a transaction, it will be used in SubmitUpdates, and there is no possibility that someone else would go and update the same record at the same time.

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.

Thorium avatar Apr 30 '18 15:04 Thorium