mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-09 01:35:18 +02:00
Fix nullable query parameters: accept omission and null for ? params
Parameters declared with `?` (e.g. `$changelogUrl: String?`) now correctly accept omission or explicit null in JSON input instead of requiring empty strings as a workaround. Adds `Literal::Null` variant and threads it through parameter parsing, type-checking, and Arrow array conversion. https://claude.ai/code/session_014oGFKL7EVg1b2cyPgt9Gne
This commit is contained in:
parent
c5a88cacb5
commit
37b7a94eb7
6 changed files with 135 additions and 13 deletions
|
|
@ -160,6 +160,7 @@ impl std::fmt::Display for AggFunc {
|
|||
|
||||
#[derive(Debug, Clone)]
|
||||
pub enum Literal {
|
||||
Null,
|
||||
String(String),
|
||||
Integer(i64),
|
||||
Float(f64),
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -284,12 +284,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(¶m.name) {
|
||||
map.insert(param.name.clone(), Literal::Null);
|
||||
}
|
||||
}
|
||||
|
||||
Ok(map)
|
||||
|
|
@ -568,15 +587,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 +900,96 @@ 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)));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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(),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue