Python query param limit (sqlc-dev#1530)

danicc097 · web-flow · commit 1e40dc828404 · 2022-08-29T08:38:11.000-07:00
* python query param limit support

* refactor DefaultQueryParameterLimit

update pluginPythonCode

unexported default limit

* refactor to optional query_parameter_limit proto

* 0 has no limit, &lt;0 invalid

* revert to default python qpl of 4
diff --git a/examples/python/sqlc.json b/examples/python/sqlc.json
@@ -10,7 +10,8 @@
           "out": "src/authors",
           "package": "authors",
           "emit_sync_querier": true,
-          "emit_async_querier": true
+          "emit_async_querier": true,
+          "query_parameter_limit": 5
         }
       }
     },
@@ -22,7 +23,8 @@
         "python": {
           "out": "src/booktest",
           "package": "booktest",
-          "emit_async_querier": true
+          "emit_async_querier": true,
+          "query_parameter_limit": 5
         }
       }
     },
@@ -34,7 +36,8 @@
         "python": {
           "out": "src/jets",
           "package": "jets",
-          "emit_async_querier": true
+          "emit_async_querier": true,
+          "query_parameter_limit": 5
         }
       }
     },
@@ -46,7 +49,8 @@
         "python": {
           "out": "src/ondeck",
           "package": "ondeck",
-          "emit_async_querier": true
+          "emit_async_querier": true,
+          "query_parameter_limit": 5
         }
       }
     }
diff --git a/internal/cmd/shim.go b/internal/cmd/shim.go
@@ -85,6 +85,7 @@ func pluginPythonCode(s config.SQLPython) *plugin.PythonCode {
 		EmitSyncQuerier:     s.EmitSyncQuerier,
 		EmitAsyncQuerier:    s.EmitAsyncQuerier,
 		EmitPydanticModels:  s.EmitPydanticModels,
+		QueryParameterLimit: s.QueryParameterLimit,
 	}
 }
 
diff --git a/internal/codegen/python/gen.go b/internal/codegen/python/gen.go
@@ -400,7 +400,11 @@ func buildQueries(req *plugin.CodeGenRequest, structs []Struct) ([]Query, error)
 			SourceName:   query.Filename,
 		}
 
