Skip to content

Commit 29e086c

Browse files
committed
readyset-server: Check that placeholders' columns exist
Migration was not checking that the column connected to a placeholder actually existed, and most confusingly, migration would even succeed! We only hit a problem when querying the resultant cache, which wouldn't work and would return an error that didn't make sense. We now try to check whether the column exists. Release-Note-Core: Check whether placeholders' columns (`zzz = $1`) exist during migration, and if not, report query as unsupported. Fixes: REA-5454 Change-Id: I33fa590bb51b19bc0355ebf13c042dcbebaf2a4c Reviewed-on: https://gerrit.readyset.name/c/readyset/+/9022 Tested-by: Buildkite CI Reviewed-by: Michael Zink <michael.z@readyset.io>
1 parent f9d8589 commit 29e086c

File tree

1 file changed

+19
-0
lines changed

1 file changed

+19
-0
lines changed

readyset-server/src/controller/sql/query_graph.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,6 +1234,25 @@ pub fn to_query_graph(stmt: SelectStatement) -> ReadySetResult<QueryGraph> {
12341234
// we also separately register it as a parameter so that we can set keys
12351235
// correctly on the leaf view
12361236
rel.parameters.push(param.clone());
1237+
} else {
1238+
let found = stmt.fields.iter().any(|f| {
1239+
matches!(
1240+
f,
1241+
FieldDefinitionExpr::Expr {
1242+
alias: None,
1243+
expr: Expr::Column(Column { name, .. }),
1244+
}
1245+
| FieldDefinitionExpr::Expr {
1246+
alias: Some(name), ..
1247+
} if *name == param.col.name
1248+
)
1249+
});
1250+
if !found {
1251+
unsupported!(
1252+
"Column `{}` as placeholder cannot be resolved",
1253+
param.col.name
1254+
);
1255+
}
12371256
}
12381257
}
12391258

0 commit comments

Comments
 (0)