-
Notifications
You must be signed in to change notification settings - Fork 7
sqlite: add optional ConnLogger #111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,9 +127,22 @@ func Connector(sqliteURI string, connInitFunc ConnInitFunc, tracer sqliteh.Trace | |
| } | ||
| } | ||
|
|
||
| // ConnectorWithLogger returns a [driver.Connector] for the given connection | ||
| // parameters. makeLogger is used to create a [ConnLogger] when [Connect] is | ||
| // called. | ||
| func ConnectorWithLogger(sqliteURI string, connInitFunc ConnInitFunc, tracer sqliteh.Tracer, makeLogger func() ConnLogger) driver.Connector { | ||
| return &connector{ | ||
| name: sqliteURI, | ||
| tracer: tracer, | ||
| makeLogger: makeLogger, | ||
| connInitFunc: connInitFunc, | ||
| } | ||
| } | ||
|
|
||
| type connector struct { | ||
| name string | ||
| tracer sqliteh.Tracer | ||
| makeLogger func() ConnLogger // or nil | ||
| connInitFunc ConnInitFunc | ||
| } | ||
|
|
||
|
|
@@ -152,12 +165,14 @@ func (p *connector) Connect(ctx context.Context) (driver.Conn, error) { | |
| } | ||
| return nil, err | ||
| } | ||
|
|
||
| c := &conn{ | ||
| db: db, | ||
| tracer: p.tracer, | ||
| id: sqliteh.TraceConnID(maxConnID.Add(1)), | ||
| } | ||
| if p.makeLogger != nil { | ||
| c.logger = p.makeLogger() | ||
| } | ||
| if p.connInitFunc != nil { | ||
| if err := p.connInitFunc(ctx, c); err != nil { | ||
| db.Close() | ||
|
|
@@ -179,6 +194,7 @@ type conn struct { | |
| db sqliteh.DB | ||
| id sqliteh.TraceConnID | ||
| tracer sqliteh.Tracer | ||
| logger ConnLogger | ||
| stmts map[string]*stmt // persisted statements | ||
| txState txState | ||
| readOnly bool | ||
|
|
@@ -202,6 +218,7 @@ func (c *conn) Close() error { | |
| err := reserr(c.db, "Conn.Close", "", c.db.Close()) | ||
| return err | ||
| } | ||
|
|
||
| func (c *conn) PrepareContext(ctx context.Context, query string) (driver.Stmt, error) { | ||
| persist := ctx.Value(persistQuery{}) != nil | ||
| return c.prepare(ctx, query, persist) | ||
|
|
@@ -341,6 +358,9 @@ func (c *conn) txInit(ctx context.Context) error { | |
| return err | ||
| } | ||
| } else { | ||
| if c.logger != nil { | ||
| c.logger.Begin() | ||
| } | ||
| // TODO(crawshaw): offer BEGIN DEFERRED (and BEGIN CONCURRENT?) | ||
| // semantics via a context annotation function. | ||
| if err := c.execInternal(ctx, "BEGIN IMMEDIATE"); err != nil { | ||
|
|
@@ -351,15 +371,16 @@ func (c *conn) txInit(ctx context.Context) error { | |
| } | ||
|
|
||
| func (c *conn) txEnd(ctx context.Context, endStmt string) error { | ||
| state, readOnly := c.txState, c.readOnly | ||
| c.txState = txStateNone | ||
| c.readOnly = false | ||
| if state != txStateBegun { | ||
| defer func() { | ||
| c.txState = txStateNone | ||
| c.readOnly = false | ||
| }() | ||
| if c.txState != txStateBegun { | ||
| return nil | ||
| } | ||
|
|
||
| err := c.execInternal(context.Background(), endStmt) | ||
| if readOnly { | ||
| if c.readOnly { | ||
| if err2 := c.execInternal(ctx, "PRAGMA query_only=false"); err == nil { | ||
| err = err2 | ||
| } | ||
|
|
@@ -377,10 +398,14 @@ func (tx *connTx) Commit() error { | |
| return ErrClosed | ||
| } | ||
|
|
||
| readonly := tx.conn.readOnly | ||
| err := tx.conn.txEnd(context.Background(), "COMMIT") | ||
| if tx.conn.tracer != nil { | ||
| tx.conn.tracer.Commit(tx.conn.id, err) | ||
| } | ||
|
Comment on lines
403
to
405
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This Tracer interface is what I was remembering with its It's kinda weird having both Tracer and Logger that are such overlaps. Oh well. I guess Tracer has the SQL with placeholders and Logger has it without? Might be worth documenting on both the relationship between the two. |
||
| if tx.conn.logger != nil && !readonly { | ||
| tx.conn.logger.Commit(err) | ||
| } | ||
| return err | ||
| } | ||
|
|
||
|
|
@@ -390,10 +415,14 @@ func (tx *connTx) Rollback() error { | |
| return ErrClosed | ||
| } | ||
|
|
||
| readonly := tx.conn.readOnly | ||
| err := tx.conn.txEnd(context.Background(), "ROLLBACK") | ||
| if tx.conn.tracer != nil { | ||
| tx.conn.tracer.Rollback(tx.conn.id, err) | ||
| } | ||
| if tx.conn.logger != nil && !readonly { | ||
| tx.conn.logger.Rollback() | ||
| } | ||
| return err | ||
| } | ||
|
|
||
|
|
@@ -490,6 +519,9 @@ func (s *stmt) ExecContext(ctx context.Context, args []driver.NamedValue) (drive | |
| if err := s.bindAll(args); err != nil { | ||
| return nil, s.reserr("Stmt.Exec(Bind)", err) | ||
| } | ||
| if s.conn.logger != nil && !s.conn.readOnly { | ||
| s.conn.logger.Statement(s.stmt.ExpandedSQL()) | ||
| } | ||
|
|
||
| if ctx.Value(queryCancelKey{}) != nil { | ||
| done := make(chan struct{}) | ||
|
|
@@ -1068,3 +1100,25 @@ func WithQueryCancel(ctx context.Context) context.Context { | |
|
|
||
| // queryCancelKey is a context key for query context enforcement. | ||
| type queryCancelKey struct{} | ||
|
|
||
| // ConnLogger is implemented by the caller to support statement-level logging | ||
| // for write transactions. Only Exec calls are logged, not Query calls, as this | ||
| // is intended as a mechanism to replay failed transactions. | ||
| // | ||
| // Aside from logging only executed statements, ConnLogger also differs from | ||
| // [sqliteh.Tracer] by logging the expanded SQL, instead of the query with | ||
| // placeholders. | ||
| type ConnLogger interface { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We probably want some kind of Close() method to ensure the file is flushed to disk when we exit.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we want |
||
| // Begin is called when a writable transaction is opened. | ||
| Begin() | ||
|
|
||
| // Statement is called with evaluated SQL when a statement is executed. | ||
| Statement(sql string) | ||
|
|
||
| // Commit is called after a commit statement, with the error resulting | ||
| // from the attempted commit. | ||
| Commit(error) | ||
|
|
||
| // Rollback is called after a rollback statement. | ||
| Rollback() | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Document when
makeLoggeris called (i.e., "when Connect is called")?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.