-		if len(query.Params) > 4 {
+		qpl := 4
+		if req.Settings.Python.QueryParameterLimit != nil {
+			qpl = int(*req.Settings.Python.QueryParameterLimit)
+		}
+		if len(query.Params) > qpl || qpl == 0 {
 			var cols []pyColumn
 			for _, p := range query.Params {
 				cols = append(cols, pyColumn{
diff --git a/internal/config/config.go b/internal/config/config.go
@@ -169,6 +169,7 @@ type SQLPython struct {
 	Out                 string     `json:"out" yaml:"out"`
 	Overrides           []Override `json:"overrides,omitempty" yaml:"overrides"`
 	EmitPydanticModels  bool       `json:"emit_pydantic_models,omitempty" yaml:"emit_pydantic_models"`
+	QueryParameterLimit *int32     `json:"query_parameter_limit,omitempty" yaml:"query_parameter_limit"`
 }
 
 type SQLJSON struct {
@@ -195,6 +196,8 @@ var ErrPluginNoType = errors.New("plugin: field `process` or `wasm` required")
 var ErrPluginBothTypes = errors.New("plugin: both `process` and `wasm` cannot both be defined")
 var ErrPluginProcessNoCmd = errors.New("plugin: missing process command")
 
+var ErrInvalidQueryParameterLimit = errors.New("invalid query parameter limit")
+
 func ParseConfig(rd io.Reader) (Config, error) {
 	var buf bytes.Buffer
 	var config Config
diff --git a/internal/config/v_one.go b/internal/config/v_one.go
@@ -84,7 +84,9 @@ func v1ParseConfig(rd io.Reader) (Config, error) {
 		if settings.Packages[j].Engine == "" {
 			settings.Packages[j].Engine = EnginePostgreSQL
 		}
+
 	}
+
 	return settings.Translate(), nil
 }
 
diff --git a/internal/config/v_two.go b/internal/config/v_two.go
@@ -36,10 +36,10 @@ func v2ParseConfig(rd io.Reader) (Config, error) {
 	}
 	// TODO: Store built-in plugins somewhere else
 	builtins := map[string]struct{}{
-		"go":     struct{}{},
-		"json":   struct{}{},
-		"kotlin": struct{}{},
-		"python": struct{}{},
+		"go":     {},
+		"json":   {},
+		"kotlin": {},
+		"python": {},
 	}
 	plugins := map[string]struct{}{}
 	for i := range conf.Plugins {
@@ -91,6 +91,11 @@ func v2ParseConfig(rd io.Reader) (Config, error) {
 			}
 		}
 		if conf.SQL[j].Gen.Python != nil {
+			if conf.SQL[j].Gen.Python.QueryParameterLimit != nil {
+				if *conf.SQL[j].Gen.Python.QueryParameterLimit < 0 {
+					return conf, ErrInvalidQueryParameterLimit
+				}
+			}
 			if conf.SQL[j].Gen.Python.Out == "" {
 				return conf, ErrNoOutPath
 			}
diff --git a/internal/endtoend/testdata/query_parameter_limit/-1/python_postgresql/query.sql b/internal/endtoend/testdata/query_parameter_limit/-1/python_postgresql/query.sql
@@ -0,0 +1,7 @@
+CREATE TABLE bar (id serial not null, name text not null, primary key (id));
+
+-- name: DeleteBarByID :execrows
+DELETE FROM bar WHERE id = $1;
+
+-- name: DeleteBarByIDAndName :execrows
+DELETE FROM bar WHERE id = $1 AND name = $2;
diff --git a/internal/endtoend/testdata/query_parameter_limit/-1/python_postgresql/sqlc.json b/internal/endtoend/testdata/query_parameter_limit/-1/python_postgresql/sqlc.json
@@ -0,0 +1,19 @@
+{
+  "version": "2",
+  "sql": [
+    {
+      "schema": "query.sql",
+      "queries": "query.sql",
+      "engine": "postgresql",
+      "gen": {
+        "python": {
+          "out": "python",
+          "package": "querytest",
+          "emit_sync_querier": true,
+          "emit_async_querier": true,
+          "query_parameter_limit": -1
+        }
+      }
+    }
+  ]
+}
diff --git a/internal/endtoend/testdata/query_parameter_limit/-1/python_postgresql/stderr.txt b/internal/endtoend/testdata/query_parameter_limit/-1/python_postgresql/stderr.txt
@@ -0,0 +1 @@
+error parsing sqlc.json: invalid query parameter limit
diff --git a/internal/endtoend/testdata/query_parameter_limit/0/python_postgresql/python/models.py b/internal/endtoend/testdata/query_parameter_limit/0/python_postgresql/python/models.py
@@ -0,0 +1,10 @@
+# Code generated by sqlc. DO NOT EDIT.
+# versions:
+#   sqlc v1.15.0
+import dataclasses
+
+
+@dataclasses.dataclass()
+class Bar:
+    id: int
+    name: str
diff --git a/internal/endtoend/testdata/query_parameter_limit/0/python_postgresql/python/query.py b/internal/endtoend/testdata/query_parameter_limit/0/python_postgresql/python/query.py
@@ -0,0 +1,57 @@
+# Code generated by sqlc. DO NOT EDIT.
+# versions:
+#   sqlc v1.15.0
+# source: query.sql
+import dataclasses
+
+import sqlalchemy
+import sqlalchemy.ext.asyncio
+
+from querytest import models
+
+
+DELETE_BAR_BY_ID = """-- name: delete_bar_by_id \\:execrows
+DELETE FROM bar WHERE id = :p1
+"""
+
+
+@dataclasses.dataclass()
+class DeleteBarByIDParams:
+    id: int
+
+
+DELETE_BAR_BY_ID_AND_NAME = """-- name: delete_bar_by_id_and_name \\:execrows
+DELETE FROM bar WHERE id = :p1 AND name = :p2
+"""
+
+
+@dataclasses.dataclass()
+class DeleteBarByIDAndNameParams:
+    id: int
+    name: str
+
+
+class Querier:
+    def __init__(self, conn: sqlalchemy.engine.Connection):
+        self._conn = conn
+
+    def delete_bar_by_id(self, arg: DeleteBarByIDParams) -> int:
+        result = self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID), {"p1": arg.id})
+        return result.rowcount
+
+    def delete_bar_by_id_and_name(self, arg: DeleteBarByIDAndNameParams) -> int:
+        result = self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID_AND_NAME), {"p1": arg.id, "p2": arg.name})
+        return result.rowcount
+
+
+class AsyncQuerier:
+    def __init__(self, conn: sqlalchemy.ext.asyncio.AsyncConnection):
+        self._conn = conn
+
+    async def delete_bar_by_id(self, arg: DeleteBarByIDParams) -> int:
+        result = await self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID), {"p1": arg.id})
+        return result.rowcount
+
+    async def delete_bar_by_id_and_name(self, arg: DeleteBarByIDAndNameParams) -> int:
+        result = await self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID_AND_NAME), {"p1": arg.id, "p2": arg.name})
+        return result.rowcount
diff --git a/internal/endtoend/testdata/query_parameter_limit/0/python_postgresql/query.sql b/internal/endtoend/testdata/query_parameter_limit/0/python_postgresql/query.sql
@@ -0,0 +1,7 @@
+CREATE TABLE bar (id serial not null, name text not null, primary key (id));
+
+-- name: DeleteBarByID :execrows
+DELETE FROM bar WHERE id = $1;
+
+-- name: DeleteBarByIDAndName :execrows
+DELETE FROM bar WHERE id = $1 AND name = $2;
diff --git a/internal/endtoend/testdata/query_parameter_limit/0/python_postgresql/sqlc.json b/internal/endtoend/testdata/query_parameter_limit/0/python_postgresql/sqlc.json
@@ -0,0 +1,19 @@
+{
+  "version": "2",
+  "sql": [
+    {
+      "schema": "query.sql",
+      "queries": "query.sql",
+      "engine": "postgresql",
+      "gen": {
+        "python": {
+          "out": "python",
+          "package": "querytest",
+          "emit_sync_querier": true,
+          "emit_async_querier": true,
+          "query_parameter_limit": 0
+        }
+      }
+    }
+  ]
+}
diff --git a/internal/endtoend/testdata/query_parameter_limit/2/python_postgresql/python/models.py b/internal/endtoend/testdata/query_parameter_limit/2/python_postgresql/python/models.py
@@ -0,0 +1,10 @@
+# Code generated by sqlc. DO NOT EDIT.
+# versions:
+#   sqlc v1.15.0
+import dataclasses
+
+
+@dataclasses.dataclass()
+class Bar:
+    id: int
+    name: str
diff --git a/internal/endtoend/testdata/query_parameter_limit/2/python_postgresql/python/query.py b/internal/endtoend/testdata/query_parameter_limit/2/python_postgresql/python/query.py
@@ -0,0 +1,44 @@
+# Code generated by sqlc. DO NOT EDIT.
+# versions:
+#   sqlc v1.15.0
+# source: query.sql
+import sqlalchemy
+import sqlalchemy.ext.asyncio
+
+from querytest import models
+
+
+DELETE_BAR_BY_ID = """-- name: delete_bar_by_id \\:execrows
+DELETE FROM bar WHERE id = :p1
+"""
+
+
+DELETE_BAR_BY_ID_AND_NAME = """-- name: delete_bar_by_id_and_name \\:execrows
+DELETE FROM bar WHERE id = :p1 AND name = :p2
+"""
+
+
+class Querier:
+    def __init__(self, conn: sqlalchemy.engine.Connection):
+        self._conn = conn
+
+    def delete_bar_by_id(self, *, id: int) -> int:
+        result = self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID), {"p1": id})
+        return result.rowcount
+
+    def delete_bar_by_id_and_name(self, *, id: int, name: str) -> int:
+        result = self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID_AND_NAME), {"p1": id, "p2": name})
+        return result.rowcount
+
+
+class AsyncQuerier:
+    def __init__(self, conn: sqlalchemy.ext.asyncio.AsyncConnection):
+        self._conn = conn
+
+    async def delete_bar_by_id(self, *, id: int) -> int:
+        result = await self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID), {"p1": id})
+        return result.rowcount
+
+    async def delete_bar_by_id_and_name(self, *, id: int, name: str) -> int:
+        result = await self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID_AND_NAME), {"p1": id, "p2": name})
+        return result.rowcount
diff --git a/internal/endtoend/testdata/query_parameter_limit/2/python_postgresql/query.sql b/internal/endtoend/testdata/query_parameter_limit/2/python_postgresql/query.sql
@@ -0,0 +1,7 @@
+CREATE TABLE bar (id serial not null, name text not null, primary key (id));
+
+-- name: DeleteBarByID :execrows
+DELETE FROM bar WHERE id = $1;
+
+-- name: DeleteBarByIDAndName :execrows
+DELETE FROM bar WHERE id = $1 AND name = $2;
diff --git a/internal/endtoend/testdata/query_parameter_limit/2/python_postgresql/sqlc.json b/internal/endtoend/testdata/query_parameter_limit/2/python_postgresql/sqlc.json
@@ -0,0 +1,19 @@
+{
+  "version": "2",
+  "sql": [
+    {
+      "schema": "query.sql",
+      "queries": "query.sql",
+      "engine": "postgresql",
+      "gen": {
+        "python": {
+          "out": "python",
+          "package": "querytest",
+          "emit_sync_querier": true,
+          "emit_async_querier": true,
+          "query_parameter_limit": 2
+        }
+      }
+    }
+  ]
+}
diff --git a/internal/endtoend/testdata/query_parameter_limit/undefined/python_postgresql/python/models.py b/internal/endtoend/testdata/query_parameter_limit/undefined/python_postgresql/python/models.py
@@ -0,0 +1,12 @@
+# Code generated by sqlc. DO NOT EDIT.
+# versions:
+#   sqlc v1.15.0
+import dataclasses
+
+
+@dataclasses.dataclass()
+class Bar:
+    id: int
+    name1: str
+    name2: str
+    name3: str
diff --git a/internal/endtoend/testdata/query_parameter_limit/undefined/python_postgresql/python/query.py b/internal/endtoend/testdata/query_parameter_limit/undefined/python_postgresql/python/query.py
@@ -0,0 +1,58 @@
+# Code generated by sqlc. DO NOT EDIT.
+# versions:
+#   sqlc v1.15.0
+# source: query.sql
+import sqlalchemy
+import sqlalchemy.ext.asyncio
+
+from querytest import models
+
+
+DELETE_BAR_BY_ID = """-- name: delete_bar_by_id \\:execrows
+DELETE FROM bar WHERE id = :p1
+"""
+
+
+DELETE_BAR_BY_ID_AND_NAME = """-- name: delete_bar_by_id_and_name \\:execrows
+DELETE FROM bar
+WHERE id = :p1
+AND name1 = :p2
+AND name2 = :p3
+AND name3 = :p4
+"""
+
+
+class Querier:
+    def __init__(self, conn: sqlalchemy.engine.Connection):
+        self._conn = conn
+
+    def delete_bar_by_id(self, *, id: int) -> int:
+        result = self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID), {"p1": id})
+        return result.rowcount
+
+    def delete_bar_by_id_and_name(self, *, id: int, name1: str, name2: str, name3: str) -> int:
+        result = self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID_AND_NAME), {
+            "p1": id,
+            "p2": name1,
+            "p3": name2,
+            "p4": name3,
+        })
+        return result.rowcount
+
+
+class AsyncQuerier:
+    def __init__(self, conn: sqlalchemy.ext.asyncio.AsyncConnection):
+        self._conn = conn
+
+    async def delete_bar_by_id(self, *, id: int) -> int:
+        result = await self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID), {"p1": id})
+        return result.rowcount
+
+    async def delete_bar_by_id_and_name(self, *, id: int, name1: str, name2: str, name3: str) -> int:
+        result = await self._conn.execute(sqlalchemy.text(DELETE_BAR_BY_ID_AND_NAME), {
+            "p1": id,
+            "p2": name1,
+            "p3": name2,
+            "p4": name3,
+        })
+        return result.rowcount
diff --git a/internal/endtoend/testdata/query_parameter_limit/undefined/python_postgresql/query.sql b/internal/endtoend/testdata/query_parameter_limit/undefined/python_postgresql/query.sql
diff --git a/internal/endtoend/testdata/query_parameter_limit/undefined/python_postgresql/sqlc.json b/internal/endtoend/testdata/query_parameter_limit/undefined/python_postgresql/sqlc.json
diff --git a/internal/plugin/codegen.pb.go b/internal/plugin/codegen.pb.go
diff --git a/internal/plugin/codegen_vtproto.pb.go b/internal/plugin/codegen_vtproto.pb.go
diff --git a/internal/python/ast/ast.pb.go b/internal/python/ast/ast.pb.go
diff --git a/protos/plugin/codegen.proto b/protos/plugin/codegen.proto

