ql icon indicating copy to clipboard operation
ql copied to clipboard

sql driver Exec should creates its own transaction

Open zan-xhipe opened this issue 9 years ago • 11 comments

when trying to exec Exec on the database connection as such

db, err := sql.Open("ql", "test.db")
if err != nil {
    t.Fatal(err)
}

_, err = db.Exec("CREATE TABLE t (i int)")
if err != nil {
    t.Fatal(err)
}

I get this error: attempt to update the DB outside of a transaction

This should create its own transaction

zan-xhipe avatar Aug 28 '15 08:08 zan-xhipe

On a two week vacations currently. Will take a look after that.

cznic avatar Aug 28 '15 09:08 cznic

I encountered a similar issue while using gorp's func (m *DbMap) CreateTablesIfNotExists() error, which is not available in a gorp.Transaction{}. Implicitly wrapping Exec calls in a transaction would help.

gesellix avatar Feb 21 '16 20:02 gesellix

ping @cznic

There is no interest in this feature. The way I see it the current implementation ensures integrity without adding overhead.

It is the duty of the user to wrap all operations which will change the state of the database in a transaction.

We can close this, and if there is another use case for this feature this issue can always be reopened.

gernest avatar Apr 06 '17 17:04 gernest

Shame on me, I forgot about this completely.

However, as @gernest pointed out, R/W transactions are not free, quite the opposite.

If someone wants to come with an implementation turning this feature on optionally, it would be accepted. (Suggestion, introduce PRAGMA statements.)

cznic avatar Apr 06 '17 17:04 cznic

@cznic can you expand a bit on what you think about PRAGMA and probably what PRAGMA is?

gernest avatar May 16 '17 10:05 gernest

I thougth that there's a PRAGMA statement for that but I cannot find anything so it probably isn't.

Relevant

cznic avatar May 16 '17 10:05 cznic

The only information useful on the sqllite you provided is that auto commit is on by default, which I think is irrelevant to this issue.

Doing this at ql.DB level is trivial, but exposing it over database/sql is not obvious. I'm still against this, given the nature of the ql database, enforcing transaction is not going to improve anything. Yet if there is a sensible suggestion I will happily help to implement.

BTW: Have you done benchmarks on ql? To see how it compares to other embedded databases e.g sqlite?

gernest avatar May 16 '17 10:05 gernest

Yet if there is a sensible suggestion I will happily help to implement.

No.

Have you done benchmarks on ql?

There's a lot of benchmarks (-bench .).

To see how it compares to other embedded databases e.g sqlite?

Probably badly. I've never finished the new backend and the current one has never been optimized. Also, from today's POV, there's a lot of bad design choices made making things slower than necessary.

cznic avatar May 16 '17 11:05 cznic

rom today's POV, there's a lot of bad design choices made making things slower than necessary.

Is there a chance to improve? or fix the mistakes?

gernest avatar May 16 '17 11:05 gernest

Sure, rewrite from scratch ;-)

But seriously, profiling and gradually improving the problematic parts will work as well. It's hard to tell which way is more work, but both are probably a lot/too much of work.

cznic avatar May 16 '17 11:05 cznic

Sure, rewrite from scratch ;-)

😄

I see.

I think ql is useful for testing. After I have added support to the orm I've been hacking on. I run all tests on my projects with ql, with prospects of later on to switch to postgres/mysql/sqlite in production by just switching the driver.

gernest avatar May 16 '17 11:05 gernest