Skip to content
This repository has been archived by the owner on Nov 22, 2018. It is now read-only.

sql driver Exec should creates its own transaction #106

Open
zan-xhipe opened this issue Aug 28, 2015 · 11 comments
Open

sql driver Exec should creates its own transaction #106

zan-xhipe opened this issue Aug 28, 2015 · 11 comments

Comments

@zan-xhipe
Copy link

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

@cznic
Copy link
Owner

cznic commented Aug 28, 2015

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

@gesellix
Copy link

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.

@gernest
Copy link
Collaborator

gernest commented Apr 6, 2017

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.

@cznic
Copy link
Owner

cznic commented Apr 6, 2017

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

@gernest
Copy link
Collaborator

gernest commented May 16, 2017

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

@cznic
Copy link
Owner

cznic commented May 16, 2017

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

Relevant

@gernest
Copy link
Collaborator

gernest commented May 16, 2017

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?

@cznic
Copy link
Owner

cznic commented May 16, 2017

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.

@gernest
Copy link
Collaborator

gernest commented May 16, 2017

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?

@cznic
Copy link
Owner

cznic commented May 16, 2017

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.

@gernest
Copy link
Collaborator

gernest commented May 16, 2017

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants