Merge pull request #3 from ModernRelay/claude/debug-optional-param-error-3xQtJ

Add support for null literals in query parameters
This commit is contained in:
Ragnor Comerford 2026-04-13 13:38:47 +02:00 committed by GitHub
commit 54252b21fd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 158 additions and 14 deletions

View file

@ -160,6 +160,7 @@ impl std::fmt::Display for AggFunc {
#[derive(Debug, Clone)]
pub enum Literal {
Null,
String(String),
Integer(i64),
Float(f64),

View file

@ -1436,6 +1436,8 @@ fn resolved_type_to_field_shape(
fn literal_type(lit: &Literal) -> Result<PropType> {
match lit {
// Null is compatible with any nullable type; default to String for inference.
Literal::Null => Ok(PropType::scalar(ScalarType::String, true)),
Literal::String(_) => Ok(PropType::scalar(ScalarType::String, false)),
Literal::Integer(_) => Ok(PropType::scalar(ScalarType::I64, false)),
Literal::Float(_) => Ok(PropType::scalar(ScalarType::F64, false)),
@ -1466,6 +1468,18 @@ fn literal_type(lit: &Literal) -> Result<PropType> {
}
fn check_literal_type(lit: &Literal, expected: &PropType, prop_name: &str) -> Result<()> {
// Null is compatible with any nullable property type.
if matches!(lit, Literal::Null) {
return if expected.nullable {
Ok(())
} else {
Err(NanoError::Type(format!(
"T3: property `{}` is non-nullable but got null",
prop_name
)))
};
}
if !expected.list
&& let ScalarType::Vector(expected_dim) = expected.scalar
&& let Some(actual_dim) = numeric_vector_literal_dim(lit)

View file

@ -270,7 +270,15 @@ pub fn json_params_to_param_map(
let mut map = ParamMap::new();
let object = match params {
Some(Value::Object(object)) => object,
Some(Value::Null) | None => return Ok(map),
Some(Value::Null) | None => {
// Still fill in Literal::Null for declared nullable params.
for param in query_params {
if param.nullable {
map.insert(param.name.clone(), Literal::Null);
}
}
return Ok(map);
}
Some(other) => {
let message = match mode {
JsonParamMode::Standard => "params must be a JSON object".to_string(),
@ -284,12 +292,31 @@ pub fn json_params_to_param_map(
for (key, value) in object {
let decl = query_params.iter().find(|param| param.name == *key);
let literal = if let Some(decl) = decl {
json_value_to_literal_typed(key, value, &decl.type_name, mode)?
if let Some(decl) = decl {
if matches!(value, Value::Null) {
if decl.nullable {
map.insert(key.clone(), Literal::Null);
} else {
return Err(RunInputError::message(format!(
"param '{}': null is not accepted for non-nullable parameter",
key
)));
}
} else {
let literal = json_value_to_literal_typed(key, value, &decl.type_name, mode)?;
map.insert(key.clone(), literal);
}
} else {
json_value_to_literal_inferred(key, value, mode)?
let literal = json_value_to_literal_inferred(key, value, mode)?;
map.insert(key.clone(), literal);
};
map.insert(key.clone(), literal);
}
// Fill in Literal::Null for declared nullable params that were omitted.
for param in query_params {
if param.nullable && !map.contains_key(&param.name) {
map.insert(param.name.clone(), Literal::Null);
}
}
Ok(map)
@ -568,15 +595,7 @@ fn json_value_to_literal_inferred(
}
Ok(Literal::List(out))
}
Value::Null => Err(match mode {
JsonParamMode::Standard => {
RunInputError::message(format!("param '{}': null is not supported", key))
}
JsonParamMode::JavaScript => RunInputError::message(format!(
"param '{}': null values are not supported as query parameters",
key
)),
}),
Value::Null => Ok(Literal::Null),
Value::Object(_) => Err(match mode {
JsonParamMode::Standard => {
RunInputError::message(format!("param '{}': object is not supported", key))
@ -889,4 +908,110 @@ query q($tags: [String], $days: [Date]?, $due_at: DateTime) {
other => panic!("expected date list param, got {:?}", other),
}
}
#[test]
fn nullable_param_omitted_becomes_null() {
let query = find_named_query(
"query q($name: String, $bio: String?) { match { $u: User } return { $u } }",
"q",
)
.expect("query");
let params = json_params_to_param_map(
Some(&json!({ "name": "Alice" })),
&query.params,
JsonParamMode::Standard,
)
.expect("should accept omitted nullable param");
assert!(matches!(params.get("name"), Some(Literal::String(v)) if v == "Alice"));
assert!(matches!(params.get("bio"), Some(Literal::Null)));
}
#[test]
fn nullable_param_explicit_null_becomes_null() {
let query = find_named_query(
"query q($name: String, $bio: String?) { match { $u: User } return { $u } }",
"q",
)
.expect("query");
let params = json_params_to_param_map(
Some(&json!({ "name": "Alice", "bio": null })),
&query.params,
JsonParamMode::Standard,
)
.expect("should accept explicit null for nullable param");
assert!(matches!(params.get("name"), Some(Literal::String(v)) if v == "Alice"));
assert!(matches!(params.get("bio"), Some(Literal::Null)));
}
#[test]
fn non_nullable_param_rejects_null() {
let query = find_named_query(
"query q($name: String) { match { $u: User } return { $u } }",
"q",
)
.expect("query");
let error = json_params_to_param_map(
Some(&json!({ "name": null })),
&query.params,
JsonParamMode::Standard,
)
.expect_err("null for non-nullable param should fail");
assert!(
error
.to_string()
.contains("null is not accepted for non-nullable parameter"),
"unexpected error: {}",
error
);
}
#[test]
fn nullable_param_with_value_works_normally() {
let query = find_named_query(
"query q($bio: String?) { match { $u: User } return { $u } }",
"q",
)
.expect("query");
let params = json_params_to_param_map(
Some(&json!({ "bio": "hello" })),
&query.params,
JsonParamMode::Standard,
)
.expect("should accept string value for nullable param");
assert!(matches!(params.get("bio"), Some(Literal::String(v)) if v == "hello"));
}
#[test]
fn inferred_null_param_becomes_literal_null() {
let params = json_params_to_param_map(
Some(&json!({ "extra": null })),
&[],
JsonParamMode::Standard,
)
.expect("inferred null should succeed");
assert!(matches!(params.get("extra"), Some(Literal::Null)));
}
#[test]
fn nullable_params_filled_when_params_is_none() {
let query = find_named_query(
"query q($bio: String?) { match { $u: User } return { $u } }",
"q",
)
.expect("query");
let params = json_params_to_param_map(None, &query.params, JsonParamMode::Standard)
.expect("None params should succeed with nullable declarations");
assert!(matches!(params.get("bio"), Some(Literal::Null)));
}
}

View file

@ -26,6 +26,7 @@ fn literal_to_typed_array(
num_rows: usize,
) -> Result<ArrayRef> {
Ok(match (lit, data_type) {
(Literal::Null, _) => arrow_array::new_null_array(data_type, num_rows),
(Literal::String(s), DataType::Utf8) => {
Arc::new(StringArray::from(vec![s.as_str(); num_rows])) as ArrayRef
}

View file

@ -74,6 +74,7 @@ fn evaluate_expr(batch: &RecordBatch, expr: &IRExpr, params: &ParamMap) -> Resul
/// Create a constant array from a literal value.
fn literal_to_array(lit: &Literal, num_rows: usize) -> Result<ArrayRef> {
Ok(match lit {
Literal::Null => arrow_array::new_null_array(&DataType::Utf8, num_rows),
Literal::String(s) => Arc::new(StringArray::from(vec![s.as_str(); num_rows])) as ArrayRef,
Literal::Integer(n) => {
// Try to match the most common integer types
@ -283,6 +284,7 @@ fn list_scalar_type(items: &[Literal]) -> Result<ScalarType> {
fn literal_scalar_type(lit: &Literal) -> Result<ScalarType> {
match lit {
Literal::Null => Ok(ScalarType::String),
Literal::String(_) => Ok(ScalarType::String),
Literal::Integer(_) => Ok(ScalarType::I64),
Literal::Float(_) => Ok(ScalarType::F64),

View file

@ -1225,6 +1225,7 @@ fn ir_expr_to_sql(expr: &IRExpr, params: &ParamMap) -> Option<String> {
pub(super) fn literal_to_sql(lit: &Literal) -> String {
match lit {
Literal::Null => "NULL".to_string(),
Literal::String(s) => format!("'{}'", s.replace('\'', "''")),
Literal::Integer(n) => n.to_string(),
Literal::Float(f) => f.to_string(),