Introduce Selector interface for generating column expressions

This is response to code review feedback, add a new `Selector `interface
whose job it is to provide an engine-agnostic way of generating output
expressions for when selecting column values with `SELECT ...` or
`RETURNING ...`. This is exclusively needed for SQLite for the time
being, which uses it to wrap all output `jsonb` column values with a
call to `json(...)` so that values are coerced to a publicly usable
format before being returned.

[1] #3968 (comment)

Author: brandur

internal/compiler/engine.go

@@ -13,6 +13,7 @@ import (
 	"github.com/sqlc-dev/sqlc/internal/engine/sqlite"
 	"github.com/sqlc-dev/sqlc/internal/opts"
 	"github.com/sqlc-dev/sqlc/internal/sql/catalog"
+	"github.com/sqlc-dev/sqlc/internal/sql/selector"
 )
 
 type Compiler struct {
@@ -23,6 +24,7 @@ type Compiler struct {
 	result   *Result
 	analyzer analyzer.Analyzer
 	client   dbmanager.Client
+	selector selector.Selector
 
 	schema []string
 }
@@ -39,12 +41,15 @@ func NewCompiler(conf config.SQL, combo config.CombinedSettings) (*Compiler, err
 	case config.EngineSQLite:
 		c.parser = sqlite.NewParser()
 		c.catalog = sqlite.NewCatalog()
+		c.selector = sqlite.NewSelector()
 	case config.EngineMySQL:
 		c.parser = dolphin.NewParser()
 		c.catalog = dolphin.NewCatalog()
+		c.selector = selector.NewDefaultSelector()
 	case config.EnginePostgreSQL:
 		c.parser = postgresql.NewParser()
 		c.catalog = postgresql.NewCatalog()
+		c.selector = selector.NewDefaultSelector()
 		if conf.Database != nil {
 			if conf.Analyzer.Database == nil || *conf.Analyzer.Database {
 				c.analyzer = analyzer.Cached(

internal/compiler/expand.go

@@ -6,7 +6,6 @@ import (
 	"strings"
 
 	"github.com/sqlc-dev/sqlc/internal/config"
-	"github.com/sqlc-dev/sqlc/internal/engine/sqlite"
 	"github.com/sqlc-dev/sqlc/internal/source"
 	"github.com/sqlc-dev/sqlc/internal/sql/ast"
 	"github.com/sqlc-dev/sqlc/internal/sql/astutils"
@@ -150,17 +149,11 @@ func (c *Compiler) expandStmt(qc *QueryCatalog, raw *ast.RawStmt, node ast.Node)
 				if counts[cname] > 1 {
 					cname = tableName + "." + cname
 				}
-				// Under SQLite, neither json nor jsonb are real data types, and
-				// rather just of type blob, so database drivers just return
-				// whatever raw binary is stored as values. This is a problem
-				// for jsonb, which is considered an internal format to SQLite
-				// and no attempt should be made to parse it outside of the
-				// database itself. For jsonb columns in SQLite, wrap returned
-				// columns in `json(col)` to coerce the internal binary format
-				// to JSON parsable by the user-space application.
-				if _, ok := c.parser.(*sqlite.Parser); ok && column.DataType == "jsonb" {
-					cname = "json(" + cname + ")"
-				}
+
+				// This is important for SQLite in particular which needs to
+				// wrap jsonb column values with `json(colname)` so they're in a
+				// publicly usable format (i.e. not jsonb).
+				cname = c.selector.ColumnExpr(cname, column.DataType)
 				cols = append(cols, cname)
 			}
 		}

internal/engine/sqlite/selector.go

@@ -0,0 +1,21 @@
+package sqlite
+
+type Selector struct{}
+
+func NewSelector() *Selector {
+	return &Selector{}
+}
+
+func (s *Selector) ColumnExpr(name string, dataType string) string {
+	// Under SQLite, neither json nor jsonb are real data types, and rather just
+	// of type blob, so database drivers just return whatever raw binary is
+	// stored as values. This is a problem for jsonb, which is considered an
+	// internal format to SQLite and no attempt should be made to parse it
+	// outside of the database itself. For jsonb columns in SQLite, wrap values
+	// in `json(col)` to coerce the internal binary format to JSON parsable by
+	// the user-space application.
+	if dataType == "jsonb" {
+		return "json(" + name + ")"
+	}
+	return name
+}

internal/engine/sqlite/selector_test.go

@@ -0,0 +1,20 @@
+package sqlite
+
+import "testing"
+
+func TestSelectorColumnExpr(t *testing.T) {
+	t.Parallel()
+
+	selector := NewSelector()
+
+	expectExpr := func(expected, name, dataType string) {
+		if actual := selector.ColumnExpr(name, dataType); expected != actual {
+			t.Errorf("Expected %v, got %v for data type %v", expected, actual, dataType)
+		}
+	}
+
+	expectExpr("my_column", "my_column", "integer")
+	expectExpr("my_column", "my_column", "json")
+	expectExpr("json(my_column)", "my_column", "jsonb")
+	expectExpr("my_column", "my_column", "text")
+}

internal/sql/selector/selector.go

@@ -0,0 +1,26 @@
+package selector
+
+// Selector is an interface used by a compiler for generating expressions for
+// output columns in a `SELECT ...` or `RETURNING ...` statement.
+//
+// This interface is exclusively needed at the moment for SQLite, which must
+// wrap output `jsonb` columns with a `json(column_name)` invocation so that a
+// publicly consumable format (i.e. not jsonb) is returned.
+type Selector interface {
+	// ColumnExpr generates output to be used in a `SELECT ...` or `RETURNING
+	// ...` statement based on input column name and metadata.
+	ColumnExpr(name string, dataType string) string
+}
+
+// DefaultSelector is a Selector implementation that does the simpliest possible
+// pass through when generating column expressions. Its use is suitable for all
+// database engines not requiring additional customization.
+type DefaultSelector struct{}
+
+func NewDefaultSelector() *DefaultSelector {
+	return &DefaultSelector{}
+}
+
+func (s *DefaultSelector) ColumnExpr(name string, dataType string) string {
+	return name
+}

internal/sql/selector/selector_test.go

@@ -0,0 +1,20 @@
+package selector
+
+import "testing"
+
+func TestDefaultSelectorColumnExpr(t *testing.T) {
+	t.Parallel()
+
+	selector := NewDefaultSelector()
+
+	expectExpr := func(expected, name, dataType string) {
+		if actual := selector.ColumnExpr(name, dataType); expected != actual {
+			t.Errorf("Expected %v, got %v for data type %v", expected, actual, dataType)
+		}
+	}
+
+	expectExpr("my_column", "my_column", "integer")
+	expectExpr("my_column", "my_column", "json")
+	expectExpr("my_column", "my_column", "jsonb")
+	expectExpr("my_column", "my_column", "text")
+}