Original file line number	Diff line number	Diff line change
`@@ -10,7 +10,8 @@`
`10`	`10`	`"out": "src/authors",`
`11`	`11`	`"package": "authors",`
`12`	`12`	`"emit_sync_querier": true,`
`13`		`- "emit_async_querier": true`
	`13`	`+ "emit_async_querier": true,`
	`14`	`+ "query_parameter_limit": 5`
`14`	`15`	`}`
`15`	`16`	`}`
`16`	`17`	`},`
`@@ -22,7 +23,8 @@`
`22`	`23`	`"python": {`
`23`	`24`	`"out": "src/booktest",`
`24`	`25`	`"package": "booktest",`
`25`		`- "emit_async_querier": true`
	`26`	`+ "emit_async_querier": true,`
	`27`	`+ "query_parameter_limit": 5`
`26`	`28`	`}`
`27`	`29`	`}`
`28`	`30`	`},`
`@@ -34,7 +36,8 @@`
`34`	`36`	`"python": {`
`35`	`37`	`"out": "src/jets",`
`36`	`38`	`"package": "jets",`
`37`		`- "emit_async_querier": true`
	`39`	`+ "emit_async_querier": true,`
	`40`	`+ "query_parameter_limit": 5`
`38`	`41`	`}`
`39`	`42`	`}`
`40`	`43`	`},`
`@@ -46,7 +49,8 @@`
`46`	`49`	`"python": {`
`47`	`50`	`"out": "src/ondeck",`
`48`	`51`	`"package": "ondeck",`
`49`		`- "emit_async_querier": true`
	`52`	`+ "emit_async_querier": true,`
	`53`	`+ "query_parameter_limit": 5`
