Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
10 changes: 7 additions & 3 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
lint:
runs-on: self-hosted
container:
image: golangci/golangci-lint:v2.12.2
image: golangci/golangci-lint:v2.15.0
steps:
- name: Checkout
uses: actions/checkout@v7
Expand Down Expand Up @@ -124,7 +124,7 @@ jobs:
test_fb:
runs-on: self-hosted
container:
image: golang:1.26
image: golang:${{ matrix.GO_VERSION || '1.26' }}
strategy:
matrix:
include:
Expand All @@ -134,8 +134,12 @@ jobs:
FB_DOWNLOAD_URL: http://minio.red-soft.biz/3rdparty/firebird/Firebird-${FB40_VERSION}-linux-x64.tar.gz
- FB_VERSION: ${FB50_VERSION}
FB_DOWNLOAD_URL: http://minio.red-soft.biz/3rdparty/firebird/Firebird-${FB50_VERSION}-linux-x64.tar.gz
- FB_VERSION: ${FB50_VERSION}
FB_DOWNLOAD_URL: http://minio.red-soft.biz/3rdparty/firebird/Firebird-${FB50_VERSION}-linux-x64.tar.gz
GO_VERSION: "1.27"
REPORT_SUFFIX: _go1.27
env:
REPORT: firebird_${{matrix.FB_VERSION}}
REPORT: firebird_${{matrix.FB_VERSION}}${{matrix.REPORT_SUFFIX}}
steps:
- run:
echo ${REPORT}
Expand Down
15 changes: 10 additions & 5 deletions fbconn/auth_options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package fbconn
import (
"context"
"fmt"
"reflect"
"testing"

"github.com/firebirdsql/fbx/fbproto/auth"
Expand Down Expand Up @@ -86,27 +85,33 @@ func TestConfigCopyConstructorOptionsMapNotLinked(t *testing.T) {
func TestConfigSetOptionsSlicesNotLinked(t *testing.T) {
registerPlugin()

conf, _ := ParseConfig("")
conf, err := ParseConfig("")
require.NoError(t, err)

wantData1 := []byte{1, 2, 3}
wantData2 := []byte{1, 2}

optionsSlice := []auth.ConstructorOption{withData(wantData1)}

err := conf.SetPluginConstructorOptions(constructorTestPluginName, optionsSlice...)
err = conf.SetPluginConstructorOptions(constructorTestPluginName, optionsSlice...)
require.NoError(t, err)

innerSlice := conf.ConstructorOptions[constructorTestPluginName]

require.Len(t, innerSlice, 1)
require.Len(t, optionsSlice, 1)
require.Equal(t, reflect.ValueOf(optionsSlice[0]).Pointer(), reflect.ValueOf(innerSlice[0]).Pointer())
plugin := &testPlugin{}
require.NoError(t, innerSlice[0](plugin))
require.Equal(t, wantData1, plugin.data)

optionsSlice[0] = withData(wantData2)

require.Len(t, innerSlice, 1)
require.Len(t, optionsSlice, 1)
require.NotEqual(t, reflect.ValueOf(optionsSlice[0]).Pointer(), reflect.ValueOf(innerSlice[0]).Pointer())
require.NoError(t, innerSlice[0](plugin))
require.Equal(t, wantData1, plugin.data)
require.NoError(t, optionsSlice[0](plugin))
require.Equal(t, wantData2, plugin.data)
}

func TestConnectConfigPassesConstructorOptionsToPlugin(t *testing.T) {
Expand Down
9 changes: 6 additions & 3 deletions fbconn/compression_close_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ func TestAsyncCloseWaitsForCompressionRead(t *testing.T) {

transport.releaseRead()
select {
case <-readDone:
case err := <-readDone:
require.ErrorIs(t, err, net.ErrClosed)
case <-time.After(time.Second):
t.Fatal("compressed read did not finish after the transport was released")
}
Expand Down Expand Up @@ -196,15 +197,17 @@ func testSynchronousCloseUnblocksCompressedReadBeforeCleanup(
}

select {
case <-closeDone:
case err := <-closeDone:
require.ErrorIs(t, err, net.ErrClosed)
transport.releaseRead()
t.Fatal("Close returned while the compressed read was active")
case <-time.After(20 * time.Millisecond):
}

transport.releaseRead()
select {
case <-readDone:
case err := <-readDone:
require.ErrorIs(t, err, net.ErrClosed)
case <-time.After(time.Second):
t.Fatal("compressed read did not finish after the transport was released")
}
Expand Down
4 changes: 3 additions & 1 deletion fbconn/events_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"errors"
"net"
"testing"
"time"

Expand Down Expand Up @@ -166,7 +167,8 @@ func TestSetEventErrorHandlerCalledForFirstFailure(t *testing.T) {
}

select {
case <-called:
case err := <-called:
require.ErrorIs(t, err, net.ErrClosed)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This branch means the handler was called more than once, which is already a test failure regardless of the error. Why do we expect net.ErrClosed here when the test only passes firstErr and secondErr? Seems we can include err in t.Fatalf instead, so the failure reports the unexpected extra call rather than an unrelated error mismatch. The same applies to the early-return checks in manager_internal_test.go, compression_close_test.go, and client_compression_test.go.

t.Fatal("event error handler called more than once for same error cycle")
default:
}
Expand Down
4 changes: 3 additions & 1 deletion fbevent/manager_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package fbevent
import (
"context"
"errors"
"net"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -412,7 +413,8 @@ func TestAddEventListenerSerializesConcurrentFirstRegistration(t *testing.T) {
}()

select {
case <-errSecondCh:
case err := <-errSecondCh:
require.ErrorIs(t, err, net.ErrClosed)
t.Fatal("second AddEventListener returned before first registration finished")
case <-time.After(150 * time.Millisecond):
}
Expand Down
3 changes: 2 additions & 1 deletion fbproto/client_compression_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,8 @@ func TestCloseCompressionWaitsForActiveCompressedWrite(t *testing.T) {
}()
<-closeStarted
select {
case <-closeDone:
case err := <-closeDone:
require.ErrorIs(t, err, net.ErrClosed)
transport.release()
t.Fatal("CloseCompression completed during an active compressed write")
case <-time.After(20 * time.Millisecond):
Expand Down
3 changes: 3 additions & 0 deletions fbtype/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,9 @@ func codecScan(codec Codec, m *Map, typeMetadata TypeMetadata, src []byte, dst a
// If v already implements sql.Scanner it is returned unchanged. Otherwise v
// must be a target supported by the Firebird array scan path, such as *[]T,
// *Array[T], *FlatArray[T], or a compatible array/slice target.
//
// As of Go 1.27, [database/sql] calls the driver directly to scan columns when using fbx's stdlib package, so this is
// no longer necessary.
func (m *Map) SQLScanner(v any) sql.Scanner {
if s, ok := v.(sql.Scanner); ok {
return s
Expand Down
24 changes: 21 additions & 3 deletions stdlib/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,17 @@
//
// # Firebird-specific data types
//
// The [fbtype] package provides Firebird-specific types and codecs.
// As of Go 1.27, [database/sql] allows drivers to implement their own scanning logic by implementing the
// driver.RowsColumnScanner interface. This allows Firebird types such as arrays to be scanned directly into Go
// values such as slices.
//
// input := []int64{1, 2, 3}
// _, err := db.Exec("insert into array_example(bigint_array) values(?)", input)
//
// var output []int64
// err = db.QueryRow("select bigint_array from array_example").Scan(&output)
//
// In older versions of Go, the [fbtype] package provides Firebird-specific types and codecs.
// [fbtype.Map.SQLScanner] is primarily used to scan Firebird arrays through
// [database/sql]:
//
Expand Down Expand Up @@ -787,6 +797,7 @@ type Rows struct {
skipNextMore bool

columnNames []string
blobValues [][]byte
}

// Columns implements [driver.Rows].
Expand Down Expand Up @@ -970,8 +981,9 @@ func scanBlobValues(rows fbx.Rows, blobColumns []bool) ([][]byte, error) {
return values, nil
}

// Next implements [driver.Rows].
func (r *Rows) Next(dest []driver.Value) (err error) {
// initValueFuncs prepares the [database/sql] representation of each column. Both
// [Next] and [ScanColumn] use these conversions so their [driver.Values] agree.
func (r *Rows) initValueFuncs() {
m := r.conn.conn.TypeMap()
fieldDescriptions := r.rows.FieldDescriptions()

Expand Down Expand Up @@ -1131,8 +1143,14 @@ func (r *Rows) Next(dest []driver.Value) (err error) {
}
}
}
}

// Next implements [driver.Rows].
func (r *Rows) Next(dest []driver.Value) (err error) {
r.initValueFuncs()

var more bool
fieldDescriptions := r.rows.FieldDescriptions()
if r.skipNext {
more = r.skipNextMore
r.skipNext = false
Expand Down
153 changes: 153 additions & 0 deletions stdlib/sql_go1.27.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
//go:build go1.27

package stdlib

import (
"database/sql"
"database/sql/driver"
"errors"
"io"
"reflect"
"time"

"github.com/firebirdsql/fbx/fbtype"
)

// [Rows] implements [driver.RowsColumnScanner] as of Go 1.27.
var _ driver.RowsColumnScanner = (*Rows)(nil)

// NextRow implements the [driver.RowsColumnScanner] interface. It advances to the
// next row of data and returns [io.EOF] when there are no more rows.
func (r *Rows) NextRow() error {
var more bool
if r.skipNext {
more = r.skipNextMore
r.skipNext = false
} else {
more = r.rows.Next()
}

if !more {
if err := r.rows.Err(); err != nil {
return err
}
return io.EOF
}

r.initValueFuncs()

var err error
if r.hasBlobCols {
r.blobValues, err = scanBlobValues(r.rows, r.blobColumns)
if err != nil {
return err
}
}

return nil
}

// ScanColumn implements the [driver.RowsColumnScanner] interface. It preserves
// [database/sql] conversions for scalar destinations and [sql.Scanner] implementations.
// Other destinations, such as Go slices, fbtype.Array, are
// scanned directly using the fbx type map.
func (r *Rows) ScanColumn(scanCtx driver.ScanContext, index int, dest any) error {
if dest == nil {
return errors.New("destination not a pointer")
}
rv := reflect.ValueOf(dest)
if rv.Kind() == reflect.Pointer && rv.IsNil() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This changes the existing database/sql behavior for typed-nil sql.Scanner values: their Scan method used to be called and could handle a nil receiver, but this check rejects them before that call. Is this intentional, or should we preserve the previous behavior? A test would help make the intended behavior explicit.

return errors.New("destination pointer is nil")
}

fd := r.rows.FieldDescriptions()[index]
typeMetadata := fbtype.NewTypeMetadata(
fd.Type,
fd.Subtype,
fd.Length,
fd.Scale,
)

arr := fd.ArrayData
if arr != nil {
arrayData, err := fbtype.NewArrayData(arr.TypeId, arr.SubType, arr.Length, arr.Scale, arr.Dimensions)
if err != nil {
return err
}
typeMetadata.SetArrayMetadata(arrayData)
}
src := r.rows.RawValues()[index]

if fd.IsBlob() {
if src == nil {
return sql.ConvertAssign(scanCtx, dest, nil)
}
blobValue, err := buildBlobValue(r.blobValues[index], &r.rows.FieldDescriptions()[index])
if err != nil {
return err
}

return sql.ConvertAssign(scanCtx, dest, blobValue)
}

if isSQLScanDestination(rv.Type()) {
var value driver.Value
if src != nil {
var err error
value, err = r.valueFuncs[index](src)
if err != nil {
return err
}
}
return sql.ConvertAssign(scanCtx, dest, value)
}

m := r.conn.conn.TypeMap()

return m.Scan(typeMetadata, src, dest)
}

// isSQLScanDestination includes named scalar types and nullable pointers to
// [database/sql] destinations. Select the conversion before scanning: retrying a
// failed scan could call user code twice or discard its error.
func isSQLScanDestination(t reflect.Type) bool {
var namedPointers map[reflect.Type]bool
for {
if t.Implements(reflect.TypeFor[sql.Scanner]()) {
return true
}
// These interfaces distinguish native byte slices, such as
// FlatArray[byte] and PreallocBytes, from ordinary []byte destinations.
if t.Implements(reflect.TypeFor[fbtype.ArraySetter]()) || t.Implements(reflect.TypeFor[fbtype.BytesScanner]()) {
return false
}
if t.Kind() != reflect.Pointer {
break
}
// Only defined pointer types can form a cycle without reaching a
// non-pointer type. Leave those to fbx's bounded scan planning.
if t.Name() != "" {
if namedPointers[t] {
return false
}
if namedPointers == nil {
namedPointers = make(map[reflect.Type]bool)
}
namedPointers[t] = true
}
t = t.Elem()
}

switch t.Kind() {
case reflect.Bool, reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64,
reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64,
reflect.Float32, reflect.Float64, reflect.String, reflect.Interface:
return true
case reflect.Slice:
return t.Elem().Kind() == reflect.Uint8
case reflect.Struct:
return t.ConvertibleTo(reflect.TypeFor[time.Time]())
default:
return false
}
}
Loading