Skip to content

Commit c62c6e7

Browse files
committed
Make selectors internal to the compiler package
This is based on original code review feedback that I'd misread initially. The selector interface doesn't need to be an outside package for any reason (it's used only internal to the compiler), and this lets us improve it somewhat by taking a full `*Column` struct rather than having to make it a `dataType string` (because `Column` is internal to `compiler` and it would otherwise introduce dependency cycles).
1 parent 0555230 commit c62c6e7

File tree

8 files changed

+86
-93
lines changed

8 files changed

+86
-93
lines changed

internal/compiler/engine.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/sqlc-dev/sqlc/internal/engine/sqlite"
1414
"github.com/sqlc-dev/sqlc/internal/opts"
1515
"github.com/sqlc-dev/sqlc/internal/sql/catalog"
16-
"github.com/sqlc-dev/sqlc/internal/sql/selector"
1716
)
1817

1918
type Compiler struct {
@@ -24,7 +23,7 @@ type Compiler struct {
2423
result *Result
2524
analyzer analyzer.Analyzer
2625
client dbmanager.Client
27-
selector selector.Selector
26+
selector selector
2827

2928
schema []string
3029
}
@@ -41,15 +40,15 @@ func NewCompiler(conf config.SQL, combo config.CombinedSettings) (*Compiler, err
4140
case config.EngineSQLite:
4241
c.parser = sqlite.NewParser()
4342
c.catalog = sqlite.NewCatalog()
44-
c.selector = sqlite.NewSelector()
43+
c.selector = newSQLiteSelector()
4544
case config.EngineMySQL:
4645
c.parser = dolphin.NewParser()
4746
c.catalog = dolphin.NewCatalog()
48-
c.selector = selector.NewDefaultSelector()
47+
c.selector = newDefaultSelector()
4948
case config.EnginePostgreSQL:
5049
c.parser = postgresql.NewParser()
5150
c.catalog = postgresql.NewCatalog()
52-
c.selector = selector.NewDefaultSelector()
51+
c.selector = newDefaultSelector()
5352
if conf.Database != nil {
5453
if conf.Analyzer.Database == nil || *conf.Analyzer.Database {
5554
c.analyzer = analyzer.Cached(

internal/compiler/expand.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ func (c *Compiler) expandStmt(qc *QueryCatalog, raw *ast.RawStmt, node ast.Node)
153153
// This is important for SQLite in particular which needs to
154154
// wrap jsonb column values with `json(colname)` so they're in a
155155
// publicly usable format (i.e. not jsonb).
156-
cname = c.selector.ColumnExpr(cname, column.DataType)
156+
cname = c.selector.ColumnExpr(cname, column)
157157
cols = append(cols, cname)
158158
}
159159
}

internal/compiler/selector.go

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package compiler
2+
3+
// selector is an interface used by a compiler for generating expressions for
4+
// output columns in a `SELECT ...` or `RETURNING ...` statement.
5+
//
6+
// This interface is exclusively needed at the moment for SQLite, which must
7+
// wrap output `jsonb` columns with a `json(column_name)` invocation so that a
8+
// publicly consumable format (i.e. not jsonb) is returned.
9+
type selector interface {
10+
// ColumnExpr generates output to be used in a `SELECT ...` or `RETURNING
11+
// ...` statement based on input column name and metadata.
12+
ColumnExpr(name string, column *Column) string
13+
}
14+
15+
// defaultSelector is a selector implementation that does the simpliest possible
16+
// pass through when generating column expressions. Its use is suitable for all
17+
// database engines not requiring additional customization.
18+
type defaultSelector struct{}
19+
20+
func newDefaultSelector() *defaultSelector {
21+
return &defaultSelector{}
22+
}
23+
24+
func (s *defaultSelector) ColumnExpr(name string, column *Column) string {
25+
return name
26+
}
27+
28+
type sqliteSelector struct{}
29+
30+
func newSQLiteSelector() *sqliteSelector {
31+
return &sqliteSelector{}
32+
}
33+
34+
func (s *sqliteSelector) ColumnExpr(name string, column *Column) string {
35+
// Under SQLite, neither json nor jsonb are real data types, and rather just
36+
// of type blob, so database drivers just return whatever raw binary is
37+
// stored as values. This is a problem for jsonb, which is considered an
38+
// internal format to SQLite and no attempt should be made to parse it
39+
// outside of the database itself. For jsonb columns in SQLite, wrap values
40+
// in `json(col)` to coerce the internal binary format to JSON parsable by
41+
// the user-space application.
42+
if column.DataType == "jsonb" {
43+
return "json(" + name + ")"
44+
}
45+
return name
46+
}

internal/compiler/selector_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package compiler
2+
3+
import "testing"
4+
5+
func TestSelector(t *testing.T) {
6+
t.Parallel()
7+
8+
selectorExpectColumnExpr := func(t *testing.T, selector selector, expected, name string, column *Column) {
9+
if actual := selector.ColumnExpr(name, column); expected != actual {
10+
t.Errorf("Expected %v, got %v for data type %v", expected, actual, column.DataType)
11+
}
12+
}
13+
14+
t.Run("DefaultSelectorColumnExpr", func(t *testing.T) {
15+
t.Parallel()
16+
17+
selector := newDefaultSelector()
18+
19+
selectorExpectColumnExpr(t, selector, "my_column", "my_column", &Column{DataType: "integer"})
20+
selectorExpectColumnExpr(t, selector, "my_column", "my_column", &Column{DataType: "json"})
21+
selectorExpectColumnExpr(t, selector, "my_column", "my_column", &Column{DataType: "jsonb"})
22+
selectorExpectColumnExpr(t, selector, "my_column", "my_column", &Column{DataType: "text"})
23+
})
24+
25+
t.Run("SQLiteSelectorColumnExpr", func(t *testing.T) {
26+
t.Parallel()
27+
28+
selector := newSQLiteSelector()
29+
30+
selectorExpectColumnExpr(t, selector, "my_column", "my_column", &Column{DataType: "integer"})
31+
selectorExpectColumnExpr(t, selector, "my_column", "my_column", &Column{DataType: "json"})
32+
selectorExpectColumnExpr(t, selector, "json(my_column)", "my_column", &Column{DataType: "jsonb"})
33+
selectorExpectColumnExpr(t, selector, "my_column", "my_column", &Column{DataType: "text"})
34+
})
35+
}

internal/engine/sqlite/selector.go

Lines changed: 0 additions & 21 deletions
This file was deleted.

internal/engine/sqlite/selector_test.go

Lines changed: 0 additions & 20 deletions
This file was deleted.

internal/sql/selector/selector.go

Lines changed: 0 additions & 26 deletions
This file was deleted.

internal/sql/selector/selector_test.go

Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

Comments
 (0)