Skip to content

Commit 993e5a1

Browse files
zachmuzachmu
authored
Merge pull request #2041 from dolthub/zachm/drizzle-2
Fixed a couple issues with prepared statements
2 parents 9d275c5 + c99183c commit 993e5a1

File tree

5 files changed

+150
-7
lines changed

5 files changed

+150
-7
lines changed

‎server/analyzer/type_sanitizer.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,9 @@ import (
4040
// to GMS types, so by taking care of all conversions here, we can ensure that Doltgres only needs to worry about its
4141
// own types.
4242
func TypeSanitizer(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, scope *plan.Scope, selector analyzer.RuleSelector, qFlags *sql.QueryFlags) (sql.Node, transform.TreeIdentity, error) {
43+
// TODO: this probably should not be opaque, we should let the analyzer dig into subqueries and analyze them when
44+
// it chooses. Doing all type transformations upfront like this masks bugs where certain tyupe conversion errors
45+
// only manifest in a subquery
4346
return pgtransform.NodeExprsWithNodeWithOpaque(node, func(n sql.Node, expr sql.Expression) (sql.Expression, transform.TreeIdentity, error) {
4447
// This can be updated if we find more expressions that return GMS types.
4548
// These should eventually be replaced with Doltgres-equivalents over time, rendering this function unnecessary.
@@ -58,7 +61,12 @@ func TypeSanitizer(ctx *sql.Context, a *analyzer.Analyzer, node sql.Node, scope
5861
}
5962
return expr, transform.SameTree, nil
6063
case *expression.Literal:
61-
return typeSanitizerLiterals(ctx, expr)
64+
// We want to leave limit literals alone, as they are expected to be GMS types when they appear in certain
65+
// parts of the query (subqueries in particular)
66+
// TODO: fix the limit validation analysis to handle doltgres types
67+
if _, isLimit := n.(*plan.Limit); !isLimit {
68+
return typeSanitizerLiterals(ctx, expr)
69+
}
6270
case *expression.Not, *expression.And, *expression.Or, *expression.Like:
6371
return pgexprs.NewGMSCast(expr), transform.NewTree, nil
6472
case sql.FunctionExpression:

‎server/connection_data.go‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,6 @@ type PreparedStatementData struct {
108108
// the types defined.
109109
func extractBindVarTypes(queryPlan sql.Node) ([]uint32, error) {
110110
inspectNode := queryPlan
111-
switch queryPlan := queryPlan.(type) {
112-
case *plan.InsertInto:
113-
inspectNode = queryPlan.Source
114-
}
115111

116112
types := make(map[string]uint32)
117113
var err error
@@ -197,6 +193,13 @@ func extractBindVarTypes(queryPlan sql.Node) ([]uint32, error) {
197193

198194
transform.InspectExpressionsWithNode(inspectNode, extractBindVars)
199195

196+
// Insert nodes are special, as their source expressions are not returned by the Expressions()
197+
// interface and must be walked separately.
198+
switch queryPlan := queryPlan.(type) {
199+
case *plan.InsertInto:
200+
transform.InspectExpressionsWithNode(queryPlan.Source, extractBindVars)
201+
}
202+
200203
// above finds types of bindvars in unordered form.
201204
// the list of types needs to be ordered as v1, v2, v3, etc.
202205
typesArr := make([]uint32, len(types))
@@ -206,7 +209,7 @@ func extractBindVarTypes(queryPlan sql.Node) ([]uint32, error) {
206209
return nil, errors.Errorf("could not determine the index of placeholder %s: %e", i, err)
207210
}
208211
if int(idx-1) >= len(types) {
209-
return nil, errors.Errorf("could not determine the index of placeholder %s: %e", i, err)
212+
return nil, errors.Errorf("could not determine the index of placeholder %s in slice of %d elements", i, len(types))
210213
}
211214
typesArr[idx-1] = t
212215
}

‎testing/go/insert_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,12 @@ func TestInsert(t *testing.T) {
130130
{2, "world", "world"},
131131
},
132132
},
133+
{
134+
Query: `INSERT INTO t2 (id, c1, c2)
135+
VALUES ($1, $2, $3)
136+
ON CONFLICT (id) do update set c1 = $4`,
137+
BindVars: []any{1, "x", "y", "no conflict expected"},
138+
},
133139
},
134140
},
135141
{

‎testing/go/issues_test.go‎

Lines changed: 122 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@
1414

1515
package _go
1616

17-
import "testing"
17+
import (
18+
"testing"
19+
20+
"github.com/dolthub/go-mysql-server/sql"
21+
)
1822

1923
func TestIssues(t *testing.T) {
2024
RunScripts(t, []ScriptTest{
@@ -51,5 +55,122 @@ func TestIssues(t *testing.T) {
5155
},
5256
},
5357
},
58+
{
59+
Name: "Issue #2030",
60+
SetUpScript: []string{
61+
`CREATE TABLE sub_entities (
62+
project_id VARCHAR(256) NOT NULL,
63+
entity_id VARCHAR(256) NOT NULL,
64+
id VARCHAR(256) NOT NULL,
65+
name VARCHAR(256) NOT NULL,
66+
PRIMARY KEY (project_id, entity_id, id)
67+
);
68+
`,
69+
`
70+
CREATE TABLE entities (
71+
project_id VARCHAR(256) NOT NULL,
72+
id VARCHAR(256) NOT NULL,
73+
name VARCHAR(256) NOT NULL,
74+
default_sub_entity_id VARCHAR(256),
75+
PRIMARY KEY (project_id, id)
76+
);
77+
`,
78+
`
79+
CREATE TABLE conversations (
80+
id VARCHAR(256) NOT NULL,
81+
tenant_id VARCHAR(256) NOT NULL,
82+
project_id VARCHAR(256) NOT NULL,
83+
active_sub_agent_id VARCHAR(256) NOT NULL,
84+
PRIMARY KEY (tenant_id, project_id, id)
85+
);
86+
`,
87+
`INSERT INTO sub_entities (project_id, entity_id, id, name) VALUES
88+
('projectA', 'entityA', 'subA1', 'Sub-Entity A1'),
89+
('projectA', 'entityB', 'subB1', 'Sub-Entity B1');
90+
`,
91+
`INSERT INTO entities (project_id, id, name, default_sub_entity_id) VALUES
92+
('projectA', 'entityA', 'Entity A', 'subA1'),
93+
('projectA', 'entityB', 'Entity B', 'subB1');
94+
`,
95+
`INSERT INTO conversations (tenant_id, project_id, id, active_sub_agent_id) VALUES
96+
('tenant1', 'projectA', 'conv1', 'subA1'),
97+
('tenant1', 'projectA', 'conv2', 'subB1');
98+
`,
99+
},
100+
Assertions: []ScriptTestAssertion{
101+
{
102+
Query: `select
103+
"entities"."project_id",
104+
"entities"."id",
105+
"entities"."name",
106+
"entities"."default_sub_entity_id",
107+
"entities_defaultSubEntity"."data" as "defaultSubEntity"
108+
from "entities" "entities"
109+
left join lateral (
110+
select json_build_array(
111+
"entities_defaultSubEntity"."project_id",
112+
"entities_defaultSubEntity"."entity_id",
113+
"entities_defaultSubEntity"."id",
114+
"entities_defaultSubEntity"."name"
115+
) as "data"
116+
from (
117+
select * from "sub_entities" "entities_defaultSubEntity"
118+
where "entities_defaultSubEntity"."id" = "entities"."default_sub_entity_id"
119+
limit $1
120+
) "entities_defaultSubEntity"
121+
) "entities_defaultSubEntity" on true
122+
where ("entities"."project_id" = $2 and "entities"."id" = $3)
123+
limit $4`,
124+
BindVars: []any{
125+
int64(1),
126+
"projectA",
127+
"entityA",
128+
int64(1),
129+
},
130+
Expected: []sql.Row{
131+
{
132+
"projectA",
133+
"entityA",
134+
"Entity A",
135+
"subA1",
136+
`["projectA", "entityA", "subA1", "Sub-Entity A1"]`,
137+
},
138+
},
139+
},
140+
{
141+
Query: `select
142+
"entities"."project_id",
143+
"entities"."id",
144+
"entities"."name",
145+
"entities"."default_sub_entity_id",
146+
"entities_defaultSubEntity"."data" as "defaultSubEntity"
147+
from "entities" "entities"
148+
left join lateral (
149+
select json_build_array(
150+
"entities_defaultSubEntity"."project_id",
151+
"entities_defaultSubEntity"."entity_id",
152+
"entities_defaultSubEntity"."id",
153+
"entities_defaultSubEntity"."name"
154+
) as "data"
155+
from (
156+
select * from "sub_entities" "entities_defaultSubEntity"
157+
where "entities_defaultSubEntity"."id" = "entities"."default_sub_entity_id"
158+
limit 1
159+
) "entities_defaultSubEntity"
160+
) "entities_defaultSubEntity" on true
161+
where ("entities"."project_id" = 'projectA' and "entities"."id" = 'entityA')
162+
limit 1`,
163+
Expected: []sql.Row{
164+
{
165+
"projectA",
166+
"entityA",
167+
"Entity A",
168+
"subA1",
169+
`["projectA", "entityA", "subA1", "Sub-Entity A1"]`,
170+
},
171+
},
172+
},
173+
},
174+
},
54175
})
55176
}

‎testing/go/limit_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ func TestLimitOffset(t *testing.T) {
3333
Query: `SELECT * FROM t LIMIT 2`,
3434
Expected: []sql.Row{{1, 1}, {2, 2}},
3535
},
36+
{
37+
Query: `SELECT * FROM t LIMIT $1`,
38+
BindVars: []interface{}{int64(2)},
39+
Expected: []sql.Row{{1, 1}, {2, 2}},
40+
},
3641
{
3742
Query: `SELECT * FROM t LIMIT 2 OFFSET 2`,
3843
Expected: []sql.Row{{3, 3}, {4, 4}},

0 commit comments

Comments
 (0)