`50`	`54`	`}`
`51`	`55`	`}`
`52`	`56`	`}`
Original file line number	Diff line number	Diff line change
`@@ -85,6 +85,7 @@ func pluginPythonCode(s config.SQLPython) *plugin.PythonCode {`
`85`	`85`	`EmitSyncQuerier: s.EmitSyncQuerier,`
`86`	`86`	`EmitAsyncQuerier: s.EmitAsyncQuerier,`
`87`	`87`	`EmitPydanticModels: s.EmitPydanticModels,`
	`88`	`+ QueryParameterLimit: s.QueryParameterLimit,`
`88`	`89`	`}`
`89`	`90`	`}`
`90`	`91`
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,9 @@ func v1ParseConfig(rd io.Reader) (Config, error) {`
`84`	`84`	`if settings.Packages[j].Engine == "" {`
`85`	`85`	`settings.Packages[j].Engine = EnginePostgreSQL`
`86`	`86`	`}`
	`87`	`+`
`87`	`88`	`}`
	`89`	`+`
`88`	`90`	`return settings.Translate(), nil`
`89`	`91`	`}`
`90`	`92`
Original file line number	Diff line number	Diff line change
`@@ -36,10 +36,10 @@ func v2ParseConfig(rd io.Reader) (Config, error) {`
`36`	`36`	`}`
`37`	`37`	`// TODO: Store built-in plugins somewhere else`
`38`	`38`	`builtins := map[string]struct{}{`
`39`		`- "go": struct{}{},`
`40`		`- "json": struct{}{},`
`41`		`- "kotlin": struct{}{},`
`42`		`- "python": struct{}{},`
	`39`	`+ "go": {},`
	`40`	`+ "json": {},`
	`41`	`+ "kotlin": {},`
	`42`	`+ "python": {},`
`43`	`43`	`}`
`44`	`44`	`plugins := map[string]struct{}{}`
`45`	`45`	`for i := range conf.Plugins {`
`@@ -91,6 +91,11 @@ func v2ParseConfig(rd io.Reader) (Config, error) {`
`91`	`91`	`}`
`92`	`92`	`}`
`93`	`93`	`if conf.SQL[j].Gen.Python != nil {`
	`94`	`+ if conf.SQL[j].Gen.Python.QueryParameterLimit != nil {`
	`95`	`+ if *conf.SQL[j].Gen.Python.QueryParameterLimit < 0 {`
	`96`	`+ return conf, ErrInvalidQueryParameterLimit`
	`97`	`+ }`
	`98`	`+ }`
`94`	`99`	`if conf.SQL[j].Gen.Python.Out == "" {`
`95`	`100`	`return conf, ErrNoOutPath`
`96`	`101`	`}`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+error parsing sqlc.json: invalid query parameter limit`