Skip to content

Commit

Permalink
[rdbms] Remove error return value from BuildQuery (#312)
Browse files Browse the repository at this point in the history
  • Loading branch information
nathan-artie authored Mar 18, 2024
1 parent 9b870d9 commit 7e6d149
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 24 deletions.
4 changes: 2 additions & 2 deletions lib/mysql/scanner/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ func queryPlaceholders(count int) []string {
return result
}

func (s scanAdapter) BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool, batchSize uint) (string, []any, error) {
func (s scanAdapter) BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool, batchSize uint) (string, []any) {
colNames := make([]string, len(s.columns))
for idx, col := range s.columns {
colNames[idx] = schema.QuoteIdentifier(col.Name)
Expand Down Expand Up @@ -168,7 +168,7 @@ func (s scanAdapter) BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool
strings.Join(quotedKeyNames, ","),
// LIMIT
batchSize,
), slices.Concat(startingValues, endingValues), nil
), slices.Concat(startingValues, endingValues)
}

func (s scanAdapter) ParseRow(values []any) error {
Expand Down
6 changes: 2 additions & 4 deletions lib/mysql/scanner/scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,15 +249,13 @@ func TestScanAdapter_BuildQuery(t *testing.T) {
}
{
// exclusive lower bound
query, parameters, err := adapter.BuildQuery(keys, false, 12)
assert.NoError(t, err)
query, parameters := adapter.BuildQuery(keys, false, 12)
assert.Equal(t, "SELECT `foo`,`bar` FROM `table` WHERE (`foo`) > (?) AND (`foo`) <= (?) ORDER BY `foo` LIMIT 12", query)
assert.Equal(t, []any{"a", "b"}, parameters)
}
{
// inclusive upper and lower bounds
query, parameters, err := adapter.BuildQuery(keys, true, 12)
assert.NoError(t, err)
query, parameters := adapter.BuildQuery(keys, true, 12)
assert.Equal(t, "SELECT `foo`,`bar` FROM `table` WHERE (`foo`) >= (?) AND (`foo`) <= (?) ORDER BY `foo` LIMIT 12", query)
assert.Equal(t, []any{"a", "b"}, parameters)
}
Expand Down
4 changes: 2 additions & 2 deletions lib/postgres/scanner.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func queryPlaceholders(offset, count int) []string {
return result
}

func (s scanAdapter) BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool, batchSize uint) (string, []any, error) {
func (s scanAdapter) BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool, batchSize uint) (string, []any) {
castedColumns := make([]string, len(s.columns))
for i, col := range s.columns {
castedColumns[i] = castColumn(col)
Expand Down Expand Up @@ -196,7 +196,7 @@ func (s scanAdapter) BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool
strings.Join(quotedKeyNames, ","),
// LIMIT
batchSize,
), slices.Concat(startingValues, endingValues), nil
), slices.Concat(startingValues, endingValues)
}

func (s scanAdapter) ParseRow(values []any) error {
Expand Down
6 changes: 2 additions & 4 deletions lib/postgres/scanner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,13 @@ func TestScanAdapter_BuildQuery(t *testing.T) {

{
// inclusive lower bound
query, parameters, err := adapter.BuildQuery(primaryKeys, true, 1)
assert.NoError(t, err)
query, parameters := adapter.BuildQuery(primaryKeys, true, 1)
assert.Equal(t, `SELECT "a","b","c","e","f",ARRAY_TO_JSON("g")::TEXT as "g" FROM "schema"."table" WHERE row("a","b","c") >= row($1,$2,$3) AND row("a","b","c") <= row($4,$5,$6) ORDER BY "a","b","c" LIMIT 1`, query)
assert.Equal(t, []any{int64(1), int64(2), "3", int64(4), int64(5), "6"}, parameters)
}
{
// exclusive lower bound
query, parameters, err := adapter.BuildQuery(primaryKeys, false, 2)
assert.NoError(t, err)
query, parameters := adapter.BuildQuery(primaryKeys, false, 2)
assert.Equal(t, `SELECT "a","b","c","e","f",ARRAY_TO_JSON("g")::TEXT as "g" FROM "schema"."table" WHERE row("a","b","c") > row($1,$2,$3) AND row("a","b","c") <= row($4,$5,$6) ORDER BY "a","b","c" LIMIT 2`, query)
assert.Equal(t, []any{int64(1), int64(2), "3", int64(4), int64(5), "6"}, parameters)
}
Expand Down
14 changes: 3 additions & 11 deletions lib/rdbms/scan/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type ScannerConfig struct {

type ScanAdapter interface {
ParsePrimaryKeyValue(columnName string, value string) (any, error)
BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool, batchSize uint) (string, []any, error)
BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool, batchSize uint) (string, []any)
ParseRow(row []any) error
}

Expand Down Expand Up @@ -105,16 +105,8 @@ func (s *Scanner) Next() ([]map[string]any, error) {
}

func (s *Scanner) scan() ([]map[string]any, error) {
query, parameters, err := s.adapter.BuildQuery(s.primaryKeys.Keys(), s.isFirstBatch, s.batchSize)
if err != nil {
return nil, fmt.Errorf("failed to build scan query: %w", err)
}

logger := slog.With(slog.String("query", query))
if len(parameters) > 0 {
logger = logger.With(slog.Any("parameters", parameters))
}
logger.Info("Scan query")
query, parameters := s.adapter.BuildQuery(s.primaryKeys.Keys(), s.isFirstBatch, s.batchSize)
slog.Info("Scan query", slog.String("query", query), slog.Any("parameters", parameters))

rows, err := retry.WithRetriesAndResult(s.retryCfg, func(_ int, _ error) (*sql.Rows, error) {
return s.db.Query(query, parameters...)
Expand Down
2 changes: 1 addition & 1 deletion lib/rdbms/scan/scan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (m mockAdapter) ParsePrimaryKeyValue(columnName string, value string) (any,
}
}

func (mockAdapter) BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool, batchSize uint) (string, []any, error) {
func (mockAdapter) BuildQuery(primaryKeys []primary_key.Key, isFirstBatch bool, batchSize uint) (string, []any) {
panic("not implemented")
}

Expand Down

0 comments on commit 7e6d149

Please sign in to comment.