Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion go/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,16 @@ type StaticTags struct {
}

type CommenterOptions struct {
Driver DriverOptions
Config CommenterConfig
Tags StaticTags
}

type DriverOptions struct {
// Setting this to true means your underlying driver supports the ConnBeginTx interface
WithBeginTX bool
}

func encodeURL(k string) string {
return url.QueryEscape(string(k))
}
Expand All @@ -71,7 +77,7 @@ func ConvertMapToComment(tags map[string]string) string {
var sb strings.Builder
i, sz := 0, len(tags)

//sort by keys
// sort by keys
sortedKeys := make([]string, 0, len(tags))
for k := range tags {
sortedKeys = append(sortedKeys, k)
Expand Down
22 changes: 22 additions & 0 deletions go/database/sql/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,19 @@ type sqlCommenterConn struct {
options core.CommenterOptions
}

type sqlCommenterConnWithTx struct {
sqlCommenterConn
}

func newConn(conn driver.Conn, options core.CommenterOptions) driver.Conn {
commenterConn := newSQLCommenterConn(conn, options)
if options.Driver.WithBeginTX {
return &sqlCommenterConnWithTx{*commenterConn}
}

return commenterConn
}

func newSQLCommenterConn(conn driver.Conn, options core.CommenterOptions) *sqlCommenterConn {
return &sqlCommenterConn{
Conn: conn,
Expand Down Expand Up @@ -89,6 +102,15 @@ func (s *sqlCommenterConn) Raw() driver.Conn {
return s.Conn
}

func (s *sqlCommenterConnWithTx) BeginTx(ctx context.Context, opts driver.TxOptions) (driver.Tx, error) {
begintxer, ok := s.Conn.(driver.ConnBeginTx)
if !ok {
return nil, driver.ErrSkip
}
Comment on lines +107 to +109
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need an extra driver config ?

Will this condition not take care of any driver not implementing `ConnBeginTx' ?

Copy link
Contributor Author

@kostyay kostyay Dec 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, it seems from this code that the go sql library will handle it differently if the interface is implemented or not
https://github.com/golang/go/blob/master/src/database/sql/ctxutil.go#L97-L135

As you can see from that code if the interface is not implemented there is a fallback that eventually returns

errors.New("sql: driver does not support read-only transactions")

Thats how I originally found this issue

Copy link
Contributor

@sjs994 sjs994 Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What i meant is do we need to create a new struct sqlCommenterConnWithTx ?

Can we just not implement BeginTx for sqlCommenterConn ?

func (s *sqlCommenterConn) BeginTx(ctx context.Context, opts driver.TxOptions) (driver.Tx, error) {...}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that :).
It's mainly your call.
Doing so makes the code simpler but it may change the expected behavior for drivers that don't implement BeginTx as the code in go library has a special handling for drivers that don't implement that interface.


return begintxer.BeginTx(ctx, opts)
}

// ***** Commenter Functions *****

func (conn *sqlCommenterConn) withComment(ctx context.Context, query string) string {
Expand Down
4 changes: 2 additions & 2 deletions go/database/sql/gosql.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (d *sqlCommenterDriver) Open(name string) (driver.Conn, error) {
if err != nil {
return nil, err
}
return newSQLCommenterConn(rawConn, d.options), nil
return newConn(rawConn, d.options), nil
}

func (d *sqlCommenterDriver) OpenConnector(name string) (driver.Connector, error) {
Expand Down Expand Up @@ -73,7 +73,7 @@ func (c *sqlCommenterConnector) Connect(ctx context.Context) (connection driver.
if err != nil {
return nil, err
}
return newSQLCommenterConn(connection, c.options), nil
return newConn(connection, c.options), nil
}

func (c *sqlCommenterConnector) Driver() driver.Driver {
Expand Down
9 changes: 9 additions & 0 deletions go/go.work
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
go 1.19

use (
./core
./database/sql
./gorrila/mux
./net/http
./samples/http
)