diff --git a/backend/parsers/windmill-parser-bash/src/lib.rs b/backend/parsers/windmill-parser-bash/src/lib.rs index 0893d65273..73dee0e36a 100644 --- a/backend/parsers/windmill-parser-bash/src/lib.rs +++ b/backend/parsers/windmill-parser-bash/src/lib.rs @@ -134,6 +134,7 @@ fn parse_bash_file(code: &str) -> anyhow::Result>> { otyp: None, has_default: default.is_some(), oidx: None, + otyp_inferred: false, }); } else { break; @@ -731,6 +732,7 @@ fn finalize_parameter( otyp, has_default, oidx: None, + otyp_inferred: false, }) } @@ -774,7 +776,8 @@ non_required="${5:-}" typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -782,7 +785,8 @@ non_required="${5:-}" typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -790,7 +794,8 @@ non_required="${5:-}" typ: Typ::Str(None), default: Some(json!("latest with spaces")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -798,7 +803,8 @@ non_required="${5:-}" typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -806,7 +812,8 @@ non_required="${5:-}" typ: Typ::Str(None), default: Some(json!("")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, @@ -833,7 +840,8 @@ non_required="${5:-}" typ: Typ::Str(None), default: None, has_default: true, // Optional (not mandatory) - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("string".to_string()), // [string] @@ -841,7 +849,8 @@ non_required="${5:-}" typ: Typ::Str(None), default: None, has_default: true, // Optional (not mandatory) - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, // No type annotation @@ -849,7 +858,8 @@ non_required="${5:-}" typ: Typ::Str(None), default: Some(json!("default value, with comma")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("int".to_string()), // [int] @@ -857,7 +867,8 @@ non_required="${5:-}" typ: Typ::Int, default: Some(json!(3)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, // Type inferred from default value @@ -865,7 +876,8 @@ non_required="${5:-}" typ: Typ::Float, default: Some(json!(5.0)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, // Type inferred from default value @@ -873,7 +885,8 @@ non_required="${5:-}" typ: Typ::Int, default: Some(json!(5)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, // No type annotation @@ -881,7 +894,8 @@ non_required="${5:-}" typ: Typ::Str(None), default: None, has_default: true, // Optional (not mandatory) - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("PSCustomObject".to_string()), // [PSCustomObject] @@ -889,7 +903,8 @@ non_required="${5:-}" typ: Typ::Object(ObjectType::new(None, None)), default: None, has_default: true, // Optional (not mandatory) - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("string[]".to_string()), // [string[]] @@ -897,7 +912,8 @@ non_required="${5:-}" typ: Typ::List(Box::new(Typ::Str(None))), default: None, has_default: true, // Optional (not mandatory) - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("string".to_string()), // [string] (last type bracket with Mandatory and ValidateSet) @@ -909,7 +925,8 @@ non_required="${5:-}" ])), // ValidateSet enum default: None, has_default: false, // Required (Mandatory attribute) - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, @@ -1462,7 +1479,8 @@ param( typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1470,7 +1488,8 @@ param( typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1478,7 +1497,8 @@ param( typ: Typ::Str(None), default: Some(json!("latest with spaces")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1486,7 +1506,8 @@ param( typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1494,7 +1515,8 @@ param( typ: Typ::Str(None), default: Some(json!("")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, diff --git a/backend/parsers/windmill-parser-csharp/src/lib.rs b/backend/parsers/windmill-parser-csharp/src/lib.rs index 254fc3ab50..2c068f0901 100644 --- a/backend/parsers/windmill-parser-csharp/src/lib.rs +++ b/backend/parsers/windmill-parser-csharp/src/lib.rs @@ -77,7 +77,7 @@ pub fn parse_csharp_sig_meta(code: &str) -> anyhow::Result { } } let (otyp, typ, name) = parse_csharp_typ(p_list_node, code)?; - args.push(Arg { name, otyp, typ, default, has_default: false, oidx: None }); + args.push(Arg { name, otyp, typ, default, has_default: false, oidx: None, otyp_inferred: false }); } } } diff --git a/backend/parsers/windmill-parser-go/src/lib.rs b/backend/parsers/windmill-parser-go/src/lib.rs index cfb73abdae..0f340943aa 100644 --- a/backend/parsers/windmill-parser-go/src/lib.rs +++ b/backend/parsers/windmill-parser-go/src/lib.rs @@ -34,6 +34,7 @@ pub fn parse_go_sig(code: &str) -> anyhow::Result { default: None, has_default: false, oidx: None, + otyp_inferred: false, } }) .collect_vec(); @@ -147,7 +148,10 @@ fn parse_go_typ(typ: &Expression) -> (Option, Typ) { Typ::Object(ObjectType::new(None, Some(typs))), ) } - Expression::TypeInterface(_) => (Some("interface{}".to_string()), Typ::Object(ObjectType::new(None, Some(vec![])))), + Expression::TypeInterface(_) => ( + Some("interface{}".to_string()), + Typ::Object(ObjectType::new(None, Some(vec![]))), + ), Expression::TypeMap(_) => ( Some("map[string]interface{}".to_string()), Typ::Object(ObjectType::new(None, Some(vec![]))), @@ -191,7 +195,8 @@ func main(x int, y string, z bool, l []string, o struct { Name string `json:"nam typ: Typ::Int, has_default: false, default: None, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("string".to_string()), @@ -199,7 +204,8 @@ func main(x int, y string, z bool, l []string, o struct { Name string `json:"nam typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("bool".to_string()), @@ -207,7 +213,8 @@ func main(x int, y string, z bool, l []string, o struct { Name string `json:"nam typ: Typ::Bool, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("[]string".to_string()), @@ -215,18 +222,23 @@ func main(x int, y string, z bool, l []string, o struct { Name string `json:"nam typ: Typ::List(Box::new(Typ::Str(None))), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("struct { Name string `json:\"name\"` }".to_string()), name: "o".to_string(), - typ: Typ::Object(ObjectType::new(None, Some(vec![ObjectProperty { - key: "name".to_string(), - typ: Box::new(Typ::Str(None)) - },]))), + typ: Typ::Object(ObjectType::new( + None, + Some(vec![ObjectProperty { + key: "name".to_string(), + typ: Box::new(Typ::Str(None)) + },]) + )), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("interface{}".to_string()), @@ -234,7 +246,8 @@ func main(x int, y string, z bool, l []string, o struct { Name string `json:"nam typ: Typ::Object(ObjectType::new(None, Some(vec![]))), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("map[string]interface{}".to_string()), @@ -242,12 +255,13 @@ func main(x int, y string, z bool, l []string, o struct { Name string `json:"nam typ: Typ::Object(ObjectType::new(None, Some(vec![]))), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, ], auto_kind: None, has_preprocessor: None, - ..Default::default() + ..Default::default() } ); diff --git a/backend/parsers/windmill-parser-graphql/src/lib.rs b/backend/parsers/windmill-parser-graphql/src/lib.rs index f7a62bdddb..3ba21a877a 100644 --- a/backend/parsers/windmill-parser-graphql/src/lib.rs +++ b/backend/parsers/windmill-parser-graphql/src/lib.rs @@ -64,6 +64,7 @@ fn parse_graphql_file(code: &str) -> anyhow::Result>> { otyp: Some(typ.unwrap()), has_default, oidx: None, + otyp_inferred: false, }); } @@ -107,7 +108,8 @@ query($i: Int, $arr: [String]!, $wahoo: String = "wahoo") { typ: Typ::Int, default: None, has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("[String]".to_string()), @@ -115,7 +117,8 @@ query($i: Int, $arr: [String]!, $wahoo: String = "wahoo") { typ: Typ::List(Box::new(Typ::Str(None))), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("String".to_string()), @@ -123,7 +126,8 @@ query($i: Int, $arr: [String]!, $wahoo: String = "wahoo") { typ: Typ::Str(None), default: Some(json!("wahoo")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, diff --git a/backend/parsers/windmill-parser-java/src/lib.rs b/backend/parsers/windmill-parser-java/src/lib.rs index f7347caabc..15db53f8b7 100644 --- a/backend/parsers/windmill-parser-java/src/lib.rs +++ b/backend/parsers/windmill-parser-java/src/lib.rs @@ -69,6 +69,7 @@ pub fn parse_java_sig_meta(code: &str) -> anyhow::Result { has_default: default.is_some(), default, oidx: None, + otyp_inferred: false, }); } } @@ -256,7 +257,8 @@ class Main { typ: Typ::Bytes, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "b".into(), @@ -264,7 +266,8 @@ class Main { typ: Typ::Int, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "c".into(), @@ -272,7 +275,8 @@ class Main { typ: Typ::Int, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "d".into(), @@ -280,7 +284,8 @@ class Main { typ: Typ::Int, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "e".into(), @@ -288,7 +293,8 @@ class Main { typ: Typ::Float, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "f".into(), @@ -296,7 +302,8 @@ class Main { typ: Typ::Float, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "g".into(), @@ -304,7 +311,8 @@ class Main { typ: Typ::Bool, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "h".into(), @@ -312,7 +320,8 @@ class Main { typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, ] ); @@ -338,7 +347,8 @@ class Main { typ: Typ::Bytes, default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "b".into(), @@ -346,7 +356,8 @@ class Main { typ: Typ::Int, default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "c".into(), @@ -354,7 +365,8 @@ class Main { typ: Typ::Int, default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "d".into(), @@ -362,7 +374,8 @@ class Main { typ: Typ::Int, default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "e".into(), @@ -370,7 +383,8 @@ class Main { typ: Typ::Float, default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "f".into(), @@ -378,7 +392,8 @@ class Main { typ: Typ::Float, default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "g".into(), @@ -386,7 +401,8 @@ class Main { typ: Typ::Bool, default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "h".into(), @@ -394,7 +410,8 @@ class Main { typ: Typ::Str(None), default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "i".into(), @@ -402,7 +419,8 @@ class Main { typ: Typ::Object(ObjectType::new(None, Some(vec![]))), default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, ] ); @@ -427,7 +445,8 @@ class Main { typ: Typ::List(Box::new(Typ::Int)), default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "b".into(), @@ -435,7 +454,8 @@ class Main { typ: Typ::List(Box::new(Typ::Object(ObjectType::new(None, Some(vec![]))))), default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "c".into(), @@ -443,7 +463,8 @@ class Main { typ: Typ::List(Box::new(Typ::Str(None))), default: Some(json!(null)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, ] ); diff --git a/backend/parsers/windmill-parser-nu/src/lib.rs b/backend/parsers/windmill-parser-nu/src/lib.rs index 8bf0615cc0..0808ed5a86 100644 --- a/backend/parsers/windmill-parser-nu/src/lib.rs +++ b/backend/parsers/windmill-parser-nu/src/lib.rs @@ -152,6 +152,7 @@ pub fn parse_nu_signature(code: &str) -> anyhow::Result { has_default: default.is_some() || optional, default: default.or_else(|| if optional { Some(json!(null)) } else { None }), oidx: None, + otyp_inferred: false, }); } diff --git a/backend/parsers/windmill-parser-nu/tests/tests.rs b/backend/parsers/windmill-parser-nu/tests/tests.rs index 00c0865229..368684b724 100644 --- a/backend/parsers/windmill-parser-nu/tests/tests.rs +++ b/backend/parsers/windmill-parser-nu/tests/tests.rs @@ -27,7 +27,8 @@ mod test { typ: Typ::Unknown, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "b".into(), @@ -35,7 +36,8 @@ mod test { typ: Typ::Unknown, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "c".into(), @@ -43,7 +45,8 @@ mod test { typ: Typ::Unknown, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "d".into(), @@ -51,7 +54,8 @@ mod test { typ: Typ::Unknown, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, @@ -80,7 +84,8 @@ mod test { typ: Typ::Unknown, default: Some(serde_json::Value::Null), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, },], auto_kind: None, has_preprocessor: None, @@ -109,7 +114,8 @@ mod test { typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "bar".into(), @@ -117,7 +123,8 @@ mod test { typ: Typ::Int, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -158,7 +165,8 @@ mod test { typ: Typ::Unknown, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "a2".into(), @@ -166,7 +174,8 @@ mod test { typ: Typ::Bool, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "a3".into(), @@ -174,7 +183,8 @@ mod test { typ: Typ::Int, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "a4".into(), @@ -182,7 +192,8 @@ mod test { typ: Typ::Float, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "a5".into(), @@ -190,7 +201,8 @@ mod test { typ: Typ::Datetime, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "a6".into(), @@ -198,7 +210,8 @@ mod test { typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "a7".into(), @@ -206,7 +219,8 @@ mod test { typ: Typ::Object(ObjectType::new(None, Some(vec![]))), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "a8".into(), @@ -214,7 +228,8 @@ mod test { typ: Typ::List(Box::new(Typ::Unknown)), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "a9".into(), @@ -222,7 +237,8 @@ mod test { typ: Typ::List(Box::new(Typ::Object(ObjectType::new(None, Some(vec![]))))), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "a10".into(), @@ -230,7 +246,8 @@ mod test { typ: Typ::Unknown, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -262,7 +279,8 @@ mod test { typ: Typ::Unknown, default: Some(json!("Foo")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "bar".into(), @@ -270,7 +288,8 @@ mod test { typ: Typ::Str(None), default: Some(json!("Bar")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "bazz".into(), @@ -278,7 +297,8 @@ mod test { typ: Typ::Unknown, default: Some(json!(3)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -375,7 +395,8 @@ mod test { typ: Typ::List(Box::new(Typ::Float)), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, },], auto_kind: None, has_preprocessor: None, @@ -406,7 +427,8 @@ mod test { typ: Typ::Unknown, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "foo".into(), @@ -414,7 +436,8 @@ mod test { typ: Typ::List(Box::new(Typ::Float)), default: Some(json!([2, 3, 4])), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "b".into(), @@ -422,7 +445,8 @@ mod test { typ: Typ::Unknown, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -452,7 +476,8 @@ mod test { typ: Typ::Datetime, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, },], auto_kind: None, has_preprocessor: None, @@ -515,7 +540,8 @@ mod test { typ: Typ::Unknown, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "b".into(), @@ -523,7 +549,8 @@ mod test { typ: Typ::Int, default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "c".into(), @@ -531,7 +558,8 @@ mod test { typ: Typ::Unknown, default: Some(serde_json::Value::Null), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "d".into(), @@ -539,7 +567,8 @@ mod test { typ: Typ::Str(None), default: Some(json!("foo")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { name: "bi".into(), @@ -547,7 +576,8 @@ mod test { typ: Typ::Unknown, default: Some(serde_json::Value::Null), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, diff --git a/backend/parsers/windmill-parser-php/src/lib.rs b/backend/parsers/windmill-parser-php/src/lib.rs index f5d4c000c6..201e41a912 100644 --- a/backend/parsers/windmill-parser-php/src/lib.rs +++ b/backend/parsers/windmill-parser-php/src/lib.rs @@ -91,6 +91,7 @@ pub fn parse_php_signature( has_default: default.is_some(), default, oidx: None, + otyp_inferred: false, } }) .collect(); @@ -146,7 +147,8 @@ function main(string $input1 = \"hey\", bool $input2 = false, int $input3 = 3, f typ: Typ::Str(None), has_default: true, default: Some(Value::String("hey".to_string())), - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -154,7 +156,8 @@ function main(string $input1 = \"hey\", bool $input2 = false, int $input3 = 3, f typ: Typ::Bool, has_default: true, default: Some(Value::Bool(false)), - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -162,7 +165,8 @@ function main(string $input1 = \"hey\", bool $input2 = false, int $input3 = 3, f typ: Typ::Int, has_default: true, default: Some(Value::Number(Number::from(3))), - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -170,7 +174,8 @@ function main(string $input1 = \"hey\", bool $input2 = false, int $input3 = 3, f typ: Typ::Float, has_default: true, default: Some(Value::Number(Number::from_f64(f64::from(4.5)).unwrap())), - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -178,7 +183,8 @@ function main(string $input1 = \"hey\", bool $input2 = false, int $input3 = 3, f typ: Typ::Resource("stripe".to_string()), has_default: false, default: None, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, diff --git a/backend/parsers/windmill-parser-py/src/lib.rs b/backend/parsers/windmill-parser-py/src/lib.rs index 45cb23f6e6..4807842724 100644 --- a/backend/parsers/windmill-parser-py/src/lib.rs +++ b/backend/parsers/windmill-parser-py/src/lib.rs @@ -477,6 +477,7 @@ pub fn parse_python_signature( has_default: has_default || default.is_some(), default, oidx: None, + otyp_inferred: false, } }) .collect(), @@ -716,7 +717,8 @@ def main(test1: str, name: datetime.datetime = datetime.now(), byte: bytes = byt typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -724,7 +726,8 @@ def main(test1: str, name: datetime.datetime = datetime.now(), byte: bytes = byt typ: Typ::Datetime, default: Some(json!("")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -732,7 +735,8 @@ def main(test1: str, name: datetime.datetime = datetime.now(), byte: bytes = byt typ: Typ::Bytes, default: Some(json!("")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -740,7 +744,8 @@ def main(test1: str, name: datetime.datetime = datetime.now(), byte: bytes = byt typ: Typ::Str(None), default: Some(json!("wewe")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -748,7 +753,8 @@ def main(test1: str, name: datetime.datetime = datetime.now(), byte: bytes = byt typ: Typ::Int, default: Some(json!(21)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -756,7 +762,8 @@ def main(test1: str, name: datetime.datetime = datetime.now(), byte: bytes = byt typ: Typ::List(Box::new(Typ::Int)), default: Some(json!([1, 2])), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -764,7 +771,8 @@ def main(test1: str, name: datetime.datetime = datetime.now(), byte: bytes = byt typ: Typ::Bool, default: Some(json!(true)), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -806,7 +814,8 @@ def main(test1: str, typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -814,7 +823,8 @@ def main(test1: str, typ: Typ::Datetime, default: Some(json!("")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -822,7 +832,8 @@ def main(test1: str, typ: Typ::Bytes, default: Some(json!("")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -830,7 +841,8 @@ def main(test1: str, typ: Typ::Resource("postgresql".to_string()), default: Some(json!("$res:g/all/resource")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, @@ -867,7 +879,8 @@ def main(test1: str, typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -875,7 +888,8 @@ def main(test1: str, typ: Typ::Resource("s3_object".to_string()), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -883,7 +897,8 @@ def main(test1: str, typ: Typ::Str(None), default: Some(json!("test")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -891,7 +906,8 @@ def main(test1: str, typ: Typ::Bytes, default: Some(json!("")), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, @@ -925,7 +941,8 @@ def main(test1: Literal["foo", "bar"], test2: List[Literal["foo", "bar"]]): retu typ: Typ::Str(Some(vec!["foo".to_string(), "bar".to_string()])), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -936,7 +953,8 @@ def main(test1: Literal["foo", "bar"], test2: List[Literal["foo", "bar"]]): retu ])))), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, @@ -969,7 +987,8 @@ def main(test1: DynSelect_foo): return typ: Typ::DynSelect("foo".to_string()), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }], auto_kind: None, has_preprocessor: Some(false), @@ -1094,7 +1113,8 @@ def main(a: list, e: List[int], b: list = [1,2,3,4], c = [1,2,3,4], d = ["a", "b typ: Typ::List(Box::new(Typ::Str(None))), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1102,7 +1122,8 @@ def main(a: list, e: List[int], b: list = [1,2,3,4], c = [1,2,3,4], d = ["a", "b typ: Typ::List(Box::new(Typ::Int)), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1110,7 +1131,8 @@ def main(a: list, e: List[int], b: list = [1,2,3,4], c = [1,2,3,4], d = ["a", "b typ: Typ::List(Box::new(Typ::Int)), default: Some(json!([1, 2, 3, 4])), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1118,7 +1140,8 @@ def main(a: list, e: List[int], b: list = [1,2,3,4], c = [1,2,3,4], d = ["a", "b typ: Typ::List(Box::new(Typ::Int)), default: Some(json!([1, 2, 3, 4])), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1126,7 +1149,8 @@ def main(a: list, e: List[int], b: list = [1,2,3,4], c = [1,2,3,4], d = ["a", "b typ: Typ::List(Box::new(Typ::Str(None))), default: Some(json!(["a", "b"])), has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, } ], auto_kind: None, @@ -1160,7 +1184,8 @@ def main(a: str, b: Optional[str], c: str | None): return typ: Typ::Str(None), default: None, has_default: false, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1168,7 +1193,8 @@ def main(a: str, b: Optional[str], c: str | None): return typ: Typ::Str(None), default: None, has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, Arg { otyp: None, @@ -1176,7 +1202,8 @@ def main(a: str, b: Optional[str], c: str | None): return typ: Typ::Str(None), default: None, has_default: true, - oidx: None + oidx: None, + otyp_inferred: false, }, ], auto_kind: None, diff --git a/backend/parsers/windmill-parser-rust/src/lib.rs b/backend/parsers/windmill-parser-rust/src/lib.rs index 74a7e15de6..2cfeb52214 100644 --- a/backend/parsers/windmill-parser-rust/src/lib.rs +++ b/backend/parsers/windmill-parser-rust/src/lib.rs @@ -21,7 +21,7 @@ pub fn parse_rust_signature(code: &str) -> anyhow::Result { .iter() .map(|param| { let (otyp, typ, name) = parse_rust_typ(param); - Arg { name, otyp, typ, default: None, has_default: false, oidx: None } + Arg { name, otyp, typ, default: None, has_default: false, oidx: None, otyp_inferred: false } }) .collect_vec(); Ok(MainArgSignature { diff --git a/backend/parsers/windmill-parser-sql/src/lib.rs b/backend/parsers/windmill-parser-sql/src/lib.rs index 1c56bec175..931f470d3c 100644 --- a/backend/parsers/windmill-parser-sql/src/lib.rs +++ b/backend/parsers/windmill-parser-sql/src/lib.rs @@ -281,6 +281,7 @@ fn parse_oracledb_file(code: &str) -> anyhow::Result>> { otyp: Some(typ), has_default, oidx: None, + otyp_inferred: false, }); } @@ -305,6 +306,7 @@ fn parse_oracledb_file(code: &str) -> anyhow::Result>> { otyp: Some(typ), has_default, oidx: None, + otyp_inferred: false, }); } } @@ -331,6 +333,7 @@ fn parse_sql_sanitized_interpolation(code: &str) -> Vec { otyp: Some(otyp.to_string()), has_default, oidx: None, + otyp_inferred: false, }); } @@ -360,6 +363,7 @@ fn parse_mysql_file(code: &str) -> anyhow::Result>> { otyp: Some(typ), has_default, oidx: None, + otyp_inferred: false, }); } @@ -384,6 +388,7 @@ fn parse_mysql_file(code: &str) -> anyhow::Result>> { otyp: Some(typ), has_default, oidx: None, + otyp_inferred: false, }); } } @@ -518,7 +523,25 @@ fn run_on_sql_statement_matches< } pub fn parse_pg_statement_arg_indices(code: &str) -> HashSet { - let mut arg_indices = HashSet::new(); + parse_pg_statement_arg_positions(code) + .into_iter() + .map(|(idx, _)| idx) + .collect() +} + +/// Like `parse_pg_statement_arg_indices`, but also returns the byte range of +/// each placeholder occurrence in `code` (excluding `$`, including the digits). +/// The same string-/comment-/dollar-quote-aware tokenizer is used, so +/// occurrences inside string literals and comments are correctly skipped — +/// this is what callers need to renumber `$N → $M` without mangling literal +/// SQL bytes that happen to match the `$\d+` pattern. +/// +/// The returned vec is in source order. Each entry is `(idx, range)` where +/// `idx` is the parameter number and `range` covers the `$N` digits (i.e. +/// `code[range.start - 1 .. range.end]` is the full `$N` token, and +/// `code[range]` is just the digits). +pub fn parse_pg_statement_arg_positions(code: &str) -> Vec<(i32, std::ops::Range)> { + let mut positions = Vec::new(); run_on_sql_statement_matches( code, true, @@ -529,21 +552,24 @@ pub fn parse_pg_statement_arg_indices(code: &str) -> HashSet { .is_some_and(|&(_, next_char)| next_char.is_ascii_digit()) }, |_, chars| { + let start = chars.peek().map(|&(i, _)| i).unwrap_or(0); let mut arg_idx = String::new(); - while let Some(&(_, char)) = chars.peek() { + let mut end = start; + while let Some(&(i, char)) = chars.peek() { if char.is_ascii_digit() { arg_idx.push(char); + end = i + char.len_utf8(); chars.next(); } else { break; } } if let Ok(arg_idx) = arg_idx.parse::() { - arg_indices.insert(arg_idx); + positions.push((arg_idx, start..end)); } }, ); - arg_indices + positions } fn parse_pg_file(code: &str) -> anyhow::Result, bool)>> { @@ -577,12 +603,16 @@ fn parse_pg_file(code: &str) -> anyhow::Result, bool)>> { otyp: Some(typ), has_default, oidx: Some(idx), + otyp_inferred: false, }); } } - // Second pass: infer types from usage for non-explicitly-typed args - let mut hm: HashMap = HashMap::new(); + // Second pass: infer types from usage for non-explicitly-typed args. + // We track whether each entry came from an inline `$N::TYPE` cast or from + // the parser's "text" fallback, so the executor can later distinguish + // "user committed to text" from "no info, use a placeholder". + let mut hm: HashMap = HashMap::new(); for cap in RE_CODE_PGSQL.captures_iter(code) { let idx = cap .get(1) @@ -594,15 +624,23 @@ fn parse_pg_file(code: &str) -> anyhow::Result, bool)>> { continue; } - let typ = cap + let cast = cap .get(2) - .map(|cap| transform_types_with_spaces(&cap, &code)) - .unwrap_or("text"); - hm.insert(idx, typ.to_string()); + .map(|cap| transform_types_with_spaces(&cap, &code)); + let inferred_default = cast.is_none(); + let typ: std::borrow::Cow = cast.unwrap_or(std::borrow::Cow::Borrowed("text")); + // Prefer an explicit cast over a previously seen default — once we + // have any inline cast for the index, lock it in. + match hm.get(&idx) { + Some((_, false)) => {} // already locked from explicit cast + _ => { + hm.insert(idx, (typ.into_owned(), inferred_default)); + } + } } // Add inferred args - for (i, v) in hm.iter() { + for (i, (v, inferred)) in hm.iter() { let typ = v.to_lowercase(); args.push(Arg { name: format!("${}", i), @@ -611,6 +649,7 @@ fn parse_pg_file(code: &str) -> anyhow::Result, bool)>> { otyp: Some(typ), has_default: false, oidx: Some(*i), + otyp_inferred: *inferred, }); } @@ -646,6 +685,7 @@ fn parse_pg_file(code: &str) -> anyhow::Result, bool)>> { otyp: oarg.otyp, has_default, oidx: oarg.oidx, + otyp_inferred: oarg.otyp_inferred, }; } } @@ -657,8 +697,12 @@ fn parse_pg_file(code: &str) -> anyhow::Result, bool)>> { } // The regex doesn't parse types with space such as "character varying" -// So we look for them manually and replace them with their shorter counterpart -fn transform_types_with_spaces<'a>(cap: &Match<'a>, code: &str) -> &'a str { +// So we look for them manually and replace them with their shorter counterpart. +// Returns `Cow::Borrowed` for the trivial case (the regex's own match) and +// `Cow::Owned` when we need to alias a multi-word type and/or append a `[]` +// suffix that the regex's `\w+` capture didn't pick up. +fn transform_types_with_spaces<'a>(cap: &Match<'a>, code: &str) -> std::borrow::Cow<'a, str> { + use std::borrow::Cow; lazy_static! { static ref TYPES: [(&'static str, &'static str); 6] = [ ("character varying", "varchar"), @@ -671,20 +715,31 @@ fn transform_types_with_spaces<'a>(cap: &Match<'a>, code: &str) -> &'a str { } let typ = &code[cap.start()..]; for (long_type, alias) in TYPES.iter() { - let mut typ = typ; + let mut rest = typ; let mut found_mismatch = false; for token in long_type.split(' ') { - if typ.len() < token.len() || !typ[..token.len()].eq_ignore_ascii_case(token) { + if rest.len() < token.len() || !rest[..token.len()].eq_ignore_ascii_case(token) { found_mismatch = true; break; } - typ = typ[token.len()..].trim_start(); + rest = rest[token.len()..].trim_start(); } if !found_mismatch { - return alias; + // The regex captured only the first word (`\w+`), so its `[]` + // detection in `(?:\[\])?` matched against the wrong position + // and is empty for multi-word types. Re-check the trailing + // bytes after the multi-word match: if they start with `[]`, + // append the array suffix to the alias so the dispatch routes + // through `convert_vec_val` instead of binding as JSONB. + let with_suffix = rest.starts_with("[]"); + return if with_suffix { + Cow::Owned(format!("{alias}[]")) + } else { + Cow::Borrowed(*alias) + }; } } - cap.as_str() + Cow::Borrowed(cap.as_str()) } pub fn parse_sql_statement_named_params(code: &str, prefix: char) -> HashSet { @@ -736,6 +791,7 @@ fn parse_bigquery_file(code: &str) -> anyhow::Result>> { otyp: Some(typ), has_default, oidx: None, + otyp_inferred: false, }); } @@ -765,6 +821,7 @@ fn parse_duckdb_file(code: &str) -> anyhow::Result>> { otyp: Some(typ), has_default, oidx: None, + otyp_inferred: false, }); } @@ -794,6 +851,7 @@ fn parse_snowflake_file(code: &str) -> anyhow::Result>> { otyp: Some(typ), has_default, oidx: None, + otyp_inferred: false, }); } @@ -823,6 +881,7 @@ fn parse_mssql_file(code: &str) -> anyhow::Result>> { otyp: Some(typ), has_default, oidx: None, + otyp_inferred: false, }); } @@ -1006,6 +1065,7 @@ SELECT * FROM table WHERE token=$1::TEXT AND image=$2::BIGINT default: None, has_default: false, oidx: Some(1), + otyp_inferred: false, }, Arg { otyp: Some("bigint".to_string()), @@ -1014,6 +1074,7 @@ SELECT * FROM table WHERE token=$1::TEXT AND image=$2::BIGINT default: None, has_default: false, oidx: Some(2), + otyp_inferred: false, }, ], auto_kind: None, @@ -1048,6 +1109,7 @@ SELECT $2::TEXT; default: None, has_default: false, oidx: Some(1), + otyp_inferred: false, }, Arg { otyp: Some("text".to_string()), @@ -1056,6 +1118,7 @@ SELECT $2::TEXT; default: None, has_default: false, oidx: Some(2), + otyp_inferred: false, }, Arg { otyp: Some("text".to_string()), @@ -1064,6 +1127,7 @@ SELECT $2::TEXT; default: None, has_default: false, oidx: Some(3), + otyp_inferred: false, }, ], auto_kind: None, @@ -1216,6 +1280,54 @@ SELECT $2;"#; Ok(()) } + #[test] + fn test_parse_pg_statement_arg_positions_skips_strings_and_comments() -> anyhow::Result<()> { + // Each occurrence's byte range covers JUST the digits (after `$`). + let code = "SELECT $5, $50"; + let positions = parse_pg_statement_arg_positions(code); + let collected: Vec<(i32, &str)> = positions + .iter() + .map(|(idx, range)| (*idx, &code[range.clone()])) + .collect(); + assert_eq!(collected, vec![(5, "5"), (50, "50")]); + + // String literals and comments must not produce positions — this is + // what stops the do_postgresql_inner rewrite from mangling SQL like + // `'price: $5'`. + let code = "SELECT 'literal $5' AS lbl, $5 FROM t -- mention $5"; + let positions = parse_pg_statement_arg_positions(code); + let positions_only: Vec<(i32, std::ops::Range)> = positions.clone(); + assert_eq!( + positions_only.iter().map(|(i, _)| *i).collect::>(), + vec![5], + "only the real $5 between 'lbl,' and 'FROM' should be returned" + ); + // The single returned position is the real placeholder (between + // `lbl, ` and ` FROM`). + let (idx, range) = &positions[0]; + assert_eq!(*idx, 5); + // `code[range.start - 1 .. range.end]` should be the full `$5` token. + assert_eq!(&code[range.start - 1..range.end], "$5"); + + // Dollar-quoted blocks similarly skipped. + let code = "SELECT $$body with $5 inside$$, $7 FROM t"; + let positions = parse_pg_statement_arg_positions(code); + assert_eq!( + positions.iter().map(|(i, _)| *i).collect::>(), + vec![7], + "$5 inside $$...$$ is part of the string" + ); + + // Repeat indices show up multiple times — caller can rewrite each. + let code = "SELECT $1, $1, $2"; + let positions = parse_pg_statement_arg_positions(code); + assert_eq!( + positions.iter().map(|(i, _)| *i).collect::>(), + vec![1, 1, 2] + ); + Ok(()) + } + #[test] fn test_parse_sql_blocks_non_pg_ignores_dollar_quotes() -> anyhow::Result<()> { // Non-Postgres dialects (MySQL/Oracle/BigQuery/Snowflake) pass `false`, @@ -1259,6 +1371,7 @@ SELECT ?, ?; default: Some(json!(3)), has_default: true, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("text".to_string()), @@ -1267,6 +1380,7 @@ SELECT ?, ?; default: None, has_default: false, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -1300,6 +1414,7 @@ SELECT :param2; default: Some(json!(3)), has_default: true, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("text".to_string()), @@ -1308,6 +1423,7 @@ SELECT :param2; default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("text".to_string()), @@ -1316,6 +1432,7 @@ SELECT :param2; default: None, has_default: false, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -1349,6 +1466,7 @@ SELECT @token; default: Some(json!("abc")), has_default: true, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("int64".to_string()), @@ -1357,6 +1475,7 @@ SELECT @token; default: None, has_default: false, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -1390,6 +1509,7 @@ SELECT ?; default: Some(json!(3)), has_default: true, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("varchar".to_string()), @@ -1398,6 +1518,7 @@ SELECT ?; default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("varchar".to_string()), @@ -1406,6 +1527,7 @@ SELECT ?; default: None, has_default: false, oidx: None, + otyp_inferred: false, } ], auto_kind: None, @@ -1439,6 +1561,7 @@ SELECT @P2; default: Some(json!(3)), has_default: true, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("varchar".to_string()), @@ -1447,6 +1570,7 @@ SELECT @P2; default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("varchar".to_string()), @@ -1455,6 +1579,7 @@ SELECT @P2; default: None, has_default: false, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -1489,6 +1614,7 @@ SELECT * FROM table_name WHERE thing = :name4; default: Some(json!(3)), has_default: true, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("text".to_string()), @@ -1497,6 +1623,7 @@ SELECT * FROM table_name WHERE thing = :name4; default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { otyp: Some("text".to_string()), @@ -1505,6 +1632,7 @@ SELECT * FROM table_name WHERE thing = :name4; default: None, has_default: false, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -1536,6 +1664,7 @@ SELECT * FROM users WHERE id = $1 AND email = $2::text; default: None, has_default: false, oidx: Some(1), + otyp_inferred: false, }, Arg { otyp: Some("text".to_string()), @@ -1544,6 +1673,7 @@ SELECT * FROM users WHERE id = $1 AND email = $2::text; default: None, has_default: false, oidx: Some(2), + otyp_inferred: false, }, ], auto_kind: None, @@ -1575,6 +1705,7 @@ SELECT * FROM users LIMIT $1 OFFSET $2; default: Some(json!(10)), has_default: true, oidx: Some(1), + otyp_inferred: false, }, Arg { otyp: Some("bigint".to_string()), @@ -1583,6 +1714,7 @@ SELECT * FROM users LIMIT $1 OFFSET $2; default: Some(json!(0)), has_default: true, oidx: Some(2), + otyp_inferred: false, }, ], auto_kind: None, @@ -1618,6 +1750,7 @@ WHERE id = $1 default: None, has_default: false, oidx: Some(1), + otyp_inferred: false, }, Arg { otyp: Some("text".to_string()), @@ -1626,6 +1759,7 @@ WHERE id = $1 default: None, has_default: false, oidx: Some(2), + otyp_inferred: false, }, Arg { otyp: Some("timestamptz".to_string()), @@ -1634,6 +1768,7 @@ WHERE id = $1 default: None, has_default: false, oidx: Some(3), + otyp_inferred: false, }, ], auto_kind: None, @@ -1663,6 +1798,7 @@ SELECT * FROM users WHERE id = ANY($1); default: None, has_default: false, oidx: Some(1), + otyp_inferred: false, },], auto_kind: None, has_preprocessor: None, @@ -1693,6 +1829,7 @@ SELECT $1::integer; default: None, has_default: false, oidx: Some(1), + otyp_inferred: false, },], auto_kind: None, has_preprocessor: None, @@ -1703,6 +1840,62 @@ SELECT $1::integer; Ok(()) } + #[test] + fn test_parse_pgsql_otyp_inferred_flag() -> anyhow::Result<()> { + // Bare `$N` (no inline cast, no decl) should produce otyp = "text" + // *and* otyp_inferred = true. This is the signal the PG executor + // uses to decide whether the user committed to a text target. + let code_bare = "SELECT $1, $2"; + let args = parse_pgsql_sig(code_bare)?.args; + let map: HashMap, bool)> = args + .into_iter() + .map(|a| (a.name, (a.otyp, a.otyp_inferred))) + .collect(); + assert_eq!( + map.get("$1").cloned(), + Some((Some("text".to_string()), true)), + "bare $1 → otyp_inferred true" + ); + assert_eq!( + map.get("$2").cloned(), + Some((Some("text".to_string()), true)), + "bare $2 → otyp_inferred true" + ); + + // Inline `$N::TYPE` cast → otyp_inferred = false (user committed). + let args = parse_pgsql_sig("SELECT $1::int, $2::text")?.args; + let map: HashMap, bool)> = args + .into_iter() + .map(|a| (a.name, (a.otyp, a.otyp_inferred))) + .collect(); + assert_eq!( + map.get("$1").cloned(), + Some((Some("int".to_string()), false)) + ); + assert_eq!( + map.get("$2").cloned(), + Some((Some("text".to_string()), false)), + "explicit $2::text → otyp_inferred false (distinct from bare $2)" + ); + + // Declaration `-- $N name (TYPE)` → otyp_inferred = false (decl is + // explicit by definition). + let args = parse_pgsql_sig("-- $1 name (text)\nSELECT $1")?.args; + assert_eq!(args[0].otyp.as_deref(), Some("text")); + assert!(!args[0].otyp_inferred); + + // Mixed: $1 has decl, $2 is bare → flag differs per arg. + let args = parse_pgsql_sig("-- $1 a (int)\nSELECT $1, $2")?.args; + let map: HashMap = args + .into_iter() + .map(|a| (a.name, a.otyp_inferred)) + .collect(); + assert_eq!(map.get("a").copied(), Some(false), "$1 decl → not inferred"); + assert_eq!(map.get("$2").copied(), Some(true), "$2 bare → inferred"); + + Ok(()) + } + #[test] fn test_parse_s3object_arg_per_dialect() -> anyhow::Result<()> { // Confirms that `(s3object)` is recognised as a resource-typed arg in every @@ -1782,6 +1975,7 @@ SELECT x default: None, has_default: false, oidx: None, + otyp_inferred: false, },], auto_kind: None, has_preprocessor: None, diff --git a/backend/parsers/windmill-parser-ts/src/lib.rs b/backend/parsers/windmill-parser-ts/src/lib.rs index 0ac3df6ec4..e63b0ef680 100644 --- a/backend/parsers/windmill-parser-ts/src/lib.rs +++ b/backend/parsers/windmill-parser-ts/src/lib.rs @@ -208,9 +208,7 @@ pub fn parse_relative_imports(code: &str, path: &str) -> anyhow::Result bool { - import_path.starts_with("./") - || import_path.starts_with("../") - || import_path.starts_with("/") + import_path.starts_with("./") || import_path.starts_with("../") || import_path.starts_with("/") } /// Normalize a path by resolving `.` and `..` components @@ -542,6 +540,7 @@ fn parse_param( default: None, has_default: ident.id.optional || nullable, oidx: None, + otyp_inferred: false, }) } // Pat::Object(ObjectPat { ... }) = todo!() @@ -596,13 +595,29 @@ fn parse_param( if typ == Typ::Unknown && dflt.is_some() { typ = json_to_typ(dflt.as_ref().unwrap(), false); } - Ok(Arg { otyp, name, typ, default: dflt, has_default: true, oidx: None }) + Ok(Arg { + otyp, + name, + typ, + default: dflt, + has_default: true, + oidx: None, + otyp_inferred: false, + }) } Pat::Object(ObjectPat { type_ann, .. }) => { let (typ, nullable) = eval_type_ann(symbol_table, type_resolver, &type_ann); *counter += 1; let name = format!("anon{}", counter); - Ok(Arg { otyp: None, name, typ, default: None, has_default: nullable, oidx: None }) + Ok(Arg { + otyp: None, + name, + typ, + default: None, + has_default: nullable, + oidx: None, + otyp_inferred: false, + }) } _ => Err(anyhow::anyhow!( "parameter syntax unsupported: `{}`: {:#?}", diff --git a/backend/parsers/windmill-parser-ts/tests/tests.rs b/backend/parsers/windmill-parser-ts/tests/tests.rs index 643b2ba554..0243ccf06d 100644 --- a/backend/parsers/windmill-parser-ts/tests/tests.rs +++ b/backend/parsers/windmill-parser-ts/tests/tests.rs @@ -2,7 +2,9 @@ mod tests { use serde_json::json; use windmill_parser::{Arg, MainArgSignature, ObjectProperty, ObjectType, Typ}; - use windmill_parser_ts::{parse_deno_signature, parse_expr_for_imports, parse_relative_imports}; + use windmill_parser_ts::{ + parse_deno_signature, parse_expr_for_imports, parse_relative_imports, + }; #[test] fn test_imports_basic() { @@ -78,6 +80,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "num_param".to_string(), @@ -86,6 +89,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "bool_param".to_string(), @@ -94,6 +98,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "any_param".to_string(), @@ -102,6 +107,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -136,6 +142,7 @@ mod tests { default: Some(json!("World")), has_default: true, oidx: None, + otyp_inferred: false, }, Arg { name: "count".to_string(), @@ -144,6 +151,7 @@ mod tests { default: Some(json!(42)), has_default: true, oidx: None, + otyp_inferred: false, }, Arg { name: "enabled".to_string(), @@ -152,6 +160,7 @@ mod tests { default: Some(json!(true)), has_default: true, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -186,6 +195,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "numbers".to_string(), @@ -194,6 +204,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "items".to_string(), @@ -202,6 +213,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -235,6 +247,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, },], auto_kind: None, has_preprocessor: Some(false), @@ -267,6 +280,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, },], auto_kind: None, has_preprocessor: Some(false), @@ -308,6 +322,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }], auto_kind: None, has_preprocessor: Some(false), @@ -347,6 +362,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }], auto_kind: None, has_preprocessor: Some(false), @@ -406,6 +422,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }], auto_kind: None, has_preprocessor: Some(false), @@ -440,6 +457,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "base64_param".to_string(), @@ -448,6 +466,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "email_param".to_string(), @@ -456,6 +475,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "sql_param".to_string(), @@ -464,6 +484,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -498,6 +519,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "optional".to_string(), @@ -506,6 +528,7 @@ mod tests { default: None, has_default: true, oidx: None, + otyp_inferred: false, }, Arg { name: "with_default".to_string(), @@ -514,6 +537,7 @@ mod tests { default: Some(json!(false)), has_default: true, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -593,6 +617,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }], auto_kind: None, has_preprocessor: Some(false), @@ -623,6 +648,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }], auto_kind: None, has_preprocessor: Some(false), @@ -653,6 +679,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }], auto_kind: None, has_preprocessor: Some(false), @@ -686,6 +713,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "numbers".to_string(), @@ -694,6 +722,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, Arg { name: "plain".to_string(), @@ -702,6 +731,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }, ], auto_kind: None, @@ -744,6 +774,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }], auto_kind: None, has_preprocessor: Some(true), @@ -775,6 +806,7 @@ mod tests { default: None, has_default: false, oidx: None, + otyp_inferred: false, }], auto_kind: None, has_preprocessor: Some(true), diff --git a/backend/parsers/windmill-parser-yaml/src/lib.rs b/backend/parsers/windmill-parser-yaml/src/lib.rs index 39dd504240..8a1f1bc097 100644 --- a/backend/parsers/windmill-parser-yaml/src/lib.rs +++ b/backend/parsers/windmill-parser-yaml/src/lib.rs @@ -50,6 +50,7 @@ pub fn parse_ansible_sig(inner_content: &str) -> anyhow::Result anyhow::Result anyhow::Result, pub has_default: bool, pub oidx: Option, + /// `true` when `otyp` is the parser's fallback default rather than a value + /// the user (or SDK) actually wrote down. Currently only set by the PG SQL + /// parser when a placeholder has no `-- $N name (TYPE)` declaration *and* + /// no `$N::TYPE` inline cast — the otyp is `"text"` purely as a + /// placeholder. Consumers that care about original intent (e.g. the PG + /// executor deciding whether to coerce `Number → String` for a text + /// target) should treat `otyp_inferred = true` as "type unknown". + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub otyp_inferred: bool, } pub fn json_to_typ(js: &Value, precise_arrays: bool) -> Typ { diff --git a/backend/tests/end_user_email.rs b/backend/tests/end_user_email.rs index 1a60f8672c..539eeaaa6e 100644 --- a/backend/tests/end_user_email.rs +++ b/backend/tests/end_user_email.rs @@ -63,10 +63,7 @@ fn authed(builder: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuil /// Create an app with inline script via API async fn create_app_with_inline_script(port: u16, path: &str) -> anyhow::Result<()> { - let url = format!( - "http://localhost:{}/api/w/test-workspace/apps/create", - port - ); + let url = format!("http://localhost:{}/api/w/test-workspace/apps/create", port); let resp = authed(client().post(&url), SAME_WS_TOKEN) .json(&json!({ "path": path, @@ -102,17 +99,18 @@ async fn create_app_with_inline_script(port: u16, path: &str) -> anyhow::Result< .send() .await?; if !resp.status().is_success() { - anyhow::bail!("create app failed: {} - {}", resp.status(), resp.text().await?); + anyhow::bail!( + "create app failed: {} - {}", + resp.status(), + resp.text().await? + ); } Ok(()) } /// Create a raw app with inline script via API (uses regular app endpoint with rawapp type) async fn create_raw_app_with_inline_script(port: u16, path: &str) -> anyhow::Result<()> { - let url = format!( - "http://localhost:{}/api/w/test-workspace/apps/create", - port - ); + let url = format!("http://localhost:{}/api/w/test-workspace/apps/create", port); let resp = authed(client().post(&url), SAME_WS_TOKEN) .json(&json!({ "path": path, @@ -146,12 +144,21 @@ async fn create_raw_app_with_inline_script(port: u16, path: &str) -> anyhow::Res .send() .await?; if !resp.status().is_success() { - anyhow::bail!("create raw app failed: {} - {}", resp.status(), resp.text().await?); + anyhow::bail!( + "create raw app failed: {} - {}", + resp.status(), + resp.text().await? + ); } Ok(()) } -async fn run_app_inline_script(port: u16, token: &str, app_path: &str, force_viewer: bool) -> anyhow::Result { +async fn run_app_inline_script( + port: u16, + token: &str, + app_path: &str, + force_viewer: bool, +) -> anyhow::Result { let url = format!( "http://localhost:{}/api/w/test-workspace/apps_u/execute_component/{}", port, app_path @@ -173,13 +180,22 @@ async fn run_app_inline_script(port: u16, token: &str, app_path: &str, force_vie .send() .await?; if !resp.status().is_success() { - anyhow::bail!("app inline script run failed: {} - {}", resp.status(), resp.text().await?); + anyhow::bail!( + "app inline script run failed: {} - {}", + resp.status(), + resp.text().await? + ); } let job_id = resp.text().await?; wait_for_job_result(port, token, &job_id).await } -async fn run_raw_app_inline_script(port: u16, token: &str, app_path: &str, force_viewer: bool) -> anyhow::Result { +async fn run_raw_app_inline_script( + port: u16, + token: &str, + app_path: &str, + force_viewer: bool, +) -> anyhow::Result { let url = format!( "http://localhost:{}/api/w/test-workspace/apps_u/execute_component/{}", port, app_path @@ -200,7 +216,11 @@ async fn run_raw_app_inline_script(port: u16, token: &str, app_path: &str, force .send() .await?; if !resp.status().is_success() { - anyhow::bail!("raw app inline script run failed: {} - {}", resp.status(), resp.text().await?); + anyhow::bail!( + "raw app inline script run failed: {} - {}", + resp.status(), + resp.text().await? + ); } let job_id = resp.text().await?; wait_for_job_result(port, token, &job_id).await @@ -215,8 +235,12 @@ async fn wait_for_job_result(port: u16, token: &str, job_id: &str) -> anyhow::Re tokio::time::sleep(std::time::Duration::from_millis(100)).await; let resp = authed(client().get(&url), token).send().await?; if resp.status().is_success() { - return Ok(resp.json::().await? - .as_str().unwrap_or("").to_string()); + return Ok(resp + .json::() + .await? + .as_str() + .unwrap_or("") + .to_string()); } } anyhow::bail!("timeout waiting for job result") @@ -268,24 +292,38 @@ async fn test_app_wm_end_user_email(db: Pool) -> anyhow::Result<()> { let app_path = "f/test/email_app"; - in_test_worker(Connection::Sql(db.clone()), async move { - // Create the app with inline script first - create_app_with_inline_script(port, app_path).await?; + in_test_worker( + Connection::Sql(db.clone()), + async move { + // Create the app with inline script first + create_app_with_inline_script(port, app_path).await?; - // Same workspace user (force_viewer mode works for workspace members) - let result = run_app_inline_script(port, SAME_WS_TOKEN, app_path, true).await?; - assert_eq!(result, SAME_WS_EMAIL, "same workspace user should get their email"); + // Same workspace user (force_viewer mode works for workspace members) + let result = run_app_inline_script(port, SAME_WS_TOKEN, app_path, true).await?; + assert_eq!( + result, SAME_WS_EMAIL, + "same workspace user should get their email" + ); - // Other workspace user (uses app's anonymous policy + token lookup) - let result = run_app_inline_script(port, OTHER_WS_TOKEN, app_path, false).await?; - assert_eq!(result, OTHER_WS_EMAIL, "other workspace user should get their email"); + // Other workspace user (uses app's anonymous policy + token lookup) + let result = run_app_inline_script(port, OTHER_WS_TOKEN, app_path, false).await?; + assert_eq!( + result, OTHER_WS_EMAIL, + "other workspace user should get their email" + ); - // No workspace user (uses app's anonymous policy + token lookup) - let result = run_app_inline_script(port, NO_WS_TOKEN, app_path, false).await?; - assert_eq!(result, NO_WS_EMAIL, "no workspace user should get their email"); + // No workspace user (uses app's anonymous policy + token lookup) + let result = run_app_inline_script(port, NO_WS_TOKEN, app_path, false).await?; + assert_eq!( + result, NO_WS_EMAIL, + "no workspace user should get their email" + ); - Ok::<(), anyhow::Error>(()) - }, port).await?; + Ok::<(), anyhow::Error>(()) + }, + port, + ) + .await?; Ok(()) } @@ -300,24 +338,38 @@ async fn test_raw_app_wm_end_user_email(db: Pool) -> anyhow::Result<() let app_path = "f/test/email_raw_app"; - in_test_worker(Connection::Sql(db.clone()), async move { - // Create the raw app with inline script first - create_raw_app_with_inline_script(port, app_path).await?; + in_test_worker( + Connection::Sql(db.clone()), + async move { + // Create the raw app with inline script first + create_raw_app_with_inline_script(port, app_path).await?; - // Same workspace user (force_viewer mode works for workspace members) - let result = run_raw_app_inline_script(port, SAME_WS_TOKEN, app_path, true).await?; - assert_eq!(result, SAME_WS_EMAIL, "same workspace user should get their email"); + // Same workspace user (force_viewer mode works for workspace members) + let result = run_raw_app_inline_script(port, SAME_WS_TOKEN, app_path, true).await?; + assert_eq!( + result, SAME_WS_EMAIL, + "same workspace user should get their email" + ); - // Other workspace user (uses app's anonymous policy + token lookup) - let result = run_raw_app_inline_script(port, OTHER_WS_TOKEN, app_path, false).await?; - assert_eq!(result, OTHER_WS_EMAIL, "other workspace user should get their email"); + // Other workspace user (uses app's anonymous policy + token lookup) + let result = run_raw_app_inline_script(port, OTHER_WS_TOKEN, app_path, false).await?; + assert_eq!( + result, OTHER_WS_EMAIL, + "other workspace user should get their email" + ); - // No workspace user (uses app's anonymous policy + token lookup) - let result = run_raw_app_inline_script(port, NO_WS_TOKEN, app_path, false).await?; - assert_eq!(result, NO_WS_EMAIL, "no workspace user should get their email"); + // No workspace user (uses app's anonymous policy + token lookup) + let result = run_raw_app_inline_script(port, NO_WS_TOKEN, app_path, false).await?; + assert_eq!( + result, NO_WS_EMAIL, + "no workspace user should get their email" + ); - Ok::<(), anyhow::Error>(()) - }, port).await?; + Ok::<(), anyhow::Error>(()) + }, + port, + ) + .await?; Ok(()) } diff --git a/backend/tests/error_handler.rs b/backend/tests/error_handler.rs index 04658988a5..5d5b7a828b 100644 --- a/backend/tests/error_handler.rs +++ b/backend/tests/error_handler.rs @@ -34,10 +34,7 @@ async fn test_error_handler_settings(db: Pool) -> anyhow::Result<()> { ) .fetch_one(&db) .await?; - assert_eq!( - after_set, - Some("script/f/test/error_handler".to_string()) - ); + assert_eq!(after_set, Some("script/f/test/error_handler".to_string())); // Verify extra_args let extra_args = sqlx::query_scalar!( @@ -162,7 +159,8 @@ export async function main(path: string, email: string, job_id: string, is_flow: priority: None, apply_preprocessor: false, concurrency_settings: ConcurrencySettings::default(), - debouncing_settings: DebouncingSettings::default(), labels: None, + debouncing_settings: DebouncingSettings::default(), + labels: None, }) .run_until_complete(&db, false, server.addr.port()) .await; @@ -285,7 +283,8 @@ async fn test_error_handler_muted_on_script(db: Pool) -> anyhow::Resul priority: None, apply_preprocessor: false, concurrency_settings: ConcurrencySettings::default(), - debouncing_settings: DebouncingSettings::default(), labels: None, + debouncing_settings: DebouncingSettings::default(), + labels: None, }) .run_until_complete(&db, false, server.addr.port()) .await; @@ -380,7 +379,8 @@ async fn test_error_handler_not_triggered_on_success(db: Pool) -> anyh priority: None, apply_preprocessor: false, concurrency_settings: ConcurrencySettings::default(), - debouncing_settings: DebouncingSettings::default(), labels: None, + debouncing_settings: DebouncingSettings::default(), + labels: None, }) .run_until_complete(&db, false, server.addr.port()) .await; diff --git a/backend/tests/otel.rs b/backend/tests/otel.rs index 7b4f7c5ec4..2cf10b52d1 100644 --- a/backend/tests/otel.rs +++ b/backend/tests/otel.rs @@ -395,10 +395,7 @@ async fn test_root_job_span_created_on_success() { attrs.contains(&"script_path"), "missing script_path attribute" ); - assert!( - attrs.contains(&"job_kind"), - "missing job_kind attribute" - ); + assert!(attrs.contains(&"job_kind"), "missing job_kind attribute"); assert!( attrs.contains(&"created_by"), "missing created_by attribute" diff --git a/backend/tests/retry.rs b/backend/tests/retry.rs index cb77e4546b..abd7c38d8d 100644 --- a/backend/tests/retry.rs +++ b/backend/tests/retry.rs @@ -1,12 +1,12 @@ #[cfg(feature = "deno_core")] mod retry { - use windmill_test_utils::*; use serde_json::json; use sqlx::{Pool, Postgres}; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use windmill_common::flow_status::FlowStatusModule; use windmill_common::flows::FlowValue; use windmill_common::jobs::JobPayload; + use windmill_test_utils::*; pub async fn initialize_tracing() { use std::sync::Once; diff --git a/backend/tests/success_handler.rs b/backend/tests/success_handler.rs index fb28a10f04..fe6127c27e 100644 --- a/backend/tests/success_handler.rs +++ b/backend/tests/success_handler.rs @@ -179,7 +179,8 @@ export async function main(path: string, email: string, job_id: string, is_flow: priority: None, apply_preprocessor: false, concurrency_settings: ConcurrencySettings::default(), - debouncing_settings: DebouncingSettings::default(), labels: None, + debouncing_settings: DebouncingSettings::default(), + labels: None, }) .run_until_complete(&db, false, server.addr.port()) .await; diff --git a/backend/tests/suspend_resume.rs b/backend/tests/suspend_resume.rs index f2347af57a..b8468f0cbd 100644 --- a/backend/tests/suspend_resume.rs +++ b/backend/tests/suspend_resume.rs @@ -247,7 +247,9 @@ mod suspend_resume { #[cfg(feature = "enterprise")] #[cfg(feature = "deno_core")] #[sqlx::test(fixtures("base"))] - async fn test_self_approval_disabled_blocks_owner_resume(db: Pool) -> anyhow::Result<()> { + async fn test_self_approval_disabled_blocks_owner_resume( + db: Pool, + ) -> anyhow::Result<()> { initialize_tracing().await; let server = ApiServer::start(db.clone()).await?; @@ -358,7 +360,9 @@ mod suspend_resume { #[cfg(feature = "enterprise")] #[cfg(feature = "deno_core")] #[sqlx::test(fixtures("base"))] - async fn test_self_approval_allowed_when_not_disabled(db: Pool) -> anyhow::Result<()> { + async fn test_self_approval_allowed_when_not_disabled( + db: Pool, + ) -> anyhow::Result<()> { initialize_tracing().await; let server = ApiServer::start(db.clone()).await?; @@ -462,7 +466,9 @@ mod suspend_resume { #[cfg(feature = "enterprise")] #[cfg(feature = "deno_core")] #[sqlx::test(fixtures("base"))] - async fn test_different_user_can_approve_when_self_approval_disabled(db: Pool) -> anyhow::Result<()> { + async fn test_different_user_can_approve_when_self_approval_disabled( + db: Pool, + ) -> anyhow::Result<()> { initialize_tracing().await; let server = ApiServer::start(db.clone()).await?; diff --git a/backend/tests/worker.rs b/backend/tests/worker.rs index 8a729befd4..9f17f7223e 100644 --- a/backend/tests/worker.rs +++ b/backend/tests/worker.rs @@ -1860,6 +1860,937 @@ async fn test_postgresql_100_jobs_cached(db: Pool) -> anyhow::Result<( Ok(()) } +/// Cover the (Value × arg_t) combinations that #8988 broke. Each shape mirrors +/// what the windmill-client SDK or a hand-written PG script can emit: +/// +/// - bare `$N` with no inline cast and no `-- $N name (type)` declaration +/// (parser defaults the otyp to "text"). The user's value can be any JSON +/// shape; the eventual column type is whatever the SQL context implies. +/// - inline `$N::TYPE` casts (the SDK's default for bare `${value}`). +/// - `CAST($N AS T)` syntax (the SDK strips its own cast when this pattern +/// surrounds the value, so the parser sees a bare `$N`). +/// - explicit declaration: `-- $N name (type)`. +/// +/// Pre-fix, the dispatch asserted `Type::TEXT` for parser-defaulted args, so +/// e.g. a `Value::Bool` bound to `Box` failed at the encoder with +/// "cannot convert between the Rust type `bool` and the Postgres type `text`" +/// before the query ever reached the server. +#[sqlx::test(fixtures("base"))] +#[serial(pg_cache)] +async fn test_postgresql_arg_type_combinations(db: Pool) -> anyhow::Result<()> { + use windmill_worker::pg_executor::clear_pg_cache; + + initialize_tracing().await; + clear_pg_cache().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + let db_arg = json!({"host": "localhost", "port": 5432, "dbname": "windmill", "user": "postgres", "password": "changeme"}); + + // Build a fresh schema for this test so we don't trip over prior runs. + let setup = r#" +DROP SCHEMA IF EXISTS wm_pg_arg_combo_test CASCADE; +CREATE SCHEMA wm_pg_arg_combo_test; +CREATE TABLE wm_pg_arg_combo_test.bugbool (flag bool); +CREATE TABLE wm_pg_arg_combo_test.sdkbug (n int, f double precision); +CREATE TABLE wm_pg_arg_combo_test.bugmix (id int, name text, payload jsonb, tags text[]); +CREATE TABLE wm_pg_arg_combo_test.allcols ( + c_bool bool, + c_int2 smallint, + c_int4 int, + c_int8 bigint, + c_float4 real, + c_float8 double precision, + c_numeric numeric, + c_text text, + c_varchar varchar(64), + c_uuid uuid, + c_date date, + c_time time, + c_ts timestamp, + c_tstz timestamptz, + c_json json, + c_jsonb jsonb, + c_int_arr int[], + c_text_arr text[] +); +CREATE TYPE wm_pg_arg_combo_test.color AS ENUM ('red','green','blue'); +CREATE TABLE wm_pg_arg_combo_test.enumtbl (c wm_pg_arg_combo_test.color); +"#; + RunJob::from(JobPayload::Code(RawCode { + hash: None, + content: setup.to_owned(), + path: None, + lock: None, + language: ScriptLang::Postgresql, + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default() + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + modules: None, + })) + .arg("database", db_arg.clone()) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + + // Each case: (name, content, args, expected_result) + let cases: Vec<(&str, String, serde_json::Value, serde_json::Value)> = vec![ + // === parser-default "text" otyp (bare $N), value drives the binding === + ( + "bool via CAST AS bool (parser default text)", + "-- $1 arg1\nINSERT INTO wm_pg_arg_combo_test.bugbool VALUES (CAST($1 AS bool)) RETURNING flag".to_owned(), + json!({"arg1": true}), + json!([{"flag": true}]), + ), + ( + "bare $1 with bool into bool col", + "-- $1 arg1\nINSERT INTO wm_pg_arg_combo_test.bugbool VALUES ($1) RETURNING flag".to_owned(), + json!({"arg1": false}), + json!([{"flag": false}]), + ), + ( + "bare $1 with bool into text via implicit cast bool->text", + "-- $1 arg1\nSELECT $1::text AS s".to_owned(), + json!({"arg1": true}), + json!([{"s": "true"}]), + ), + ( + "bare $1 with int into text via implicit cast int->text", + "-- $1 arg1\nSELECT $1::text AS s".to_owned(), + json!({"arg1": 42}), + json!([{"s": "42"}]), + ), + ( + "object via CAST AS jsonb (parser default text)", + "-- $1 arg1\nSELECT CAST($1 AS jsonb) AS v".to_owned(), + json!({"arg1": {"k": 1}}), + json!([{"v": {"k": 1}}]), + ), + ( + "string '42' via CAST AS int (parser default text)", + "-- $1 arg1\nSELECT CAST($1 AS int) AS v".to_owned(), + json!({"arg1": "42"}), + json!([{"v": 42}]), + ), + ( + "NULL into bool col via CAST", + "-- $1 arg1\nINSERT INTO wm_pg_arg_combo_test.bugbool VALUES (CAST($1 AS bool)) RETURNING flag".to_owned(), + json!({"arg1": null}), + json!([{"flag": null}]), + ), + // === SDK happy path — inline ::TYPE injected by the client === + ( + "SDK shape: $1::BIGINT, $2::DOUBLE PRECISION", + "-- $1 arg1\n-- $2 arg2\nINSERT INTO wm_pg_arg_combo_test.sdkbug VALUES ($1::BIGINT, $2::DOUBLE PRECISION) RETURNING n, f".to_owned(), + json!({"arg1": 42, "arg2": 3.14}), + json!([{"n": 42, "f": 3.14}]), + ), + ( + "SDK shape: $1::TEXT with int target via explicit ::int", + "-- $1 arg1\nSELECT $1::TEXT::int AS v".to_owned(), + json!({"arg1": "7"}), + json!([{"v": 7}]), + ), + // === explicit declaration -- $N name (type) === + ( + "explicit decl (text)", + "-- $1 arg1 (text)\nSELECT $1 AS v".to_owned(), + json!({"arg1": "hello"}), + json!([{"v": "hello"}]), + ), + ( + "explicit decl (jsonb) with object", + "-- $1 arg1 (jsonb)\nSELECT $1 AS v".to_owned(), + json!({"arg1": {"k": [1, 2]}}), + json!([{"v": {"k": [1, 2]}}]), + ), + // === mixed args: int, text, jsonb, text[] === + ( + "mixed: int + text + jsonb + text[]", + "-- $1 arg1\n-- $2 arg2\n-- $3 arg3\n-- $4 arg4\nINSERT INTO wm_pg_arg_combo_test.bugmix VALUES ($1::int, $2::text, $3::jsonb, $4::text[]) RETURNING *".to_owned(), + json!({"arg1": 7, "arg2": "hello", "arg3": {"k": 1}, "arg4": ["a", "b"]}), + json!([{"id": 7, "name": "hello", "payload": {"k": 1}, "tags": ["a", "b"]}]), + ), + + // === Every PG type, SDK happy path (inline ::TYPE) === + ( + "all PG types via inline casts", + r#"-- $1 arg1 +-- $2 arg2 +-- $3 arg3 +-- $4 arg4 +-- $5 arg5 +-- $6 arg6 +-- $7 arg7 +-- $8 arg8 +-- $9 arg9 +-- $10 arg10 +-- $11 arg11 +-- $12 arg12 +-- $13 arg13 +-- $14 arg14 +-- $15 arg15 +-- $16 arg16 +-- $17 arg17 +-- $18 arg18 +INSERT INTO wm_pg_arg_combo_test.allcols VALUES ( + $1::bool, $2::int2, $3::int4, $4::int8, + $5::real, $6::double precision, $7::numeric, + $8::text, $9::varchar, + $10::uuid, $11::date, $12::time, $13::timestamp, $14::timestamptz, + $15::json, $16::jsonb, + $17::int[], $18::text[] +) RETURNING c_bool, c_int4, c_int8, c_text, c_uuid, c_int_arr"#.to_owned(), + json!({ + "arg1": true, "arg2": 1, "arg3": 2, "arg4": 3, + "arg5": 1.5, "arg6": 2.5, "arg7": 3.14, + "arg8": "hello", "arg9": "varhello", + "arg10": "550e8400-e29b-41d4-a716-446655440000", + "arg11": "2024-01-15", "arg12": "10:30:00", + "arg13": "2024-01-15T10:30:00", "arg14": "2024-01-15T10:30:00Z", + "arg15": {"k": 1}, "arg16": {"k": 2}, + "arg17": [1,2,3], "arg18": ["a","b"] + }), + json!([{"c_bool": true, "c_int4": 2, "c_int8": 3, "c_text": "hello", + "c_uuid": "550e8400-e29b-41d4-a716-446655440000", + "c_int_arr": [1,2,3]}]), + ), + + // === Every PG type, declaration-style otyp (Python SDK shape) === + ( + "all PG types via -- $N name (TYPE) decls", + r#"-- $1 arg1 (bool) +-- $2 arg2 (int2) +-- $3 arg3 (int4) +-- $4 arg4 (int8) +-- $5 arg5 (real) +-- $6 arg6 (float8) +-- $7 arg7 (numeric) +-- $8 arg8 (text) +-- $9 arg9 (varchar) +-- $10 arg10 (uuid) +-- $11 arg11 (date) +-- $12 arg12 (time) +-- $13 arg13 (timestamp) +-- $14 arg14 (timestamptz) +-- $15 arg15 (json) +-- $16 arg16 (jsonb) +-- $17 arg17 (int[]) +-- $18 arg18 (text[]) +INSERT INTO wm_pg_arg_combo_test.allcols VALUES ( + $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18 +) RETURNING c_bool, c_int4, c_int8, c_text, c_int_arr, c_text_arr"#.to_owned(), + json!({ + "arg1": false, "arg2": 4, "arg3": 5, "arg4": 6, + "arg5": 1.5, "arg6": 2.5, "arg7": 3.14, + "arg8": "world", "arg9": "varworld", + "arg10": "550e8400-e29b-41d4-a716-446655440000", + "arg11": "2024-01-15", "arg12": "10:30:00", + "arg13": "2024-01-15T10:30:00", "arg14": "2024-01-15T10:30:00Z", + "arg15": [1,2], "arg16": [3,4], + "arg17": [10,20], "arg18": ["x","y"] + }), + json!([{"c_bool": false, "c_int4": 5, "c_int8": 6, "c_text": "world", + "c_int_arr": [10,20], "c_text_arr": ["x","y"]}]), + ), + + // === Edge values per type === + ("int8 negative", "-- $1 arg1\nSELECT $1::int8 AS v".to_owned(), json!({"arg1": -42}), json!([{"v": -42}])), + ("int8 zero", "-- $1 arg1\nSELECT $1::int8 AS v".to_owned(), json!({"arg1": 0}), json!([{"v": 0}])), + ("int4 max", "-- $1 arg1\nSELECT $1::int4 AS v".to_owned(), json!({"arg1": 2147483647i64}), json!([{"v": 2147483647i64}])), + ("int8 max", "-- $1 arg1\nSELECT $1::int8 AS v".to_owned(), json!({"arg1": 9223372036854775807i64}), json!([{"v": 9223372036854775807i64}])), + ("float8 fraction", "-- $1 arg1\nSELECT $1::float8 AS v".to_owned(), json!({"arg1": 0.1 + 0.2}), json!([{"v": 0.1 + 0.2}])), + ("empty string", "-- $1 arg1\nSELECT $1::text AS v".to_owned(), json!({"arg1": ""}), json!([{"v": ""}])), + ("empty array", "-- $1 arg1\nSELECT $1::int[] AS v".to_owned(), json!({"arg1": []}), json!([{"v": []}])), + ("empty object", "-- $1 arg1\nSELECT $1::jsonb AS v".to_owned(), json!({"arg1": {}}), json!([{"v": {}}])), + + // === Bool roundtrip across every shape === + ("Bool/bare $1 → bool col", "-- $1 arg1\nSELECT $1 AS v".to_owned(), json!({"arg1": true}), json!([{"v": true}])), + ("Bool/inline ::bool", "-- $1 arg1\nSELECT $1::bool AS v".to_owned(), json!({"arg1": true}), json!([{"v": true}])), + ("Bool/decl (bool)", "-- $1 arg1 (bool)\nSELECT $1 AS v".to_owned(), json!({"arg1": true}), json!([{"v": true}])), + ("Bool/CAST AS bool", "-- $1 arg1\nSELECT CAST($1 AS bool) AS v".to_owned(), json!({"arg1": false}), json!([{"v": false}])), + ("Bool/CAST AS bool inside SELECT","-- $1 arg1\nSELECT CAST($1 AS bool) AS v WHERE CAST($1 AS bool) IS NOT NULL".to_owned(), json!({"arg1": true}), json!([{"v": true}])), + + // === Object/Array via CAST AS jsonb (regression: non-text otyp via CAST) === + ("Object via CAST AS jsonb", "-- $1 arg1\nSELECT CAST($1 AS jsonb) AS v".to_owned(), json!({"arg1": {"a":1,"b":[2,3]}}), json!([{"v": {"a":1,"b":[2,3]}}])), + ("Array via CAST AS jsonb", "-- $1 arg1\nSELECT CAST($1 AS jsonb) AS v".to_owned(), json!({"arg1": [1,"two",{"three":3}]}), json!([{"v": [1,"two",{"three":3}]}])), + ("Object inline ::json", "-- $1 arg1\nSELECT $1::json AS v".to_owned(), json!({"arg1": {"k":1}}), json!([{"v": {"k":1}}])), + ("Object decl (jsonb)", "-- $1 arg1 (jsonb)\nSELECT $1 AS v".to_owned(), json!({"arg1": {"k":1}}), json!([{"v": {"k":1}}])), + + // === Numbers: implicit/explicit casts to text === + ("Number/CAST AS text", "-- $1 arg1\nSELECT CAST($1 AS text) AS v".to_owned(), json!({"arg1": 42}), json!([{"v": "42"}])), + ("Number/inline ::text", "-- $1 arg1\nSELECT $1::text AS v".to_owned(), json!({"arg1": 42}), json!([{"v": "42"}])), + ("Float/CAST AS text", "-- $1 arg1\nSELECT CAST($1 AS text) AS v".to_owned(), json!({"arg1": 3.14}), json!([{"v": "3.14"}])), + ("Negative/CAST AS int8", "-- $1 arg1\nSELECT CAST($1 AS int8) AS v".to_owned(), json!({"arg1": -1}), json!([{"v": -1}])), + + // === Strings parsed into typed targets === + ("String '42' as int", "-- $1 arg1\nSELECT $1::int AS v".to_owned(), json!({"arg1": "42"}), json!([{"v": 42}])), + ("String '42' as bigint", "-- $1 arg1\nSELECT $1::bigint AS v".to_owned(), json!({"arg1": "42"}), json!([{"v": 42}])), + ("String '1.5' as real", "-- $1 arg1\nSELECT $1::real AS v".to_owned(), json!({"arg1": "1.5"}), json!([{"v": 1.5}])), + ("String uuid", "-- $1 arg1\nSELECT $1::uuid AS v".to_owned(), json!({"arg1": "550e8400-e29b-41d4-a716-446655440000"}), json!([{"v": "550e8400-e29b-41d4-a716-446655440000"}])), + + // === NULL handling for every type === + ("Null/bool", "-- $1 arg1\nSELECT CAST($1 AS bool) AS v".to_owned(), json!({"arg1": null}), json!([{"v": null}])), + ("Null/int", "-- $1 arg1\nSELECT CAST($1 AS int) AS v".to_owned(), json!({"arg1": null}), json!([{"v": null}])), + ("Null/bigint", "-- $1 arg1\nSELECT CAST($1 AS bigint) AS v".to_owned(), json!({"arg1": null}), json!([{"v": null}])), + ("Null/text", "-- $1 arg1\nSELECT $1::text AS v".to_owned(), json!({"arg1": null}), json!([{"v": null}])), + ("Null/jsonb", "-- $1 arg1\nSELECT CAST($1 AS jsonb) AS v".to_owned(), json!({"arg1": null}), json!([{"v": null}])), + ("Null/uuid", "-- $1 arg1\nSELECT CAST($1 AS uuid) AS v".to_owned(), json!({"arg1": null}), json!([{"v": null}])), + ("Null/timestamp", "-- $1 arg1\nSELECT CAST($1 AS timestamp) AS v".to_owned(), json!({"arg1": null}), json!([{"v": null}])), + ("Null/date", "-- $1 arg1\nSELECT CAST($1 AS date) AS v".to_owned(), json!({"arg1": null}), json!([{"v": null}])), + + // === Multi-statement (datatable scripts often DROP/CREATE/INSERT) === + ( + "multi-statement: DROP/CREATE/INSERT", + r#"DROP TABLE IF EXISTS wm_pg_arg_combo_test.tmp_multi; +CREATE TABLE wm_pg_arg_combo_test.tmp_multi (n int, b bool); +-- $1 arg1 +-- $2 arg2 +INSERT INTO wm_pg_arg_combo_test.tmp_multi VALUES ($1::int, $2::bool) RETURNING *"#.to_owned(), + json!({"arg1": 10, "arg2": true}), + json!([{"n": 10, "b": true}]), + ), + + // === Same arg used in multiple positions (parser reorders) === + ( + "same arg twice", + "-- $1 arg1\nSELECT $1::int + $1::int AS v".to_owned(), + json!({"arg1": 5}), + json!([{"v": 10}]), + ), + + // === Custom enum (unrecognised arg_t → prepare fallback path) === + // The unrecognised-arg_t fallback works when the user formats the + // value as the enum's text representation themselves, so the binding + // never has to encode a Rust String *as* an enum (which the + // tokio-postgres `ToSql` impls don't support): + ( + "custom enum: text representation cast in SQL", + r#"-- $1 arg1 +INSERT INTO wm_pg_arg_combo_test.enumtbl +VALUES (CAST($1::text AS wm_pg_arg_combo_test.color)) +RETURNING c::text AS c"# + .to_owned(), + json!({"arg1": "green"}), + json!([{"c": "green"}]), + ), + + // === Sparse positional placeholders ($5, $50) === + // The pre-fix `String::replace($5 → $1)` chain mangled `$50` into + // `$10`, breaking sparse-index queries. The regex-based renumbering + // handles them as distinct units. + ( + "sparse $5 / $50 renumbering", + "-- $5 arg5\n-- $50 arg50\nSELECT $5::int AS a, $50::int AS b".to_owned(), + json!({"arg5": 5, "arg50": 50}), + json!([{"a": 5, "b": 50}]), + ), + ( + "sparse $5 / $50 reversed in SQL", + "-- $5 arg5\n-- $50 arg50\nSELECT $50::int AS a, $5::int AS b".to_owned(), + json!({"arg5": 5, "arg50": 50}), + json!([{"a": 50, "b": 5}]), + ), + ( + "sparse same arg used twice + sparse", + "-- $5 arg5\n-- $50 arg50\nSELECT $5::int + $5::int AS a, $50::int AS b".to_owned(), + json!({"arg5": 7, "arg50": 50}), + json!([{"a": 14, "b": 50}]), + ), + + // === Explicit (text) decl + non-string value: should coerce === + // Without the otyp_inferred flag, the executor would bind the value's + // natural type (INT8/BOOL) and the WHERE comparison `text = int8` / + // `text = bool` would fail with "operator does not exist". With the + // flag, the parser tells the executor "user committed to text" and + // the value is JSON-stringified so `text = text` works. + ( + "decl (text) + Number used in WHERE text comparison", + r#"-- $1 arg1 (text) +SELECT name FROM (VALUES ('42'::text)) AS t(name) WHERE name = $1"# + .to_owned(), + json!({"arg1": 42}), + json!([{"name": "42"}]), + ), + ( + "decl (text) + Bool used in WHERE text comparison", + r#"-- $1 arg1 (text) +SELECT name FROM (VALUES ('true'::text)) AS t(name) WHERE name = $1"# + .to_owned(), + json!({"arg1": true}), + json!([{"name": "true"}]), + ), + ( + "decl (varchar) + Number used in WHERE", + r#"-- $1 arg1 (varchar) +SELECT name FROM (VALUES ('99'::varchar)) AS t(name) WHERE name = $1"# + .to_owned(), + json!({"arg1": 99}), + json!([{"name": "99"}]), + ), + + // === Bare $N (parser-default text) + non-string value: bind native === + // The user wrote no annotation — we bind the value's natural type so + // it works against whatever column the SQL eventually targets. + ( + "bare $1 + Bool into bool col", + "-- $1 arg1\nSELECT $1 = true AS v".to_owned(), + json!({"arg1": true}), + json!([{"v": true}]), + ), + + // === Custom enum (Kind::Enum) — round-trip via AnyTextValue === + // Pre-fix this failed at the encoder ("cannot convert String → color") + // because vanilla tokio_postgres' ToSql/FromSql for String reject + // Kind::Enum. The wrapper accepts enum kinds in both directions. + ( + "enum: explicit ::wm_pg_arg_combo_test.color cast", + r#"-- $1 arg1 +INSERT INTO wm_pg_arg_combo_test.enumtbl +VALUES ($1::wm_pg_arg_combo_test.color) RETURNING c"# + .to_owned(), + json!({"arg1": "blue"}), + json!([{"c": "blue"}]), + ), + ( + "enum: SELECT a literal value cast to enum", + "-- $1 arg1\nSELECT $1::wm_pg_arg_combo_test.color AS c".to_owned(), + json!({"arg1": "red"}), + json!([{"c": "red"}]), + ), + + // === Extended String→numeric/real/double/oid/bool arms (#10) === + ( + "String '3.14' → numeric", + "-- $1 arg1\nSELECT $1::numeric AS v".to_owned(), + json!({"arg1": "3.14"}), + json!([{"v": 3.14}]), + ), + ( + "String '1.5' → real", + "-- $1 arg1\nSELECT $1::real AS v".to_owned(), + json!({"arg1": "1.5"}), + json!([{"v": 1.5}]), + ), + ( + "String '2.5' → double", + "-- $1 arg1\nSELECT $1::double precision AS v".to_owned(), + json!({"arg1": "2.5"}), + json!([{"v": 2.5}]), + ), + ( + "String 'true' → bool", + "-- $1 arg1\nSELECT $1::bool AS v".to_owned(), + json!({"arg1": "true"}), + json!([{"v": true}]), + ), + ( + "String 't' → bool", + "-- $1 arg1\nSELECT $1::bool AS v".to_owned(), + json!({"arg1": "t"}), + json!([{"v": true}]), + ), + ( + "String '0' → bool false", + "-- $1 arg1\nSELECT $1::bool AS v".to_owned(), + json!({"arg1": "0"}), + json!([{"v": false}]), + ), + ( + "String '42' → oid", + "-- $1 arg1\nSELECT $1::oid AS v".to_owned(), + json!({"arg1": "42"}), + json!([{"v": 42}]), + ), + + // === String literals containing $N must NOT be renumbered === + // Sparse positional args force a rewrite pass; the literal + // `'price: $5'` and the comment `-- mention $5` must survive intact. + ( + "renumber must skip $N inside string literal", + r#"-- $5 arg5 +-- $50 arg50 +SELECT 'price: $5' AS lbl, $5::int + $50::int AS sum"# + .to_owned(), + json!({"arg5": 1, "arg50": 2}), + json!([{"lbl": "price: $5", "sum": 3}]), + ), + + // === Multi-word PG type names with [] array suffix === + // Pre-fix: `transform_types_with_spaces` returned a `&str` alias + // ("double" / "varchar" / "timestamptz" / …) and dropped the trailing + // `[]`, so the dispatch routed `Value::Array` through `Type::JSONB` + // and the server failed with "cannot cast type jsonb to []". + ( + "multi-word array: double precision[]", + "-- $1 a\nSELECT $1::double precision[] AS v".to_owned(), + json!({"a": [1.5, 2.5]}), + json!([{"v": [1.5, 2.5]}]), + ), + ( + "multi-word array: character varying[]", + "-- $1 a\nSELECT $1::character varying[] AS v".to_owned(), + json!({"a": ["x", "y"]}), + json!([{"v": ["x", "y"]}]), + ), + ( + "multi-word array: timestamp without time zone[]", + "-- $1 a\nSELECT $1::timestamp without time zone[] AS v".to_owned(), + json!({"a": ["2024-01-15T10:30:00"]}), + json!([{"v": ["2024-01-15T10:30:00"]}]), + ), + + // === Stringified primitives in array args === + // Mirror the scalar `Value::String → ` arms so values + // sent as e.g. `["1.5", "2.5"]` for `numeric[]` (typical for bulk- + // loading via `unnest` or BigInt-stringified arrays) round-trip + // instead of erroring with "Mixed types in array". + ( + "numeric[] from stringified decimals", + "-- $1 a\nSELECT $1::numeric[] AS v".to_owned(), + json!({"a": ["1.5", "2.5"]}), + json!([{"v": [1.5, 2.5]}]), + ), + ( + "int[] from stringified ints", + "-- $1 a\nSELECT $1::int[] AS v".to_owned(), + json!({"a": ["1", "2", "3"]}), + json!([{"v": [1, 2, 3]}]), + ), + ( + "bool[] from stringified bools", + "-- $1 a\nSELECT $1::bool[] AS v".to_owned(), + json!({"a": ["true", "f", "yes"]}), + json!([{"v": [true, false, true]}]), + ), + ]; + + for (name, content, args, expected) in cases { + let mut job = RunJob::from(JobPayload::Code(RawCode { + hash: None, + content, + path: None, + lock: None, + language: ScriptLang::Postgresql, + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default( + ) + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + modules: None, + })) + .arg("database", db_arg.clone()); + for (k, v) in args.as_object().unwrap() { + job = job.arg(k, v.clone()); + } + let result = job + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap_or_else(|| panic!("case '{name}': no json result")); + assert_eq!(result, expected, "case '{name}' mismatch"); + } + + Ok(()) +} + +/// Pooler-safety regression test for #8988: when every arg has a resolvable +/// otyp (the common SDK case), the dispatch must use unnamed prepared +/// statements (`query_typed_raw`) and *must not* leak named statements +/// (`s0, s1, ...`) on the cached connection. Behind a transaction-mode pooler +/// (PgBouncer / Supabase pooler / RDS Proxy), accumulated names get dropped +/// when the prepare and execute land on different backend connections, which +/// is what produced the original "prepared statement \"sN\" does not exist" +/// errors. +#[sqlx::test(fixtures("base"))] +#[serial(pg_cache)] +async fn test_postgresql_no_named_statements_after_typed_args( + db: Pool, +) -> anyhow::Result<()> { + use windmill_worker::pg_executor::clear_pg_cache; + + initialize_tracing().await; + clear_pg_cache().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + let db_arg = json!({"host": "localhost", "port": 5432, "dbname": "windmill", "user": "postgres", "password": "changeme"}); + + let make_pg_job = |content: String| { + RunJob::from(JobPayload::Code(RawCode { + hash: None, + content, + path: None, + lock: None, + language: ScriptLang::Postgresql, + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default( + ) + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + modules: None, + })) + .arg("database", db_arg.clone()) + }; + + // Run several SDK-shape queries (parameterised, all args resolvable) on the + // same cached connection. None of them should land in `pg_prepared_statements`. + for i in 0..5 { + let result = make_pg_job(format!( + "-- $1 arg1\n-- $2 arg2\nSELECT $1::BIGINT AS a, $2::TEXT AS b, {} AS i;", + i + )) + .arg("arg1", json!(i)) + .arg("arg2", json!(format!("v{i}"))) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + assert_eq!( + result, + json!([{"a": i, "b": format!("v{i}"), "i": i}]), + "iteration {i}" + ); + } + + // Use the *same* connection (cached one) to peek at pg_prepared_statements. + // Anything matching the tokio-postgres "s\d+" naming would mean the + // pooler-unsafe `prepare + query_raw` path was taken. + let probe = make_pg_job( + "SELECT count(*)::int AS n FROM pg_prepared_statements WHERE name ~ '^s[0-9]+$'".to_owned(), + ) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + let n = probe[0]["n"].as_i64().unwrap_or(-1); + assert_eq!( + n, 0, + "expected no leaked named prepared statements, found {n}" + ); + + Ok(()) +} + +/// Exercise the `prepare + query_raw` fallback path. The dispatch takes this +/// path when `otyp_to_pg_type` doesn't recognise the parser-derived arg_t — +/// typical for custom enums, domains, and extension types. +/// +/// Vanilla `tokio_postgres`'s `ToSql for String` doesn't actually accept +/// `Kind::Enum` / `Kind::Domain`, so an end-to-end happy-path test of an +/// arbitrary custom type isn't possible without `postgres-derive`. What we +/// CAN lock in here is *which dispatch path runs*: when the arg_t is +/// unrecognised, the prepare path must be taken (the server resolves the +/// param type from the cast and the binding then errors at the encoder). +/// If a regression accidentally routes unrecognised types through +/// `query_typed_raw`, the failure mode flips: instead of "cannot convert +/// `String` to ``" we'd see "cannot convert `String` to +/// `text`" (because we'd assert TEXT). The error-text check below catches +/// that flip. +#[sqlx::test(fixtures("base"))] +#[serial(pg_cache)] +async fn test_postgresql_prepare_fallback_for_unrecognised_arg_t( + db: Pool, +) -> anyhow::Result<()> { + use windmill_worker::pg_executor::clear_pg_cache; + + initialize_tracing().await; + clear_pg_cache().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + let db_arg = json!({"host": "localhost", "port": 5432, "dbname": "windmill", "user": "postgres", "password": "changeme"}); + + let make_pg_job = |content: String| { + RunJob::from(JobPayload::Code(RawCode { + hash: None, + content, + path: None, + lock: None, + language: ScriptLang::Postgresql, + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default( + ) + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + modules: None, + })) + .arg("database", db_arg.clone()) + }; + + // Set up a custom enum in a dedicated schema so the test is self-contained. + let setup = r#" +DROP SCHEMA IF EXISTS fallback_test CASCADE; +CREATE SCHEMA fallback_test; +CREATE TYPE fallback_test.color AS ENUM ('red', 'green', 'blue'); +"#; + make_pg_job(setup.to_owned()) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + + // Inline cast `::fallback_test.color` — the parser only captures + // `fallback_test` (regex stops at the dot), which otyp_to_pg_type won't + // recognise. Dispatch must take the prepare fallback path. + // + // Thanks to the AnyTextValue ToSql/FromSql wrapper, this case now + // round-trips end-to-end (the wrapper accepts Kind::Enum on both + // directions). Pre-fix, vanilla tokio_postgres rejected String → color + // and the user had to write `CAST($1::text AS color)` as a workaround. + let result = make_pg_job("-- $1 arg1\nSELECT $1::fallback_test.color AS c".to_owned()) + .arg("arg1", json!("red")) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + assert_eq!(result, json!([{"c": "red"}])); + + // Reading enum columns also goes through AnyTextValue's FromSql impl on + // the result side, so the value comes back as a JSON string. + let result = make_pg_job( + r#"-- $1 arg1 +SELECT $1::fallback_test.color AS c1, + 'green'::fallback_test.color AS c2"# + .to_owned(), + ) + .arg("arg1", json!("blue")) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + assert_eq!(result, json!([{"c1": "blue", "c2": "green"}])); + + Ok(()) +} + +/// Regression: custom enum / domain queries must not error with `prepared +/// statement "sN" does not exist` when run on a cached connection. +/// +/// Root cause: when we used `DISCARD ALL` to reset the cached connection +/// between jobs, the included `DEALLOCATE ALL` deallocated *every* prepared +/// statement server-side — including the typeinfo statements that +/// tokio_postgres caches per-client to resolve custom-type Oids. The Rust +/// client still held `Statement` objects whose names the server had +/// forgotten, so the next custom-type query failed. +/// +/// Fix: switched the cached-connection probe to `RESET ALL; UNLISTEN *; +/// CLOSE ALL;` which covers windmill's session-isolation needs (GUC reset, +/// listen channels, open cursors) without nuking the prepared-statement +/// cache. This test runs the failing pattern (enum query → domain query on +/// the same cached connection, several times) to lock the behaviour in. +#[sqlx::test(fixtures("base"))] +#[serial(pg_cache)] +async fn test_postgresql_custom_types_on_cached_connection( + db: Pool, +) -> anyhow::Result<()> { + use windmill_worker::pg_executor::clear_pg_cache; + + initialize_tracing().await; + clear_pg_cache().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + let db_arg = json!({"host": "localhost", "port": 5432, "dbname": "windmill", "user": "postgres", "password": "changeme"}); + + let make_pg_job = |content: String| { + RunJob::from(JobPayload::Code(RawCode { + hash: None, + content, + path: None, + lock: None, + language: ScriptLang::Postgresql, + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default( + ) + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + modules: None, + })) + .arg("database", db_arg.clone()) + }; + + // Set up custom enum + domain types in a dedicated schema (avoid the + // `pg_*` reserved prefix for user schemas). + let setup = r#" +DROP SCHEMA IF EXISTS wm_pg_cached_test CASCADE; +CREATE SCHEMA wm_pg_cached_test; +CREATE TYPE wm_pg_cached_test.color AS ENUM ('red', 'green', 'blue'); +CREATE DOMAIN wm_pg_cached_test.short_name AS TEXT CHECK (length(VALUE) BETWEEN 1 AND 10); +"#; + make_pg_job(setup.to_owned()) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + + // Run a long alternating sequence of enum + domain queries on the same + // cached connection. Each query goes through the prepare-fallback path + // (otyp_to_pg_type doesn't recognise these custom-type names) and needs + // tokio_postgres' typeinfo cache to resolve the Oids server-side. Pre-fix + // this would fail intermittently with `prepared statement "sN" does not + // exist` after the first cached-conn reuse. + for i in 0..10 { + let r = make_pg_job("-- $1 arg1\nSELECT $1::wm_pg_cached_test.color AS c".to_owned()) + .arg("arg1", json!(if i % 2 == 0 { "red" } else { "blue" })) + .run_until_complete(&db, false, port) + .await; + assert!( + r.success, + "iter {i} enum query failed (was the DISCARD ALL bug); result: {:?}", + r.result + ); + + let r = make_pg_job("-- $1 arg1\nSELECT $1::wm_pg_cached_test.short_name AS s".to_owned()) + .arg("arg1", json!(format!("v{i}"))) + .run_until_complete(&db, false, port) + .await; + assert!( + r.success, + "iter {i} domain query failed (was the DISCARD ALL bug); result: {:?}", + r.result + ); + } + + Ok(()) +} + +/// Security regression: a previous job that did `SET ROLE` or `SET SESSION +/// AUTHORIZATION` to a different role must not leak that role into the next +/// job that reuses the cached connection. +/// +/// This is the case `RESET ALL` alone does *not* cover — neither SET ROLE +/// nor SET SESSION AUTHORIZATION are GUC parameters, so they survive +/// `RESET ALL`. We rely on `RESET SESSION AUTHORIZATION` (which subsumes +/// `RESET ROLE`) explicitly being part of the cached-connection probe. +/// +/// The pre-existing `test_postgresql_single_worker_session_isolation` test +/// only did `SET ROLE postgres` (the connecting user), so the leak was +/// invisible — this test catches it by switching to a *different* role. +#[sqlx::test(fixtures("base"))] +#[serial(pg_cache)] +async fn test_postgresql_set_role_does_not_leak_across_cached_connection( + db: Pool, +) -> anyhow::Result<()> { + use windmill_worker::pg_executor::clear_pg_cache; + + initialize_tracing().await; + clear_pg_cache().await; + + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + + let db_arg = json!({"host": "localhost", "port": 5432, "dbname": "windmill", "user": "postgres", "password": "changeme"}); + + let make_pg_job = |content: String| { + RunJob::from(JobPayload::Code(RawCode { + hash: None, + content, + path: None, + lock: None, + language: ScriptLang::Postgresql, + cache_ttl: None, + cache_ignore_s3_path: None, + dedicated_worker: None, + concurrency_settings: windmill_common::runnable_settings::ConcurrencySettings::default( + ) + .into(), + debouncing_settings: windmill_common::runnable_settings::DebouncingSettings::default(), + modules: None, + })) + .arg("database", db_arg.clone()) + }; + + // Create a non-postgres role to switch to. Idempotent so the test survives + // re-runs against the same DB. + make_pg_job( + "DO $$ BEGIN \ + IF NOT EXISTS (SELECT FROM pg_roles WHERE rolname = 'wm_isolation_test_role') THEN \ + CREATE ROLE wm_isolation_test_role; \ + END IF; \ + END $$" + .to_owned(), + ) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + + // Job 1: SET ROLE to a different role. + let r1 = make_pg_job( + "SET ROLE wm_isolation_test_role; SELECT current_user AS cu, session_user AS su".to_owned(), + ) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + assert_eq!(r1[0]["cu"], "wm_isolation_test_role"); + assert_eq!(r1[0]["su"], "postgres"); + + // Job 2 (cached connection reuse): role MUST be back to the connecting + // user. Pre-fix with `RESET ALL` alone, this would still see + // `wm_isolation_test_role` because RESET ALL doesn't cover SET ROLE. + let r2 = make_pg_job("SELECT current_user AS cu, session_user AS su".to_owned()) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + assert_eq!( + r2[0]["cu"], "postgres", + "SET ROLE leaked across cached connection: current_user is {}", + r2[0]["cu"] + ); + + // Job 3: SET SESSION AUTHORIZATION (changes both current_user and + // session_user — RESET ALL does NOT touch this either). + let r3 = make_pg_job( + "SET SESSION AUTHORIZATION wm_isolation_test_role; \ + SELECT current_user AS cu, session_user AS su" + .to_owned(), + ) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + assert_eq!(r3[0]["cu"], "wm_isolation_test_role"); + assert_eq!(r3[0]["su"], "wm_isolation_test_role"); + + // Job 4 (cached): both must be restored to the connecting user. + let r4 = make_pg_job("SELECT current_user AS cu, session_user AS su".to_owned()) + .run_until_complete(&db, false, port) + .await + .json_result() + .unwrap(); + assert_eq!( + r4[0]["cu"], "postgres", + "SET SESSION AUTHORIZATION leaked across cached connection: current_user is {}", + r4[0]["cu"] + ); + assert_eq!( + r4[0]["su"], "postgres", + "SET SESSION AUTHORIZATION leaked across cached connection: session_user is {}", + r4[0]["su"] + ); + + Ok(()) +} + #[cfg(feature = "mysql")] #[sqlx::test(fixtures("base"))] async fn test_mysql_job(db: Pool) -> anyhow::Result<()> { diff --git a/backend/tests/workspace_dependencies.rs b/backend/tests/workspace_dependencies.rs index 4f1023d54d..773a4fd965 100644 --- a/backend/tests/workspace_dependencies.rs +++ b/backend/tests/workspace_dependencies.rs @@ -1,12 +1,12 @@ mod workspace_dependencies { - use windmill_test_utils::in_test_worker; - use windmill_test_utils::init_client; - use windmill_test_utils::listen_for_completed_jobs; use sqlx::{Pool, Postgres}; use tokio_stream::StreamExt; use windmill_common::scripts::ScriptLang; use windmill_dep_map::workspace_dependencies::NewWorkspaceDependencies; + use windmill_test_utils::in_test_worker; + use windmill_test_utils::init_client; + use windmill_test_utils::listen_for_completed_jobs; mod deps { pub const REQUIREMENTS_IN: &'static str = "tiny==0.1.3"; // pub const GO_MOD: &'static str = r##" diff --git a/backend/windmill-worker/src/ai/providers/anthropic.rs b/backend/windmill-worker/src/ai/providers/anthropic.rs index 7d8b0a3db8..2bbfd83768 100644 --- a/backend/windmill-worker/src/ai/providers/anthropic.rs +++ b/backend/windmill-worker/src/ai/providers/anthropic.rs @@ -371,11 +371,7 @@ pub struct AnthropicQueryBuilder { } impl AnthropicQueryBuilder { - pub fn new( - provider_kind: AIProvider, - platform: AIPlatform, - enable_1m_context: bool, - ) -> Self { + pub fn new(provider_kind: AIProvider, platform: AIPlatform, enable_1m_context: bool) -> Self { Self { provider_kind, platform, enable_1m_context } } diff --git a/backend/windmill-worker/src/ai/providers/google_ai.rs b/backend/windmill-worker/src/ai/providers/google_ai.rs index 50284597b8..6098cb22d8 100644 --- a/backend/windmill-worker/src/ai/providers/google_ai.rs +++ b/backend/windmill-worker/src/ai/providers/google_ai.rs @@ -1,8 +1,8 @@ use async_trait::async_trait; use windmill_ai::ai_google::{ - openai_messages_to_gemini, openai_tools_to_gemini, GeminiGenerationConfig, - GeminiImageContent, GeminiImageRequest, GeminiImageResponse, GeminiInlineData, GeminiPart, - GeminiPredictContent, GeminiTextRequest, GeminiTool, + openai_messages_to_gemini, openai_tools_to_gemini, GeminiGenerationConfig, GeminiImageContent, + GeminiImageRequest, GeminiImageResponse, GeminiInlineData, GeminiPart, GeminiPredictContent, + GeminiTextRequest, GeminiTool, }; use windmill_common::{client::AuthedClient, error::Error}; @@ -135,7 +135,10 @@ impl GoogleAIQueryBuilder { openai_tools_to_gemini(tool_defs, &tool_params, has_websearch) } - fn build_generation_config(&self, args: &BuildRequestArgs<'_>) -> Option { + fn build_generation_config( + &self, + args: &BuildRequestArgs<'_>, + ) -> Option { let has_output_schema = args .output_schema .and_then(|s| s.properties.as_ref()) @@ -145,7 +148,10 @@ impl GoogleAIQueryBuilder { let (response_mime_type, response_schema) = if has_output_schema { let mut schema = args.output_schema.unwrap().clone(); schema.sanitize_for_google(); - (Some("application/json".to_string()), serde_json::to_value(&schema).ok()) + ( + Some("application/json".to_string()), + serde_json::to_value(&schema).ok(), + ) } else { (None, None) }; @@ -253,7 +259,11 @@ impl QueryBuilder for GoogleAIQueryBuilder { }); Ok(ParsedResponse::Text { - content: if accumulated_content.is_empty() { None } else { Some(accumulated_content) }, + content: if accumulated_content.is_empty() { + None + } else { + Some(accumulated_content) + }, tool_calls: accumulated_tool_calls.into_values().collect(), events_str: Some(events_str), annotations, @@ -271,8 +281,11 @@ impl QueryBuilder for GoogleAIQueryBuilder { format!("{}/{}:streamGenerateContent?alt=sse", base_url, model) } OutputType::Image => { - let url_suffix = - if model.contains("imagen") { "predict" } else { "generateContent" }; + let url_suffix = if model.contains("imagen") { + "predict" + } else { + "generateContent" + }; format!("{}/{}:{}", base_url, model, url_suffix) } } @@ -280,11 +293,17 @@ impl QueryBuilder for GoogleAIQueryBuilder { // Standard Google AI: base_url is generativelanguage.googleapis.com/v1beta match output_type { OutputType::Text => { - format!("{}/models/{}:streamGenerateContent?alt=sse", base_url, model) + format!( + "{}/models/{}:streamGenerateContent?alt=sse", + base_url, model + ) } OutputType::Image => { - let url_suffix = - if model.contains("imagen") { "predict" } else { "generateContent" }; + let url_suffix = if model.contains("imagen") { + "predict" + } else { + "generateContent" + }; format!("{}/models/{}:{}", base_url, model, url_suffix) } } diff --git a/backend/windmill-worker/src/ai/providers/openai.rs b/backend/windmill-worker/src/ai/providers/openai.rs index fbcf805e5f..52feb2eca7 100644 --- a/backend/windmill-worker/src/ai/providers/openai.rs +++ b/backend/windmill-worker/src/ai/providers/openai.rs @@ -242,12 +242,10 @@ fn convert_content_to_responses_format( image_url: image_url.url.clone(), }) } - ContentPart::File { file } => { - Some(ImageGenerationContent::InputFile { - filename: file.filename.clone(), - file_data: file.file_data.clone(), - }) - } + ContentPart::File { file } => Some(ImageGenerationContent::InputFile { + filename: file.filename.clone(), + file_data: file.file_data.clone(), + }), // S3 objects should have been resolved earlier, but handle gracefully ContentPart::S3Object { .. } => None, }) @@ -433,8 +431,7 @@ impl OpenAIQueryBuilder { if let Some(attachments) = args.attachments { for attachment in attachments.iter() { if !attachment.s3.is_empty() { - let part = - s3_object_to_content_part(attachment, client, workspace_id).await?; + let part = s3_object_to_content_part(attachment, client, workspace_id).await?; match part { ContentPart::File { file } => { content.push(ImageGenerationContent::InputFile { diff --git a/backend/windmill-worker/src/ai/query_builder.rs b/backend/windmill-worker/src/ai/query_builder.rs index 92f207aeb7..d74c45c098 100644 --- a/backend/windmill-worker/src/ai/query_builder.rs +++ b/backend/windmill-worker/src/ai/query_builder.rs @@ -24,9 +24,9 @@ pub fn create_query_builder(provider: &ProviderWithResource) -> Box Box::new(GoogleAIQueryBuilder::new( - provider.get_platform().clone(), - )), + AIProvider::GoogleAI => { + Box::new(GoogleAIQueryBuilder::new(provider.get_platform().clone())) + } AIProvider::OpenAI => Box::new(OpenAIQueryBuilder::new(provider.kind.clone())), AIProvider::Anthropic => Box::new(AnthropicQueryBuilder::new( provider.kind.clone(), diff --git a/backend/windmill-worker/src/ai/sse.rs b/backend/windmill-worker/src/ai/sse.rs index 7b5c06913d..2cc03b148e 100644 --- a/backend/windmill-worker/src/ai/sse.rs +++ b/backend/windmill-worker/src/ai/sse.rs @@ -9,10 +9,7 @@ use windmill_ai::{ ai_google::{parse_gemini_sse_event, GeminiUsageMetadata}, ai_types::{ExtraContent, GoogleExtraContent, OpenAIFunction, OpenAIToolCall}, }; -use windmill_common::{ - error::Error, - utils::rd_string, -}; +use windmill_common::{error::Error, utils::rd_string}; use crate::ai::{ query_builder::StreamEventSink, @@ -502,7 +499,10 @@ impl SSEParser for GeminiSSEParser { if let Some(text) = parsed.text { self.accumulated_content.push_str(&text); self.stream_event_processor - .send(StreamingEvent::TokenDelta { content: text }, &mut self.events_str) + .send( + StreamingEvent::TokenDelta { content: text }, + &mut self.events_str, + ) .await?; } diff --git a/backend/windmill-worker/src/pg_executor.rs b/backend/windmill-worker/src/pg_executor.rs index 44d782d950..16a555cc7e 100644 --- a/backend/windmill-worker/src/pg_executor.rs +++ b/backend/windmill-worker/src/pg_executor.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::net::IpAddr; use std::sync::atomic::{AtomicU64, AtomicUsize, Ordering}; use std::sync::Arc; @@ -18,7 +18,7 @@ use tokio::sync::{Mutex, RwLock}; use tokio_postgres::Client; use tokio_postgres::{types::ToSql, Row}; use tokio_postgres::{ - types::{FromSql, Type}, + types::{FromSql, IsNull, Kind, Type}, Column, }; use uuid::Uuid; @@ -31,7 +31,7 @@ use windmill_common::workspaces::get_datatable_resource_from_db_unchecked; use windmill_common::{PgDatabase, PrepareQueryColumnInfo, PrepareQueryResult, DB}; use windmill_parser::{Arg, Typ}; use windmill_parser_sql::{ - parse_db_resource, parse_pg_statement_arg_indices, parse_pgsql_sig_with_typed_schema, + parse_db_resource, parse_pg_statement_arg_positions, parse_pgsql_sig_with_typed_schema, parse_s3_mode, parse_sql_blocks, }; use windmill_queue::{CanceledBy, MiniPulledJob}; @@ -94,6 +94,201 @@ async fn new_pg_connection( Ok((client, handle)) } +/// `ToSql` / `FromSql` wrapper for a value whose Postgres wire format is plain +/// UTF-8 text regardless of the column's *type kind*. Vanilla +/// `tokio_postgres`'s `ToSql for String` / `FromSql for String` only accepts a +/// fixed list of base text types (TEXT/VARCHAR/BPCHAR/NAME/UNKNOWN + citext) — +/// they reject user-defined `Kind::Enum` and `Kind::Domain` even though +/// enum/domain wire format is just the variant name / the underlying base +/// type's text. This wrapper plugs that gap on both directions: +/// +/// - **bind side** (prepare-fallback path): `INSERT INTO t VALUES +/// ($1::my_enum)` works end-to-end without users needing the +/// `CAST($1::text AS my_enum)` workaround. +/// - **read side** (`pg_cell_to_json_value`'s fallback): `SELECT +/// $1::my_enum`, `SELECT enum_col FROM t`, etc. round-trip into a JSON +/// string instead of erroring with "cannot convert Option and the +/// Postgres type `my_enum`". +#[derive(Debug)] +struct AnyTextValue(String); + +fn any_text_accepts(ty: &Type) -> bool { + // Base text-like types, plus the citext extension type matched by name + // (it's not in `tokio_postgres::types::Type`'s constants), plus + // enum/domain kinds. We accept `Kind::Domain` unconditionally — the + // server is responsible for parsing the bytes and any domain whose + // base type accepts text on the wire (which is most of them) round-trips + // naturally. + matches!( + *ty, + Type::TEXT | Type::VARCHAR | Type::BPCHAR | Type::NAME | Type::UNKNOWN + ) || ty.name() == "citext" + || matches!(ty.kind(), Kind::Enum(_) | Kind::Domain(_)) +} + +impl ToSql for AnyTextValue { + fn to_sql( + &self, + _ty: &Type, + out: &mut bytes::BytesMut, + ) -> Result> { + use bytes::BufMut; + out.put_slice(self.0.as_bytes()); + Ok(IsNull::No) + } + + fn accepts(ty: &Type) -> bool { + any_text_accepts(ty) + } + + tokio_postgres::types::to_sql_checked!(); +} + +impl<'a> FromSql<'a> for AnyTextValue { + fn from_sql( + _ty: &Type, + raw: &'a [u8], + ) -> Result> { + // Postgres' text wire format for enums / domains-over-text / the + // base text types is the same: UTF-8 bytes of the value. + Ok(AnyTextValue(std::str::from_utf8(raw)?.to_owned())) + } + + fn accepts(ty: &Type) -> bool { + any_text_accepts(ty) + } +} + +impl ResultFormatState { + /// Decide whether to actually run the precision-loss check for this cell. + /// Returns `true` for the first `NUMERIC_PRECISION_CHECK_BUDGET` calls, + /// then `false` thereafter — and always `false` once the warning has + /// already been triggered. Cheap on the hot path: an atomic load + an + /// atomic decrement (Relaxed ordering), no allocation. + fn should_check_precision(&self) -> bool { + use std::sync::atomic::Ordering; + if self.numeric_precision_loss.load(Ordering::Relaxed) { + return false; + } + // `fetch_sub` returns the value BEFORE the decrement. When that's + // > 0 we had budget left for this cell. After the budget reaches 0 + // the next call would wrap to `u32::MAX-1`; pin it back to 0. + let prev = self + .numeric_precision_check_budget + .fetch_sub(1, Ordering::Relaxed); + if prev == 0 { + self.numeric_precision_check_budget + .store(0, Ordering::Relaxed); + false + } else { + true + } + } +} + +/// Emit a single job-log warning if `state.numeric_precision_loss` flipped +/// during the row iteration. The detection itself is bounded by +/// `NUMERIC_PRECISION_CHECK_BUDGET` cells, so this only adds a constant-cost +/// log call at end-of-query. +async fn warn_on_numeric_precision_loss( + state: &ResultFormatState, + job_id: Uuid, + workspace_id: &str, + log_conn: &Connection, +) { + use std::sync::atomic::Ordering; + if state.numeric_precision_loss.load(Ordering::Relaxed) { + windmill_queue::append_logs( + &job_id, + workspace_id, + "warning: at least one `numeric` value in the result lost precision \ + when serialised as a JSON number (the JSON Number format goes through \ + f64, which has ~15-17 significant digits). To preserve full precision, \ + cast the column to text in your SQL — e.g. `SELECT col::text` — and \ + parse the string client-side with a Decimal library.\n", + log_conn, + ) + .await; + } +} + +/// Emit a one-shot warning naming each declared arg the user didn't supply a +/// value for. PG executor binds these as NULL for back-compat — without a +/// warning, a misspelled arg key in the args object silently produces a row +/// of NULLs, which is a notoriously hard DX bug to track down. +async fn warn_on_missing_args( + missing: &[String], + job_id: Uuid, + workspace_id: &str, + log_conn: &Connection, +) { + if missing.is_empty() { + return; + } + let names = missing + .iter() + .map(|n| format!("`{n}`")) + .collect::>() + .join(", "); + windmill_queue::append_logs( + &job_id, + workspace_id, + format!( + "warning: argument(s) {names} declared in the query but not provided in the \ + args object — bound as NULL. Add the value(s) to the job args, declare a \ + default in the SQL (`-- $1 name (type) = default`), or remove the \ + declaration if the arg isn't used.\n" + ), + log_conn, + ) + .await; +} + +/// Short stable label for the JSON value's variant — used in error messages +/// so users can see *what kind of value* hit a binding error. +fn json_value_kind(v: &Value) -> &'static str { + match v { + Value::Null => "null", + Value::Bool(_) => "bool", + Value::Number(_) => "number", + Value::String(_) => "string", + Value::Array(_) => "array", + Value::Object(_) => "object", + } +} + +/// rust-postgres reports parameter encoding failures as +/// `error serializing parameter N: ` with N being a 0-based index. +/// Pull N out so we can attach our own metadata. +fn parse_param_index_from_err_msg(msg: &str) -> Option { + msg.strip_prefix("error serializing parameter ") + .and_then(|rest| rest.split(':').next()) + .and_then(|n| n.parse::().ok()) +} + +/// Replace a rust-postgres encoder error with one that names the offending +/// arg, its JSON value kind, and the Postgres type we asserted, plus a hint +/// about how to fix it. Other errors are passed through unchanged. +fn wrap_param_encoding_error( + err: tokio_postgres::Error, + param_meta: &[(String, &'static str)], + param_types: &[Type], +) -> Error { + let msg = err.to_string(); + if let Some(idx) = parse_param_index_from_err_msg(&msg) { + if let (Some((name, kind)), Some(t)) = (param_meta.get(idx), param_types.get(idx)) { + return Error::ExecutionErr(format!( + "Cannot bind arg `{name}` (JSON {kind}) as Postgres type `{t}` ({err}). \ + Try adding an explicit cast in the SQL — e.g. `${pos}::` \ + or `CAST(${pos} AS )` — or declare the type via \ + `-- ${pos} {name} ()`.", + pos = idx + 1, + )); + } + } + to_anyhow(err).into() +} + fn otyp_to_pg_type(otyp: &str) -> error::Result { let base = otyp.trim_end_matches("[]"); let is_array = otyp.ends_with("[]"); @@ -145,38 +340,96 @@ fn do_postgresql_inner<'a>( ) -> error::Result>>>> { let mut query_params = vec![]; let mut param_types: Vec = vec![]; - // Try to resolve a Postgres Type for every arg (using parser-supplied otyp, - // which defaults to "text" for inferred args). If we succeed for all, we can - // send the query as an unnamed prepared statement and avoid named statements - // entirely — see the dispatch comment below. + // Per-param metadata used to wrap rust-postgres `error serializing + // parameter N` errors with actionable context (arg name, JSON value kind, + // asserted Postgres type) — see error wrapping at the dispatch site. + let mut param_meta: Vec<(String, &'static str)> = vec![]; + // Track whether every arg has a resolvable Postgres type. We need *both* + // the parser-supplied otyp to be in `otyp_to_pg_type`'s map (so the arg + // isn't a custom enum / extension type) *and* convert_val to produce a + // (binding, type) pair that the encoder can actually serialize. If both + // hold for every arg, we send the query as an unnamed prepared statement + // (query_typed_raw) — see the dispatch comment below. Otherwise we fall + // back to prepare + query_raw and let the server resolve the parameter + // types from the SQL context. let mut all_types_resolved = true; - let arg_indices = parse_pg_statement_arg_indices(&query); + // Single tokenizer pass — derive both the index set (for the param + // dispatch loop below) and the byte ranges (for sparse renumbering) from + // one walk over the SQL. Positions skip occurrences inside string + // literals, comments, and dollar-quoted blocks, so the rewrite below + // doesn't mangle a query like `SELECT 'price: $5' AS lbl, $5 FROM t`. + let positions = parse_pg_statement_arg_positions(&query); + let arg_indices: HashSet = positions.iter().map(|(i, _)| *i).collect(); + + // Renumber sparse positional placeholders (e.g. $5, $50 → $1, $2) by + // byte position, walking back-to-front so earlier positions don't shift. + let renumber_mapping: HashMap = arg_indices + .iter() + .sorted() + .enumerate() + .map(|(i, oidx)| (*oidx, i + 1)) + .collect(); + if renumber_mapping + .iter() + .any(|(oidx, new_i)| *oidx as usize != *new_i) + { + let mut positions = positions.clone(); + positions.sort_by_key(|(_, range)| std::cmp::Reverse(range.start)); + for (oidx, range) in positions { + if let Some(new_i) = renumber_mapping.get(&oidx) { + if oidx as usize != *new_i { + query.replace_range(range, &new_i.to_string()); + } + } + } + } + + // Args the user didn't supply a value for — if their declaration doesn't + // carry a default, we still bind NULL (back-compat with how the PG + // executor has worked for years), but we collect them here to emit a + // single one-shot warning to the job logs after query execution so a typo + // / missing key doesn't silently turn into a row of NULLs. + let mut missing_args: Vec = Vec::new(); + // Stash declaration-default values so we can borrow them by reference + // alongside user-supplied values — both paths feed `convert_val(&Value)`. + let mut default_values: HashMap = HashMap::new(); - let mut i = 1; for oidx in arg_indices.iter().sorted() { if let Some((arg, value)) = param_idx_to_arg_and_value.get(&oidx) { - if *oidx as usize != i { - query = query.replace(&format!("${}", oidx), &format!("${}", i)); - } - let value = value.unwrap_or_else(|| &serde_json::Value::Null); + // Resolve the value: explicit user value > declaration default > NULL. + let value: &serde_json::Value = match (value, arg.default.as_ref()) { + (Some(v), _) => *v, + (None, Some(d)) => default_values.entry(*oidx).or_insert_with(|| d.clone()), + (None, None) => { + if !arg.has_default && !missing_args.contains(&arg.name) { + missing_args.push(arg.name.clone()); + } + &serde_json::Value::Null + } + }; let arg_t = arg .otyp .as_ref() .ok_or_else(|| anyhow::anyhow!("Missing otyp for pg arg"))?; let typ = &arg.typ; - let param = convert_val(value, arg_t, typ)?; + let (param, natural_type) = convert_val(value, arg_t, typ, arg.otyp_inferred)?; query_params.push(param); + param_meta.push((arg.name.clone(), json_value_kind(value))); if all_types_resolved { - match otyp_to_pg_type(arg_t) { - Ok(t) => param_types.push(t), - Err(_) => { - all_types_resolved = false; - param_types.clear(); - } + if otyp_to_pg_type(arg_t).is_ok() { + // The Type comes from convert_val (paired with the binding's + // concrete Rust type) rather than from `otyp_to_pg_type(arg_t)` + // — this prevents the parser-default "text" otyp from + // forcing an assertion that the encoder can't satisfy + // (e.g. Value::Bool with parser-defaulted text → Type::TEXT + // on a Box). + param_types.push(natural_type); + } else { + all_types_resolved = false; + param_types.clear(); } } - i += 1; } } @@ -196,10 +449,12 @@ fn do_postgresql_inner<'a>( .iter() .zip(param_types.iter()) .map(|(p, t)| (&**p as &(dyn ToSql + Sync), t.clone())); - client - .query_typed_raw(&query, typed_params) - .await - .map_err(to_anyhow)? + match client.query_typed_raw(&query, typed_params).await { + Ok(rows) => rows, + Err(e) => { + return Err(wrap_param_encoding_error(e, ¶m_meta, ¶m_types)); + } + } } else { let query_params = query_params .iter() @@ -212,12 +467,23 @@ fn do_postgresql_inner<'a>( .map_err(to_anyhow)? }; + // One state object per query — `pg_cell_to_json_value_with_state` + // flips `numeric_precision_loss` once if any `numeric` cell can't + // round-trip through f64. We emit a single warning to the job log + // after the iteration finishes, instead of error-by-error or per + // row, and the per-row check short-circuits on the flag so the cost + // is one branch after the first lossy value. + let format_state = ResultFormatState::default(); + if skip_collect { futures::pin_mut!(rows); while rows.try_next().await.map_err(to_anyhow)?.is_some() {} } else if let Some(ref s3) = s3 { - let rows_stream = rows.map_err(to_anyhow).map(|row_result| { - row_result.and_then(|row| postgres_row_to_json_value(row).map_err(to_anyhow)) + let format_state_ref = &format_state; + let rows_stream = rows.map_err(to_anyhow).map(move |row_result| { + row_result.and_then(|row| { + postgres_row_to_json_value_with_state(row, format_state_ref).map_err(to_anyhow) + }) }); s3_stream_and_upload_with_logs( @@ -230,6 +496,9 @@ fn do_postgresql_inner<'a>( ) .await?; + warn_on_numeric_precision_loss(&format_state, job_id, workspace_id, log_conn).await; + warn_on_missing_args(&missing_args, job_id, workspace_id, log_conn).await; + return Ok(vec![to_raw_value(&s3.to_return_s3_obj())]); } else { let rows = if first_row_only { @@ -254,7 +523,7 @@ fn do_postgresql_inner<'a>( } for row in rows.into_iter() { - let r = postgres_row_to_json_value(row); + let r = postgres_row_to_json_value_with_state(row, &format_state); if let Ok(v) = r.as_ref() { let size = sizeof_val(v); siz.fetch_add(size, Ordering::Relaxed); @@ -277,6 +546,9 @@ fn do_postgresql_inner<'a>( } } + warn_on_numeric_precision_loss(&format_state, job_id, workspace_id, log_conn).await; + warn_on_missing_args(&missing_args, job_id, workspace_id, log_conn).await; + Ok(res) }; @@ -366,11 +638,58 @@ pub async fn do_postgresql( .as_ref() .is_some_and(|x| x.as_ref().is_some_and(|y| y.0 == database_string)) { - // Probe the cached connection with DISCARD ALL before using it. - // This resets the full session (role, GUCs, temp tables, prepared - // statements, advisory locks) and also detects broken connections. + // Probe the cached connection with a curated session reset before + // reusing it. Each statement targets a specific class of state: + // + // RESET ALL — GUC parameters (search_path, + // application_name, statement_ + // timeout, transaction_*…). Note + // that this does NOT reset SET + // ROLE or SET SESSION + // AUTHORIZATION (security!). + // RESET SESSION AUTHORIZATION — undoes both `SET SESSION + // AUTHORIZATION` and `SET ROLE`, + // restoring the connecting user. + // Without this a previous job + // leaving an elevated role + // active would silently leak + // permissions into the next. + // UNLISTEN * — drops LISTEN registrations. + // CLOSE ALL — closes open cursors. + // pg_advisory_unlock_all() — releases any session-scoped + // advisory locks. Without this + // a job that called + // pg_advisory_lock and exited + // without unlocking would block + // later jobs holding the same + // key (DISCARD ALL covered this + // too). + // + // We deliberately do NOT use `DISCARD ALL`. DISCARD includes + // `DEALLOCATE ALL`, which deallocates *all* prepared statements + // server-side — including the typeinfo statements that + // tokio_postgres caches per-Client to resolve custom enum/domain + // Oids. After DISCARD, tokio_postgres still holds Statement + // objects whose names the server has forgotten, so the next + // custom-type query fails with `prepared statement "sN" does not + // exist`. The trade-off: temp tables and user-PREPARE statements + // may persist across cached-connection reuse (rare in datatable / + // script workloads). + // + // Doubles as a liveness probe — if the connection is broken any + // statement in the chain fails and we replace it. let probe_client = &guard.as_ref().unwrap().as_ref().unwrap().1; - if probe_client.batch_execute("DISCARD ALL").await.is_ok() { + if probe_client + .batch_execute( + "RESET ALL; \ + RESET SESSION AUTHORIZATION; \ + UNLISTEN *; \ + CLOSE ALL; \ + SELECT pg_advisory_unlock_all();", + ) + .await + .is_ok() + { tracing::info!("Using cached connection"); CACHE_HITS.fetch_add(1, std::sync::atomic::Ordering::Relaxed); LAST_QUERY.store( @@ -728,74 +1047,175 @@ fn map_as_single_type( } } +/// A boxed `ToSql` value paired with the Postgres `Type` that matches its +/// concrete Rust type. Returned by `convert_val` / `convert_vec_val` so the +/// dispatch in `do_postgresql_inner` always asserts the type that the encoder +/// can actually produce — never a parser-derived guess that drifts from the +/// runtime binding. +type ConvertedParam = (Box, Type); + fn convert_vec_val( vec: Option<&Vec>, arg_t: &String, -) -> windmill_common::error::Result> { +) -> windmill_common::error::Result { match arg_t.as_str() { - "bool" | "boolean" => Ok(Box::new(map_as_single_type(vec, |v| v.as_bool())?)), - "char" | "character" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_i64().map(|x| x as i8) - })?)), - "smallint" | "smallserial" | "int2" | "serial2" => { - Ok(Box::new(map_as_single_type(vec, |v| { - v.as_i64().map(|x| x as i16) - })?)) - } - "int" | "integer" | "int4" | "serial" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_i64().map(|x| x as i32) - })?)), - "numeric" | "decimal" => Ok(Box::new(map_as_single_type(vec, |v| { - if v.is_i64() { - Decimal::from_i64(v.as_i64().unwrap()) - } else if v.is_f64() { - Decimal::from_f64(v.as_f64().unwrap()) - } else { - None - } - })?)), - "oid" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_u64().map(|x| x as u32) - })?)), - "bigint" | "bigserial" | "int8" | "serial8" => { - Ok(Box::new(map_as_single_type(vec, |v| { - v.as_u64().map(|x| x as i64) - })?)) - } - "real" | "float4" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_f64().map(|x| x as f32) - })?)), - "double" | "double precision" | "float8" => { - Ok(Box::new(map_as_single_type(vec, |v| v.as_f64())?)) - } - "uuid" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_str().map(|x| Uuid::parse_str(x).ok()).flatten() - })?)), - "date" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_str().and_then(|x| parse_naive_date(x).ok()) - })?)), - "time" | "timetz" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_str().and_then(|x| parse_naive_time(x).ok()) - })?)), - "timestamp" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_str().and_then(|x| parse_naive_datetime(x).ok()) - })?)), - "timestamptz" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_str().and_then(|x| parse_datetime_utc(x).ok()) - })?)), - "jsonb" | "json" => Ok(Box::new( - vec.map(|v| v.clone().into_iter().map(Some).collect_vec()), + // Each integer / bool array arm accepts both JSON-native values AND + // stringified counterparts ("1", "true", …) — same coercion the + // scalar `Value::String → ` arms in `convert_val` apply, so an + // array passed via `JSON.stringify(BigInt(...))` or hand-quoted + // values doesn't trip a confusing "Mixed types in array" error. + "bool" | "boolean" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_bool() + .or_else(|| match v.as_str()?.to_ascii_lowercase().as_str() { + "true" | "t" | "yes" | "y" | "1" | "on" => Some(true), + "false" | "f" | "no" | "n" | "0" | "off" => Some(false), + _ => None, + }) + })?), + Type::BOOL_ARRAY, + )), + "char" | "character" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_i64() + .map(|x| x as i8) + .or_else(|| v.as_str().and_then(|s| s.parse::().ok())) + })?), + Type::CHAR_ARRAY, + )), + "smallint" | "smallserial" | "int2" | "serial2" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_i64() + .map(|x| x as i16) + .or_else(|| v.as_str().and_then(|s| s.parse::().ok())) + })?), + Type::INT2_ARRAY, + )), + "int" | "integer" | "int4" | "serial" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_i64() + .map(|x| x as i32) + .or_else(|| v.as_str().and_then(|s| s.parse::().ok())) + })?), + Type::INT4_ARRAY, + )), + // Mirror the scalar `Value::String → numeric` parsing arm so an array + // like `["1.5", "2.5"]` works against `$1::numeric[]` — useful for + // bulk-loading via `unnest`. Without this the user would see an + // unhelpful "Mixed types in array" error. + "numeric" | "decimal" => Ok(( + Box::new(map_as_single_type(vec, |v| { + if v.is_i64() { + Decimal::from_i64(v.as_i64().unwrap()) + } else if v.is_f64() { + Decimal::from_f64(v.as_f64().unwrap()) + } else { + v.as_str().and_then(|s| s.parse::().ok()) + } + })?), + Type::NUMERIC_ARRAY, + )), + "oid" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_u64() + .map(|x| x as u32) + .or_else(|| v.as_str().and_then(|s| s.parse::().ok())) + })?), + Type::OID_ARRAY, + )), + "bigint" | "bigserial" | "int8" | "serial8" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_i64() + .or_else(|| v.as_u64().map(|x| x as i64)) + .or_else(|| v.as_str().and_then(|s| s.parse::().ok())) + })?), + Type::INT8_ARRAY, + )), + "real" | "float4" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_f64() + .map(|x| x as f32) + .or_else(|| v.as_str().and_then(|s| s.parse::().ok())) + })?), + Type::FLOAT4_ARRAY, + )), + "double" | "double precision" | "float8" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_f64() + .or_else(|| v.as_str().and_then(|s| s.parse::().ok())) + })?), + Type::FLOAT8_ARRAY, + )), + "uuid" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_str().map(|x| Uuid::parse_str(x).ok()).flatten() + })?), + Type::UUID_ARRAY, + )), + "date" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_str().and_then(|x| parse_naive_date(x).ok()) + })?), + Type::DATE_ARRAY, + )), + "time" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_str().and_then(|x| parse_naive_time(x).ok()) + })?), + Type::TIME_ARRAY, + )), + "timetz" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_str().and_then(|x| parse_naive_time(x).ok()) + })?), + // chrono's `NaiveTime` only encodes for `TIME` — same caveat as + // the scalar `timetz` arm. Asserting `TIMETZ_ARRAY` here would + // fail at the encoder. Postgres has an implicit `time → timetz` + // assignment cast at the column site. + Type::TIME_ARRAY, + )), + "timestamp" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_str().and_then(|x| parse_naive_datetime(x).ok()) + })?), + Type::TIMESTAMP_ARRAY, + )), + "timestamptz" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_str().and_then(|x| parse_datetime_utc(x).ok()) + })?), + Type::TIMESTAMPTZ_ARRAY, + )), + "jsonb" => Ok(( + Box::new(vec.map(|v| v.clone().into_iter().map(Some).collect_vec())), + Type::JSONB_ARRAY, + )), + "json" => Ok(( + Box::new(vec.map(|v| v.clone().into_iter().map(Some).collect_vec())), + Type::JSON_ARRAY, + )), + "bytea" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_str().map(|x| { + engine::general_purpose::STANDARD + .decode(x) + .unwrap_or(vec![]) + }) + })?), + Type::BYTEA_ARRAY, + )), + "varchar" | "character varying" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_str().map(|x| x.to_string()) + })?), + Type::VARCHAR_ARRAY, + )), + "text" => Ok(( + Box::new(map_as_single_type(vec, |v| { + v.as_str().map(|x| x.to_string()) + })?), + Type::TEXT_ARRAY, )), - "bytea" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_str().map(|x| { - engine::general_purpose::STANDARD - .decode(x) - .unwrap_or(vec![]) - }) - })?)), - "text" | "varchar" => Ok(Box::new(map_as_single_type(vec, |v| { - v.as_str().map(|x| x.to_string()) - })?)), _ => Err(anyhow::anyhow!("Unsupported JSON array type"))?, } } @@ -804,7 +1224,20 @@ fn convert_val( value: &Value, arg_t: &String, typ: &Typ, -) -> windmill_common::error::Result> { + otyp_inferred: bool, +) -> windmill_common::error::Result { + // Helper: was the user's intent explicitly "text" / "varchar" / "char"? + // True when the parser saw an inline `$N::text` cast or a `-- $N (text)` + // declaration. False when the parser fell back to "text" because nothing + // else was found (in which case the caller has no real target type + // committed and we should bind the value's natural type). + let explicit_text_target = !otyp_inferred + && (matches!(typ, Typ::Str(_)) + && (arg_t == "text" + || arg_t == "varchar" + || arg_t == "character varying" + || arg_t == "char" + || arg_t == "character")); match value { Value::Array(vec) if arg_t.ends_with("[]") => { let arg_t = arg_t.trim_end_matches("[]").to_string(); @@ -815,29 +1248,90 @@ fn convert_val( convert_vec_val(None, &arg_t) } Value::Null => match arg_t.as_str() { - "bool" | "boolean" => Ok(Box::new(None::)), - "char" | "character" => Ok(Box::new(None::)), - "smallint" | "smallserial" | "int2" | "serial2" => Ok(Box::new(None::)), - "int" | "integer" | "int4" | "serial" => Ok(Box::new(None::)), - "numeric" | "decimal" => Ok(Box::new(None::)), - "oid" => Ok(Box::new(None::)), - "bigint" | "bigserial" | "int8" | "serial8" => Ok(Box::new(None::)), - "real" | "float4" => Ok(Box::new(None::)), - "double" | "double precision" | "float8" => Ok(Box::new(None::)), - "uuid" => Ok(Box::new(None::)), - "date" => Ok(Box::new(None::)), - "time" | "timetz" => Ok(Box::new(None::)), - "timestamp" => Ok(Box::new(None::)), - "timestamptz" => Ok(Box::new(None::>)), - "jsonb" | "json" => Ok(Box::new(None::>)), - "bytea" => Ok(Box::new(None::>)), - "text" | "varchar" => Ok(Box::new(None::)), - _ => Err(anyhow::anyhow!("Unsupported JSON null type"))?, + "bool" | "boolean" => Ok((Box::new(None::), Type::BOOL)), + "char" | "character" => Ok((Box::new(None::), Type::CHAR)), + "smallint" | "smallserial" | "int2" | "serial2" => { + Ok((Box::new(None::), Type::INT2)) + } + "int" | "integer" | "int4" | "serial" => Ok((Box::new(None::), Type::INT4)), + "numeric" | "decimal" => Ok((Box::new(None::), Type::NUMERIC)), + "oid" => Ok((Box::new(None::), Type::OID)), + "bigint" | "bigserial" | "int8" | "serial8" => Ok((Box::new(None::), Type::INT8)), + "real" | "float4" => Ok((Box::new(None::), Type::FLOAT4)), + "double" | "double precision" | "float8" => Ok((Box::new(None::), Type::FLOAT8)), + "uuid" => Ok((Box::new(None::), Type::UUID)), + "date" => Ok((Box::new(None::), Type::DATE)), + "time" => Ok((Box::new(None::), Type::TIME)), + // chrono's NaiveTime has no timezone, so its ToSql impl only + // accepts TIME. We assert TIME and rely on Postgres' implicit + // assignment cast time → timetz at the use site. + "timetz" => Ok((Box::new(None::), Type::TIME)), + "timestamp" => Ok((Box::new(None::), Type::TIMESTAMP)), + "timestamptz" => Ok((Box::new(None::>), Type::TIMESTAMPTZ)), + "jsonb" => Ok((Box::new(None::), Type::JSONB)), + "json" => Ok((Box::new(None::), Type::JSON)), + "bytea" => Ok((Box::new(None::>), Type::BYTEA)), + "varchar" | "character varying" => Ok((Box::new(None::), Type::VARCHAR)), + "text" => Ok((Box::new(None::), Type::TEXT)), + // Unrecognised arg_t — bind as TEXT NULL. The dispatch will fall + // back to prepare + query_raw, where the server resolves the + // actual column type and `Option`'s ToSql will accept the + // resolved Type for any text-like base; for enum/domain kinds + // None is encoded as the literal NULL message body, so the + // accepts() check is the only place that matters and we just need + // a binding whose accepts() is permissive enough. + _ => Ok((Box::new(None::), Type::TEXT)), }, - Value::Bool(b) => Ok(Box::new(b.clone())), - Value::Number(n) if matches!(typ, Typ::Str(_)) => Ok(Box::new(n.to_string())), + // Bool / Number with an *explicitly* text-typed arg: coerce to + // String. Used when the user wrote `-- $N (text)` or `$N::text` — + // they committed to text and may rely on equality comparisons like + // `WHERE text_col = $1`, which need a `text = text` operator (PG has + // no implicit `bool/int → text` cast in expression context). + Value::Bool(b) if explicit_text_target => { + // `char` (Type::CHAR, OID 18) is single-byte and `to_string()` of + // a bool is multi-byte ("true"/"false") — we can't bind it as + // CHAR. Fail explicitly with an actionable hint rather than + // silently sending BOOL (which the server then can't compare + // against a CHAR column — `operator does not exist: bool = char`). + // `character` (= bpchar, fixed-length text) has the same issue. + // For text/varchar/character varying we coerce to a string. + match arg_t.as_str() { + "char" | "character" => Err(Error::ExecutionErr(format!( + "Cannot bind a JSON bool to a `{arg_t}` arg. \ + `char` and `character` are single-byte / fixed-width text — \ + pass the value as a string (e.g. \"t\" / \"f\") or change \ + the arg type to `bool`." + ))), + "varchar" | "character varying" => Ok((Box::new(b.to_string()), Type::VARCHAR)), + _ => Ok((Box::new(b.to_string()), Type::TEXT)), + } + } + // Bool: bind as BOOL when no explicit text target. Postgres has an + // implicit assignment cast bool→text, so INSERTs into text columns + // still work — this only differs from the explicit-text branch above + // for expression-context uses (WHERE clauses, etc.). + Value::Bool(_) if arg_t == "jsonb" => Ok((Box::new(value.clone()), Type::JSONB)), + Value::Bool(_) if arg_t == "json" => Ok((Box::new(value.clone()), Type::JSON)), + Value::Bool(b) => Ok((Box::new(b.clone()), Type::BOOL)), + // Number with an explicitly text-typed arg: coerce to String. Same + // reasoning as the Bool branch — preserves pre-#8988 behaviour for + // hand-written PG scripts that use `WHERE text_col = $1` with a + // numeric value and an explicit text declaration. + // Skip `char`/`character`: those go to the existing single-byte arm + // below or the generic INT8 fallthrough. + Value::Number(n) + if explicit_text_target + && (arg_t == "text" || arg_t == "varchar" || arg_t == "character varying") => + { + let t = if arg_t == "varchar" || arg_t == "character varying" { + Type::VARCHAR + } else { + Type::TEXT + }; + Ok((Box::new(n.to_string()), t)) + } Value::Number(n) if arg_t == "char" && n.is_i64() => { - Ok(Box::new(n.as_i64().unwrap() as i8)) + Ok((Box::new(n.as_i64().unwrap() as i8), Type::CHAR)) } Value::Number(n) if (arg_t == "smallint" @@ -846,31 +1340,33 @@ fn convert_val( || arg_t == "serial2") && n.is_i64() => { - Ok(Box::new(n.as_i64().unwrap() as i16)) + Ok((Box::new(n.as_i64().unwrap() as i16), Type::INT2)) } Value::Number(n) if (arg_t == "int" || arg_t == "integer" || arg_t == "int4" || arg_t == "serial") && n.is_i64() => { - Ok(Box::new(n.as_i64().unwrap() as i32)) + Ok((Box::new(n.as_i64().unwrap() as i32), Type::INT4)) } Value::Number(n) if (arg_t == "real" || arg_t == "float4") && n.as_f64().is_some() => { - Ok(Box::new(n.as_f64().unwrap() as f32)) + Ok((Box::new(n.as_f64().unwrap() as f32), Type::FLOAT4)) } Value::Number(n) if (arg_t == "double" || arg_t == "double precision" || arg_t == "float8") && n.as_f64().is_some() => { - Ok(Box::new(n.as_f64().unwrap())) + Ok((Box::new(n.as_f64().unwrap()), Type::FLOAT8)) } - Value::Number(n) if (arg_t == "numeric" || arg_t == "decimal") && n.is_i64() => Ok( + Value::Number(n) if (arg_t == "numeric" || arg_t == "decimal") && n.is_i64() => Ok(( Box::new(Decimal::from_i64(n.as_i64().unwrap()).unwrap_or_default()), - ), - Value::Number(n) if (arg_t == "numeric" || arg_t == "decimal") && n.is_f64() => Ok( + Type::NUMERIC, + )), + Value::Number(n) if (arg_t == "numeric" || arg_t == "decimal") && n.is_f64() => Ok(( Box::new(Decimal::from_f64(n.as_f64().unwrap()).unwrap_or_default()), - ), + Type::NUMERIC, + )), Value::Number(n) if arg_t == "oid" && n.is_u64() => { - Ok(Box::new(n.as_u64().unwrap() as u32)) + Ok((Box::new(n.as_u64().unwrap() as u32), Type::OID)) } Value::Number(n) if (arg_t == "bigint" @@ -879,11 +1375,11 @@ fn convert_val( || arg_t == "serial8") && n.is_u64() => { - Ok(Box::new(n.as_u64().unwrap() as i64)) + Ok((Box::new(n.as_u64().unwrap() as i64), Type::INT8)) } - Value::Number(n) if n.is_i64() => Ok(Box::new(n.as_i64().unwrap())), - Value::Number(n) => Ok(Box::new(n.as_f64().unwrap())), - Value::String(s) if arg_t == "uuid" => Ok(Box::new(Uuid::parse_str(s)?)), + Value::Number(n) if n.is_i64() => Ok((Box::new(n.as_i64().unwrap()), Type::INT8)), + Value::Number(n) => Ok((Box::new(n.as_f64().unwrap()), Type::FLOAT8)), + Value::String(s) if arg_t == "uuid" => Ok((Box::new(Uuid::parse_str(s)?), Type::UUID)), Value::String(s) if arg_t == "smallint" || arg_t == "smallserial" @@ -891,14 +1387,14 @@ fn convert_val( || arg_t == "serial2" => { s.parse::() - .map(|n| Box::new(n) as Box) + .map(|n| (Box::new(n) as Box, Type::INT2)) .map_err(|e| anyhow::anyhow!("Cannot parse '{s}' as smallint: {e}").into()) } Value::String(s) if arg_t == "int" || arg_t == "integer" || arg_t == "int4" || arg_t == "serial" => { s.parse::() - .map(|n| Box::new(n) as Box) + .map(|n| (Box::new(n) as Box, Type::INT4)) .map_err(|e| anyhow::anyhow!("Cannot parse '{s}' as integer: {e}").into()) } Value::String(s) @@ -908,49 +1404,184 @@ fn convert_val( || arg_t == "serial8" => { s.parse::() - .map(|n| Box::new(n) as Box) + .map(|n| (Box::new(n) as Box, Type::INT8)) .map_err(|e| anyhow::anyhow!("Cannot parse '{s}' as bigint: {e}").into()) } Value::String(s) if arg_t == "date" => { let date = parse_naive_date(s) .map_err(|e| Error::ExecutionErr(format!("Cannot parse '{s}' as date: {e}")))?; - Ok(Box::new(date)) + Ok((Box::new(date), Type::DATE)) } - Value::String(s) if arg_t == "time" || arg_t == "timetz" => { + Value::String(s) if arg_t == "time" => { let time = parse_naive_time(s) .map_err(|e| Error::ExecutionErr(format!("Cannot parse '{s}' as time: {e}")))?; - Ok(Box::new(time)) + Ok((Box::new(time), Type::TIME)) + } + Value::String(s) if arg_t == "timetz" => { + let time = parse_naive_time(s) + .map_err(|e| Error::ExecutionErr(format!("Cannot parse '{s}' as time: {e}")))?; + // See the timetz Null arm — assert TIME, server casts to TIMETZ. + Ok((Box::new(time), Type::TIME)) } Value::String(s) if arg_t == "timestamp" => { let datetime = parse_naive_datetime(s).map_err(|e| { Error::ExecutionErr(format!("Cannot parse '{s}' as timestamp: {e}")) })?; - Ok(Box::new(datetime)) + Ok((Box::new(datetime), Type::TIMESTAMP)) } Value::String(s) if arg_t == "timestamptz" => { let datetime = parse_datetime_utc(s).map_err(|e| { Error::ExecutionErr(format!("Cannot parse '{s}' as timestamptz: {e}")) })?; - Ok(Box::new(datetime)) + Ok((Box::new(datetime), Type::TIMESTAMPTZ)) } Value::String(s) if arg_t == "bytea" => { let bytes = engine::general_purpose::STANDARD .decode(s) .unwrap_or(vec![]); - Ok(Box::new(bytes)) + Ok((Box::new(bytes), Type::BYTEA)) } - Value::Array(_) if arg_t == "jsonb" || arg_t == "json" => Ok(Box::new(value.clone())), - Value::Object(_) if arg_t == "text" || arg_t == "varchar" => { - Ok(Box::new(serde_json::to_string(value).map_err(|err| { + // Parse Strings into the matching native Rust type for the remaining + // recognised arg_ts that didn't have a dedicated arm. Without these, + // a string value lands in the generic Value::String fallback below + // (Box + TEXT) and the server-side comparison + // ` = text` fails since PG has no implicit cast. + Value::String(s) if arg_t == "numeric" || arg_t == "decimal" => s + .parse::() + .map(|d| (Box::new(d) as Box, Type::NUMERIC)) + .map_err(|e| anyhow::anyhow!("Cannot parse '{s}' as numeric: {e}").into()), + Value::String(s) if arg_t == "real" || arg_t == "float4" => s + .parse::() + .map(|n| (Box::new(n) as Box, Type::FLOAT4)) + .map_err(|e| anyhow::anyhow!("Cannot parse '{s}' as real: {e}").into()), + Value::String(s) + if arg_t == "double" || arg_t == "double precision" || arg_t == "float8" => + { + s.parse::() + .map(|n| (Box::new(n) as Box, Type::FLOAT8)) + .map_err(|e| anyhow::anyhow!("Cannot parse '{s}' as double: {e}").into()) + } + Value::String(s) if arg_t == "oid" => s + .parse::() + .map(|n| (Box::new(n) as Box, Type::OID)) + .map_err(|e| anyhow::anyhow!("Cannot parse '{s}' as oid: {e}").into()), + Value::String(s) if arg_t == "bool" || arg_t == "boolean" => { + // Accept the same literals Postgres' boolin() does. + let b = match s.to_ascii_lowercase().as_str() { + "true" | "t" | "yes" | "y" | "1" | "on" => true, + "false" | "f" | "no" | "n" | "0" | "off" => false, + _ => { + return Err( + anyhow::anyhow!("Cannot parse '{s}' as bool: invalid literal").into(), + ) + } + }; + Ok((Box::new(b), Type::BOOL)) + } + Value::String(s) if arg_t == "varchar" || arg_t == "character varying" => { + Ok((Box::new(s.clone()), Type::VARCHAR)) + } + // For arg_t in (json, jsonb): bind a JSON-encodable Value with the + // matching pg type. Falling through to TEXT here would assert TEXT + // and break query_typed_raw's encoder check. + // Object / Array (no `[]` suffix): bind as JSONB by default and + // JSON-stringify when the target is text-like. + // + // Note the asymmetry vs the Bool/Number arms above: we coerce to + // text on `matches!(typ, Typ::Str(_))` (which is true for both + // explicit `(text)` decls AND parser-default text), not on + // `explicit_text_target`. Reason: serialising a JSON object/array + // as JSONB and binding against a parser-default-text arg would + // assert `JSONB` for what could be a plain-text column. Postgres + // has no implicit cast `jsonb → text` in expression context, so + // `WHERE text_col = $1::JSONB` would fail. JSON-stringifying into + // TEXT is what users almost always want for these JSON shapes + // (and the result is itself valid JSON, so a `::jsonb` cast in + // SQL still round-trips). Bool/Number don't need this safety + // because `bool → text` and `int → text` have implicit assignment + // casts; the asymmetry is therefore semantic, not a bug. + Value::Array(_) if arg_t == "jsonb" => Ok((Box::new(value.clone()), Type::JSONB)), + Value::Array(_) if arg_t == "json" => Ok((Box::new(value.clone()), Type::JSON)), + Value::Array(_) if matches!(typ, Typ::Str(_)) => { + let s = serde_json::to_string(value).map_err(|err| { Error::ExecutionErr(format!("Failed to convert JSON to text: {}", err)) - })?)) + })?; + let t = if arg_t == "varchar" { + Type::VARCHAR + } else { + Type::TEXT + }; + Ok((Box::new(s), t)) + } + // Default for arrays without a [] suffix: bind as JSONB. + Value::Array(_) => Ok((Box::new(value.clone()), Type::JSONB)), + Value::Object(_) if arg_t == "json" => Ok((Box::new(value.clone()), Type::JSON)), + Value::Object(_) if arg_t == "varchar" || arg_t == "character varying" => Ok(( + Box::new(serde_json::to_string(value).map_err(|err| { + Error::ExecutionErr(format!("Failed to convert JSON to text: {}", err)) + })?), + Type::VARCHAR, + )), + Value::Object(_) if arg_t == "text" || matches!(typ, Typ::Str(_)) => Ok(( + Box::new(serde_json::to_string(value).map_err(|err| { + Error::ExecutionErr(format!("Failed to convert JSON to text: {}", err)) + })?), + Type::TEXT, + )), + Value::Object(_) => Ok((Box::new(value.clone()), Type::JSONB)), + // Generic String fallback. Use `AnyTextValue` (rather than plain + // `String`) so the binding's `accepts()` covers `Kind::Enum` and + // `Kind::Domain` in addition to the base text types — this is what + // makes `INSERT INTO t VALUES ($1::my_enum)` work end-to-end without + // users needing the `CAST($1::text AS my_enum)` workaround. + // + // We always assert `Type::TEXT` (not `Type::UNKNOWN`): tokio_postgres + // sends parameter values in binary format, and Postgres rejects + // binary-formatted bytes for `UNKNOWN` parameters in operator + // contexts ("incorrect binary data format in bind parameter N"). The + // trade-off is that bare `$1` against a non-text column still needs + // an explicit cast (`$1::my_enum`), but the failure mode is a clear + // server error rather than a cryptic protocol mismatch. + Value::String(s) => Ok((Box::new(AnyTextValue(s.clone())), Type::TEXT)), + } +} + +/// Hard cap on how many `numeric` cells we test for f64-precision loss per +/// query. The check is `Decimal -> f64 -> Decimal` round-trip + `==` (~tens +/// of ns each); on a query returning millions of numeric cells, an +/// unbounded check would add measurable latency. After this many "fits +/// fine" observations we assume the rest do too — the pathological case +/// (rows 1..N fit, row N+1 loses precision) goes silently truncated, but +/// users who care about precision in such results can `::text`-cast their +/// SQL anyway. The first cell that does NOT fit short-circuits the budget +/// (the warning fires once and the per-row check stops immediately). +const NUMERIC_PRECISION_CHECK_BUDGET: u32 = 256; + +/// Per-query state carried through result formatting. Currently used to +/// detect precision loss on the first `numeric` cell that doesn't round-trip +/// through f64, so the caller can emit a single warning per job rather than +/// silently truncating every row. Uses atomics (rather than `Cell`) so the +/// s3-streaming path — which moves the closure across futures and requires +/// `Send` — can borrow it. +pub struct ResultFormatState { + /// `true` once we've observed a `numeric` value that loses precision when + /// converted via f64. Once flipped, the per-row check short-circuits. + pub numeric_precision_loss: std::sync::atomic::AtomicBool, + /// Decremented for each `numeric` cell we actually check. When it hits 0 + /// the per-row check is skipped (along with the precision-loss flag) for + /// the rest of the query — see the rationale on + /// `NUMERIC_PRECISION_CHECK_BUDGET`. + numeric_precision_check_budget: std::sync::atomic::AtomicU32, +} + +impl Default for ResultFormatState { + fn default() -> Self { + Self { + numeric_precision_loss: std::sync::atomic::AtomicBool::new(false), + numeric_precision_check_budget: std::sync::atomic::AtomicU32::new( + NUMERIC_PRECISION_CHECK_BUDGET, + ), } - Value::Object(_) => Ok(Box::new(value.clone())), - Value::String(s) => Ok(Box::new(s.clone())), - _ => Err(Error::ExecutionErr(format!( - "Unsupported type in query: {:?} and signature {arg_t:?}", - value - ))), } } @@ -959,9 +1590,34 @@ pub fn pg_cell_to_json_value( column: &Column, column_i: usize, ) -> Result { + pg_cell_to_json_value_with_state(row, column, column_i, &ResultFormatState::default()) +} + +pub fn pg_cell_to_json_value_with_state( + row: &Row, + column: &Column, + column_i: usize, + state: &ResultFormatState, +) -> Result { + // JSON has no encoding for NaN / +Inf / -Inf, but Postgres `float4` / + // `float8` (and `numeric`, via the special `'NaN'` value) do return them. + // Pre-fix the worker errored with "invalid json-float", failing the + // entire query. Round-trip these as JSON strings ("NaN", "Infinity", + // "-Infinity") so the rest of the row still comes through; users who + // need numeric semantics can filter them out client-side. let f64_to_json_number = |raw_val: f64| -> Result { - let temp = serde_json::Number::from_f64(raw_val.into()) - .ok_or(anyhow::anyhow!("invalid json-float"))?; + if raw_val.is_nan() { + return Ok(JSONValue::String("NaN".to_string())); + } + if raw_val.is_infinite() { + return Ok(JSONValue::String(if raw_val > 0.0 { + "Infinity".to_string() + } else { + "-Infinity".to_string() + })); + } + let temp = + serde_json::Number::from_f64(raw_val).ok_or(anyhow::anyhow!("invalid json-float"))?; Ok(JSONValue::Number(temp)) }; Ok(match *column.type_() { @@ -990,20 +1646,30 @@ pub fn pg_cell_to_json_value( Type::TEXT | Type::VARCHAR => { get_basic(row, column, column_i, |a: String| Ok(JSONValue::String(a)))? } + // ISO-8601 / RFC-3339 for temporal types so values round-trip through + // JS / Python clients (`new Date(s)`, `datetime.fromisoformat(s)`) + // without manual parsing. chrono's default `to_string()` returns + // space-separated for naive datetimes and " UTC" suffix for tz-aware, + // neither of which is parseable as ISO 8601. Type::TIMESTAMP => get_basic(row, column, column_i, |a: chrono::NaiveDateTime| { - Ok(JSONValue::String(a.to_string())) + Ok(JSONValue::String(format_naive_datetime_iso(&a))) })?, Type::DATE => get_basic(row, column, column_i, |a: chrono::NaiveDate| { + // chrono's `NaiveDate::to_string` is already ISO-8601 (`%Y-%m-%d`). Ok(JSONValue::String(a.to_string())) })?, Type::TIME => get_basic(row, column, column_i, |a: chrono::NaiveTime| { + // `NaiveTime::to_string` is already ISO-8601 (`%H:%M:%S` with + // optional `.f`). Ok(JSONValue::String(a.to_string())) })?, Type::TIMETZ => get_basic(row, column, column_i, |a: TimeTZStr| { + // TimeTZStr's `from_sql` already formats as ISO-8601 (see impl + // below). Ok(JSONValue::String(a.0)) })?, Type::TIMESTAMPTZ => get_basic(row, column, column_i, |a: chrono::DateTime| { - Ok(JSONValue::String(a.to_string())) + Ok(JSONValue::String(a.to_rfc3339())) })?, Type::UUID => get_basic(row, column, column_i, |a: uuid::Uuid| { Ok(JSONValue::String(a.to_string())) @@ -1018,7 +1684,25 @@ pub fn pg_cell_to_json_value( Type::FLOAT4 => get_basic(row, column, column_i, |a: f32| { Ok(f64_to_json_number(a.into())?) })?, + // Pre-existing behaviour: `numeric` is serialised as a JSON Number + // via `Decimal::serialize`, which goes through f64 and silently + // truncates past ~15-17 significant digits. Switching to JSON String + // would preserve precision but break any user script doing arithmetic + // / comparison on numeric column results (`row.amount + 1` becomes + // string concat, `row.amount > 100` is lexicographic). Left as Number + // for back-compat. Instead, on the FIRST cell whose decimal + // representation can't round-trip through f64, we flip + // `state.numeric_precision_loss` so the caller can emit a single + // job-log warning recommending a `::text` cast. The check is bounded + // by `NUMERIC_PRECISION_CHECK_BUDGET` cells (see comment there) and + // short-circuits on the first lossy value, so the hot path on a + // numeric-heavy result set is two atomic loads + an early return. Type::NUMERIC => get_basic(row, column, column_i, |a: Decimal| { + if state.should_check_precision() && !decimal_fits_f64_losslessly(&a) { + state + .numeric_precision_loss + .store(true, std::sync::atomic::Ordering::Relaxed); + } Ok(serde_json::to_value(a) .map_err(|_| anyhow::anyhow!("Cannot convert decimal to json"))?) })?, @@ -1064,7 +1748,14 @@ pub fn pg_cell_to_json_value( Type::FLOAT8_ARRAY => { get_array(row, column, column_i, |a: f64| Ok(f64_to_json_number(a)?))? } + // See scalar NUMERIC arm — kept as JSON Number for back-compat, + // with bounded precision-loss detection. Type::NUMERIC_ARRAY => get_array(row, column, column_i, |a: Decimal| { + if state.should_check_precision() && !decimal_fits_f64_losslessly(&a) { + state + .numeric_precision_loss + .store(true, std::sync::atomic::Ordering::Relaxed); + } Ok(serde_json::to_value(a) .map_err(|_| anyhow::anyhow!("Cannot convert decimal to json"))?) })?, @@ -1072,8 +1763,9 @@ pub fn pg_cell_to_json_value( Type::TS_VECTOR_ARRAY => get_array(row, column, column_i, |a: StringCollector| { Ok(JSONValue::String(a.0)) })?, + // Same ISO-8601 formatting as the scalar arms above. Type::TIMESTAMP_ARRAY => get_array(row, column, column_i, |a: chrono::NaiveDateTime| { - Ok(JSONValue::String(a.to_string())) + Ok(JSONValue::String(format_naive_datetime_iso(&a))) })?, Type::DATE_ARRAY => get_array(row, column, column_i, |a: chrono::NaiveDate| { Ok(JSONValue::String(a.to_string())) @@ -1085,18 +1777,32 @@ pub fn pg_cell_to_json_value( Ok(JSONValue::String(a.0)) })?, Type::TIMESTAMPTZ_ARRAY => get_array(row, column, column_i, |a: chrono::DateTime| { - Ok(JSONValue::String(a.to_string())) + Ok(JSONValue::String(a.to_rfc3339())) })?, Type::BYTEA_ARRAY => get_array(row, column, column_i, |a: Vec| { Ok(JSONValue::String(format!("\\x{}", hex::encode(a)))) })?, Type::VOID => JSONValue::Null, - _ => get_basic(row, column, column_i, |a: String| Ok(JSONValue::String(a)))?, + // Default fallback for unhandled column types: read as text. We use + // `AnyTextValue` instead of plain `String` so that `Kind::Enum`, + // `Kind::Domain`, and citext columns round-trip into JSON strings + // rather than erroring with `cannot convert between Option + // and the Postgres type \`\``. + _ => get_basic(row, column, column_i, |a: AnyTextValue| { + Ok(JSONValue::String(a.0)) + })?, }) } pub fn postgres_row_to_json_value(row: Row) -> Result { - let row_data = postgres_row_to_row_data(row)?; + postgres_row_to_json_value_with_state(row, &ResultFormatState::default()) +} + +pub fn postgres_row_to_json_value_with_state( + row: Row, + state: &ResultFormatState, +) -> Result { + let row_data = postgres_row_to_row_data_with_state(row, state)?; Ok(JSONValue::Object(row_data)) } @@ -1105,15 +1811,34 @@ pub type JSONValue = serde_json::Value; pub type RowData = Map; pub fn postgres_row_to_row_data(row: Row) -> Result { + postgres_row_to_row_data_with_state(row, &ResultFormatState::default()) +} + +pub fn postgres_row_to_row_data_with_state( + row: Row, + state: &ResultFormatState, +) -> Result { let mut result: Map = Map::new(); for (i, column) in row.columns().iter().enumerate() { let name = column.name(); - let json_value = pg_cell_to_json_value(&row, column, i)?; + let json_value = pg_cell_to_json_value_with_state(&row, column, i, state)?; result.insert(name.to_string(), json_value); } Ok(result) } +/// Returns true if the `Decimal` value can round-trip through `f64` without +/// losing precision. Used to detect when the user's `numeric` results are +/// being silently truncated by the JSON Number serialisation path so the +/// worker can log a one-shot warning recommending a `::text` cast in SQL. +fn decimal_fits_f64_losslessly(d: &Decimal) -> bool { + use rust_decimal::prelude::ToPrimitive; + match d.to_f64() { + Some(f) if f.is_finite() => Decimal::from_f64(f).is_some_and(|round| &round == d), + _ => false, + } +} + fn get_basic<'a, T: FromSql<'a>>( row: &'a Row, column: &Column, @@ -1165,7 +1890,8 @@ impl<'a> FromSql<'a> for TimeTZStr { ((microsecond % 1_000_000) * 1_000) as u32, ) .ok_or_else(|| anyhow::anyhow!("Invalid time value"))?; - Ok(TimeTZStr(format!("{:?} UTC", utc))) + // ISO-8601: append `+00:00` since TIMETZ is normalised to UTC here. + Ok(TimeTZStr(format!("{}+00:00", utc))) } fn accepts(ty: &Type) -> bool { @@ -1173,6 +1899,18 @@ impl<'a> FromSql<'a> for TimeTZStr { } } +/// Format a `NaiveDateTime` as ISO-8601 (`YYYY-MM-DDTHH:MM:SS[.fff…]`). +/// chrono's default `to_string` uses a space separator, which is not parseable +/// by `new Date(s)` in older JS engines or Python's `datetime.fromisoformat` +/// before 3.11. Use the explicit format string so output is portable. +fn format_naive_datetime_iso(dt: &chrono::NaiveDateTime) -> String { + if dt.and_utc().timestamp_subsec_nanos() == 0 { + dt.format("%Y-%m-%dT%H:%M:%S").to_string() + } else { + dt.format("%Y-%m-%dT%H:%M:%S%.f").to_string() + } +} + fn get_array<'a, T: FromSql<'a>>( row: &'a Row, column: &Column, @@ -1344,4 +2082,953 @@ mod tests { let parsed = parse_naive_time(&serialized).unwrap(); assert_eq!(original, parsed); } + + // --------------------------------------------------------------------- + // convert_val: exhaustive (Value × otyp) → Type matrix. + // + // For every (JSON Value, parser otyp) combination that can occur from + // either windmill-client SDK output (TS or Python) or a hand-written + // Postgres script, verify that convert_val returns a `(Box, Type)` pair + // where the Type matches the Box's concrete Rust type. This is the core + // invariant that makes `query_typed_raw` safe — if it ever drifts again + // (the bug introduced by #8988), users get + // `cannot convert between the Rust type X and the Postgres type Y`. + // + // We can't introspect the Box's Rust type at runtime, but we *can* feed + // each (Box, Type) through `to_sql_checked` against the asserted Type — + // that's exactly the codepath `query_typed_raw` uses, so any mismatch + // surfaces here as a `ToSql` error. + // --------------------------------------------------------------------- + + use bytes::BytesMut; + use serde_json::json; + use tokio_postgres::types::IsNull; + + /// Verify that convert_val for `(value, arg_t)` returns a binding whose + /// Rust type matches the asserted Postgres `Type` — exactly the check + /// `query_typed_raw` performs when serialising parameters. + /// + /// Defaults to `otyp_inferred = false` (= "user explicitly typed this"). + /// Tests that need the parser-default flavour use + /// `assert_convert_val_consistent_inferred`. + fn assert_convert_val_consistent( + label: &str, + value: Value, + arg_t: &str, + typ: Typ, + expected_type: Type, + ) { + assert_convert_val_consistent_full(label, value, arg_t, typ, expected_type, false) + } + + fn assert_convert_val_consistent_inferred( + label: &str, + value: Value, + arg_t: &str, + typ: Typ, + expected_type: Type, + ) { + assert_convert_val_consistent_full(label, value, arg_t, typ, expected_type, true) + } + + fn assert_convert_val_consistent_full( + label: &str, + value: Value, + arg_t: &str, + typ: Typ, + expected_type: Type, + otyp_inferred: bool, + ) { + let (boxed, t) = convert_val(&value, &arg_t.to_string(), &typ, otyp_inferred) + .unwrap_or_else(|e| panic!("{label}: convert_val errored: {e}")); + assert_eq!( + t, expected_type, + "{label}: expected Type {expected_type}, got {t}" + ); + // Run the encoder check — this is what query_typed_raw does internally + // when binding the param. A mismatch between the boxed Rust type and + // the asserted Type fails here as a `WrongType` error. + let mut buf = BytesMut::new(); + match boxed.to_sql_checked(&t, &mut buf) { + Ok(IsNull::Yes) | Ok(IsNull::No) => {} + Err(e) => panic!( + "{label}: ToSql failed for value={value:?} arg_t={arg_t} (asserted {t}): {e}" + ), + } + } + + fn typ_for(arg_t: &str) -> Typ { + windmill_parser_sql::parse_pg_typ(arg_t) + } + + #[test] + fn convert_val_null_for_every_known_arg_t() { + // `Value::Null` for every type the parser may resolve, plus an unknown + // arg_t (custom enum / extension). Each must produce a matching Type + // and serialise without error. + let cases: &[(&str, Type)] = &[ + ("bool", Type::BOOL), + ("boolean", Type::BOOL), + ("char", Type::CHAR), + ("character", Type::CHAR), + ("smallint", Type::INT2), + ("int2", Type::INT2), + ("smallserial", Type::INT2), + ("serial2", Type::INT2), + ("int", Type::INT4), + ("integer", Type::INT4), + ("int4", Type::INT4), + ("serial", Type::INT4), + ("bigint", Type::INT8), + ("int8", Type::INT8), + ("bigserial", Type::INT8), + ("serial8", Type::INT8), + ("real", Type::FLOAT4), + ("float4", Type::FLOAT4), + ("double", Type::FLOAT8), + ("double precision", Type::FLOAT8), + ("float8", Type::FLOAT8), + ("numeric", Type::NUMERIC), + ("decimal", Type::NUMERIC), + ("oid", Type::OID), + ("uuid", Type::UUID), + ("date", Type::DATE), + ("time", Type::TIME), + // chrono::NaiveTime can only encode as TIME — see the Null arm. + ("timetz", Type::TIME), + ("timestamp", Type::TIMESTAMP), + ("timestamptz", Type::TIMESTAMPTZ), + ("json", Type::JSON), + ("jsonb", Type::JSONB), + ("bytea", Type::BYTEA), + ("text", Type::TEXT), + ("varchar", Type::VARCHAR), + ("character varying", Type::VARCHAR), + // Unknown / custom type: convert_val falls back to TEXT NULL — the + // dispatch then takes the prepare + query_raw path so the server + // resolves the actual column type. + ("my_custom_enum", Type::TEXT), + ]; + for (arg_t, expected) in cases { + assert_convert_val_consistent( + &format!("Null/{arg_t}"), + Value::Null, + arg_t, + typ_for(arg_t), + expected.clone(), + ); + } + } + + #[test] + fn convert_val_bool_against_every_arg_t() { + // Value::Bool. Pre-#8988 this always produced Box, which + // mismatched the asserted Type for parser-default "text" (the + // regression we fix). Post-fix: + // - explicit text-like target (`-- $1 (text)` / `$1::text`): + // coerce to Box+TEXT so `WHERE text_col = $1` works. + // - parser-default text (bare `$N`, no annotation): bind as BOOL + // natively, server casts at the use site. + // - any other target: bind as BOOL. + let bool_targets = ["bool", "boolean"]; + let json_targets = [("json", Type::JSON), ("jsonb", Type::JSONB)]; + let explicit_text_targets = [ + ("text", Type::TEXT), + ("varchar", Type::VARCHAR), + ("character varying", Type::VARCHAR), + ]; + let bind_as_bool = [ + "smallint", + "int", + "integer", + "bigint", + "int4", + "int8", + "real", + "double", + "double precision", + "numeric", + "uuid", + "date", + "time", + "timestamp", + "timestamptz", + "bytea", + "oid", + // unknown — server resolves via prepare path + "my_custom_enum", + ]; + + for v in [true, false] { + for arg_t in &bool_targets { + assert_convert_val_consistent( + &format!("Bool({v})/{arg_t}"), + Value::Bool(v), + arg_t, + typ_for(arg_t), + Type::BOOL, + ); + } + for (arg_t, expected) in &json_targets { + assert_convert_val_consistent( + &format!("Bool({v})/{arg_t}"), + Value::Bool(v), + arg_t, + typ_for(arg_t), + expected.clone(), + ); + } + for (arg_t, expected) in &explicit_text_targets { + // Explicit (otyp_inferred=false): coerce to text. + assert_convert_val_consistent( + &format!("Bool({v})/{arg_t} explicit"), + Value::Bool(v), + arg_t, + typ_for(arg_t), + expected.clone(), + ); + // Inferred (parser-default): keep BOOL. + assert_convert_val_consistent_inferred( + &format!("Bool({v})/{arg_t} inferred"), + Value::Bool(v), + arg_t, + typ_for(arg_t), + Type::BOOL, + ); + } + // `char` and `character` (= bpchar) are single-byte / fixed-width + // text. Explicit decl with a JSON bool errors with an actionable + // hint instead of silently binding BOOL (which a CHAR column + // can't compare against). Inferred-default still binds BOOL. + for arg_t in &["char", "character"] { + let err = convert_val( + &Value::Bool(v), + &arg_t.to_string(), + &typ_for(arg_t), + /* otyp_inferred = */ false, + ) + .err() + .unwrap_or_else(|| panic!("Bool({v})/{arg_t} explicit should error")); + let msg = err.to_string(); + assert!( + msg.contains("Cannot bind a JSON bool"), + "Bool({v})/{arg_t} explicit error didn't have expected message: {msg}" + ); + assert_convert_val_consistent_inferred( + &format!("Bool({v})/{arg_t} inferred"), + Value::Bool(v), + arg_t, + typ_for(arg_t), + Type::BOOL, + ); + } + for arg_t in &bind_as_bool { + assert_convert_val_consistent( + &format!("Bool({v})/{arg_t}"), + Value::Bool(v), + arg_t, + typ_for(arg_t), + Type::BOOL, + ); + } + } + } + + #[test] + fn convert_val_integer_number_against_every_arg_t() { + // JSON integers. Each arg_t selects its matching encoder; for arg_ts + // that don't have a numeric encoder (uuid, date, …), the value falls + // through to the generic Number arm — Box bound as INT8 — and + // server-side casts handle the rest if the SQL wants it. + let cases: &[(&str, Type)] = &[ + ("char", Type::CHAR), + // "character" (= bpchar in PG) doesn't have a Number arm, so it + // falls through to generic Number → Box + INT8. Server + // casts at the SQL site if the column is bpchar. + ("character", Type::INT8), + ("smallint", Type::INT2), + ("smallserial", Type::INT2), + ("int2", Type::INT2), + ("serial2", Type::INT2), + ("int", Type::INT4), + ("integer", Type::INT4), + ("int4", Type::INT4), + ("serial", Type::INT4), + ("bigint", Type::INT8), + ("bigserial", Type::INT8), + ("int8", Type::INT8), + ("serial8", Type::INT8), + ("oid", Type::OID), + ("numeric", Type::NUMERIC), + ("decimal", Type::NUMERIC), + // Unknown arg_t falls through to generic Number → INT8. + ("my_custom_enum", Type::INT8), + ]; + for (arg_t, expected) in cases { + assert_convert_val_consistent( + &format!("Number(42)/{arg_t}"), + json!(42), + arg_t, + typ_for(arg_t), + expected.clone(), + ); + } + // Text targets: split between explicit (coerce to TEXT) and inferred + // (parser-default, bind as INT8 — server casts at the use site). + for (arg_t, expected_text) in [("text", Type::TEXT), ("varchar", Type::VARCHAR)] { + assert_convert_val_consistent( + &format!("Number(42)/{arg_t} explicit"), + json!(42), + arg_t, + typ_for(arg_t), + expected_text, + ); + assert_convert_val_consistent_inferred( + &format!("Number(42)/{arg_t} inferred"), + json!(42), + arg_t, + typ_for(arg_t), + Type::INT8, + ); + } + // Negative integer (is_u64 false → falls to generic i64 arm for bigint). + assert_convert_val_consistent( + "Number(-7)/bigint", + json!(-7), + "bigint", + typ_for("bigint"), + Type::INT8, + ); + assert_convert_val_consistent("Number(0)/oid", json!(0), "oid", typ_for("oid"), Type::OID); + } + + #[test] + fn convert_val_float_number_against_every_arg_t() { + let cases: &[(&str, Type)] = &[ + ("real", Type::FLOAT4), + ("float4", Type::FLOAT4), + ("double", Type::FLOAT8), + ("double precision", Type::FLOAT8), + ("float8", Type::FLOAT8), + ("numeric", Type::NUMERIC), + ("decimal", Type::NUMERIC), + // Unknown arg_t falls through to generic → Box+FLOAT8. + ("my_custom_enum", Type::FLOAT8), + ]; + for (arg_t, expected) in cases { + assert_convert_val_consistent( + &format!("Number(3.14)/{arg_t}"), + json!(3.14), + arg_t, + typ_for(arg_t), + expected.clone(), + ); + } + // Text targets: split between explicit (coerce to TEXT/VARCHAR) and + // inferred (parser-default, bind as FLOAT8 — server casts at use site). + for (arg_t, expected_text) in [("text", Type::TEXT), ("varchar", Type::VARCHAR)] { + assert_convert_val_consistent( + &format!("Number(3.14)/{arg_t} explicit"), + json!(3.14), + arg_t, + typ_for(arg_t), + expected_text, + ); + assert_convert_val_consistent_inferred( + &format!("Number(3.14)/{arg_t} inferred"), + json!(3.14), + arg_t, + typ_for(arg_t), + Type::FLOAT8, + ); + } + } + + #[test] + fn convert_val_string_against_every_arg_t() { + // Strings parse into the matching Rust type when arg_t resolves to a + // numeric / temporal / uuid / bytea type; otherwise they bind as TEXT. + assert_convert_val_consistent( + "String('42')/smallint", + json!("42"), + "smallint", + typ_for("smallint"), + Type::INT2, + ); + assert_convert_val_consistent( + "String('42')/int", + json!("42"), + "int", + typ_for("int"), + Type::INT4, + ); + assert_convert_val_consistent( + "String('42')/bigint", + json!("42"), + "bigint", + typ_for("bigint"), + Type::INT8, + ); + assert_convert_val_consistent( + "String(uuid)/uuid", + json!("550e8400-e29b-41d4-a716-446655440000"), + "uuid", + typ_for("uuid"), + Type::UUID, + ); + assert_convert_val_consistent( + "String(date)/date", + json!("2024-01-15"), + "date", + typ_for("date"), + Type::DATE, + ); + assert_convert_val_consistent( + "String(time)/time", + json!("10:30:00"), + "time", + typ_for("time"), + Type::TIME, + ); + assert_convert_val_consistent( + "String(time)/timetz", + json!("10:30:00"), + "timetz", + typ_for("timetz"), + Type::TIME, + ); + assert_convert_val_consistent( + "String(ts)/timestamp", + json!("2024-01-15T10:30:00"), + "timestamp", + typ_for("timestamp"), + Type::TIMESTAMP, + ); + assert_convert_val_consistent( + "String(tstz)/timestamptz", + json!("2024-01-15T10:30:00Z"), + "timestamptz", + typ_for("timestamptz"), + Type::TIMESTAMPTZ, + ); + assert_convert_val_consistent( + "String(b64)/bytea", + json!("aGVsbG8="), + "bytea", + typ_for("bytea"), + Type::BYTEA, + ); + // Generic text arms. + for (arg_t, expected) in [ + ("text", Type::TEXT), + ("varchar", Type::VARCHAR), + ("character varying", Type::VARCHAR), + // Unknown → TEXT (prepare fallback in dispatch). + ("my_custom_enum", Type::TEXT), + ] { + assert_convert_val_consistent( + &format!("String('hello')/{arg_t}"), + json!("hello"), + arg_t, + typ_for(arg_t), + expected, + ); + } + } + + #[test] + fn convert_val_object_against_every_arg_t() { + // Object values: bind as JSONB (default), JSON if explicitly typed, + // or JSON-stringify into TEXT/VARCHAR when arg_t is text-like. + assert_convert_val_consistent( + "Object/jsonb", + json!({"k": 1}), + "jsonb", + typ_for("jsonb"), + Type::JSONB, + ); + assert_convert_val_consistent( + "Object/json", + json!({"k": 1}), + "json", + typ_for("json"), + Type::JSON, + ); + assert_convert_val_consistent( + "Object/text", + json!({"k": 1}), + "text", + typ_for("text"), + Type::TEXT, + ); + assert_convert_val_consistent( + "Object/varchar", + json!({"k": 1}), + "varchar", + typ_for("varchar"), + Type::VARCHAR, + ); + // Parser-default text (Typ::Str) still routes to TEXT-string via the + // `matches!(typ, Typ::Str(_))` arm. + assert_convert_val_consistent( + "Object/parser-default-text", + json!({"k": 1}), + "text", + Typ::Str(None), + Type::TEXT, + ); + // Unknown arg_t parses to Typ::Str (parser's catch-all), so the + // text-coercion arm picks it up — Box + TEXT. The dispatch + // then takes the prepare + query_raw path because otyp_to_pg_type + // returns Err for the unknown name, letting the server resolve the + // actual column type (e.g. a custom enum that accepts JSON via cast). + assert_convert_val_consistent( + "Object/my_custom_enum", + json!({"k": 1}), + "my_custom_enum", + typ_for("my_custom_enum"), + Type::TEXT, + ); + } + + #[test] + fn convert_val_array_against_every_arg_t() { + // arg_t with [] suffix routes to convert_vec_val. + let int_array_cases: &[(&str, Type)] = &[ + ("int[]", Type::INT4_ARRAY), + ("integer[]", Type::INT4_ARRAY), + ("int4[]", Type::INT4_ARRAY), + ("smallint[]", Type::INT2_ARRAY), + ("bigint[]", Type::INT8_ARRAY), + ]; + for (arg_t, expected) in int_array_cases { + assert_convert_val_consistent( + &format!("Array([1,2])/{arg_t}"), + json!([1, 2]), + arg_t, + typ_for(arg_t), + expected.clone(), + ); + } + assert_convert_val_consistent( + "Array(strs)/text[]", + json!(["a", "b"]), + "text[]", + typ_for("text[]"), + Type::TEXT_ARRAY, + ); + assert_convert_val_consistent( + "Array(strs)/varchar[]", + json!(["a", "b"]), + "varchar[]", + typ_for("varchar[]"), + Type::VARCHAR_ARRAY, + ); + assert_convert_val_consistent( + "Array(bools)/bool[]", + json!([true, false]), + "bool[]", + typ_for("bool[]"), + Type::BOOL_ARRAY, + ); + assert_convert_val_consistent( + "Array(floats)/double[]", + json!([1.5, 2.5]), + "double[]", + typ_for("double[]"), + Type::FLOAT8_ARRAY, + ); + assert_convert_val_consistent( + "Array(uuids)/uuid[]", + json!(["550e8400-e29b-41d4-a716-446655440000"]), + "uuid[]", + typ_for("uuid[]"), + Type::UUID_ARRAY, + ); + // `timetz[]` falls back to TIME_ARRAY for the same reason the scalar + // `timetz` falls back to TIME — chrono's `NaiveTime` only encodes for + // TIME. The encoder check (to_sql_checked) catches a mistakenly + // asserted TIMETZ_ARRAY here. + assert_convert_val_consistent( + "Array(times)/timetz[] → TIME_ARRAY", + json!(["10:30:00", "11:00:00"]), + "timetz[]", + typ_for("timetz[]"), + Type::TIME_ARRAY, + ); + assert_convert_val_consistent( + "Array(times)/time[]", + json!(["10:30:00", "11:00:00"]), + "time[]", + typ_for("time[]"), + Type::TIME_ARRAY, + ); + // Array without [] suffix on arg_t: bind as JSONB (or JSON / TEXT). + assert_convert_val_consistent( + "Array/jsonb (no [])", + json!([1, 2, 3]), + "jsonb", + typ_for("jsonb"), + Type::JSONB, + ); + assert_convert_val_consistent( + "Array/json (no [])", + json!([1, 2, 3]), + "json", + typ_for("json"), + Type::JSON, + ); + assert_convert_val_consistent( + "Array/text (parser-default)", + json!([1, 2, 3]), + "text", + typ_for("text"), + Type::TEXT, + ); + // See Object/my_custom_enum: Typ::Str catch-all → TEXT-stringify; + // dispatch falls back to prepare for the unknown arg_t. + assert_convert_val_consistent( + "Array/my_custom_enum", + json!([1, 2, 3]), + "my_custom_enum", + typ_for("my_custom_enum"), + Type::TEXT, + ); + // NULL-array shape: Value::Null with arg_t ending in [] + assert_convert_val_consistent( + "Null/int[]", + Value::Null, + "int[]", + typ_for("int[]"), + Type::INT4_ARRAY, + ); + } + + /// Edge cases mirroring what the SDKs (TS / Python) and hand-written PG + /// scripts can actually emit. Each case is a real input → encode round + /// trip, and would have failed under #8988 if the asserted Type drifted + /// from the binding's Rust type. + #[test] + fn convert_val_sdk_edge_cases() { + // TS SDK shapes — `${val}` is auto-tagged with ::TYPE. + assert_convert_val_consistent( + "TS SDK ${42}", + json!(42), + "bigint", + typ_for("bigint"), + Type::INT8, + ); + assert_convert_val_consistent( + "TS SDK ${3.14}", + json!(3.14), + "double", + typ_for("double"), + Type::FLOAT8, + ); + assert_convert_val_consistent( + "TS SDK ${true}", + json!(true), + "boolean", + typ_for("boolean"), + Type::BOOL, + ); + assert_convert_val_consistent( + "TS SDK ${\"hello\"}", + json!("hello"), + "text", + typ_for("text"), + Type::TEXT, + ); + assert_convert_val_consistent( + "TS SDK ${{x:1}}", + json!({"x": 1}), + "json", + typ_for("json"), + Type::JSON, + ); + assert_convert_val_consistent( + "TS SDK ${[1,2,3]}", + json!([1, 2, 3]), + "json", + typ_for("json"), + Type::JSON, + ); + assert_convert_val_consistent( + "TS SDK ${null}", + Value::Null, + "text", + typ_for("text"), + Type::TEXT, + ); + + // CAST(${val} AS T) shape — the SDK strips its own ::TYPE here, so + // the parser sees a bare $N and otyp defaults to "text" *with + // otyp_inferred = true*. This is the original regression #8988 + // introduced; the inferred-default flag is what lets convert_val + // bind the value's natural type rather than coerce to TEXT. + assert_convert_val_consistent_inferred( + "CAST AS bool / Bool true", + json!(true), + "text", + typ_for("text"), + Type::BOOL, + ); + assert_convert_val_consistent_inferred( + "CAST AS bool / Bool false", + json!(false), + "text", + typ_for("text"), + Type::BOOL, + ); + // Object falls into the text-coercion arm (Object branch checks + // `matches!(typ, Typ::Str(_))` regardless of otyp_inferred — JSON + // serialisation is always safer than asserting JSONB for an + // unannotated arg). + assert_convert_val_consistent_inferred( + "CAST AS jsonb / Object", + json!({"a": 1, "b": [2, 3]}), + "text", + typ_for("text"), + Type::TEXT, + ); + assert_convert_val_consistent_inferred( + "CAST AS int / Number", + json!(7), + "text", + typ_for("text"), + Type::INT8, + ); + + // Python SDK datatable shape — type sits in the declaration comment + // (`-- $1 arg1 (BIGINT)`). Parser resolves otyp before convert_val. + assert_convert_val_consistent( + "Python decl bigint / Number", + json!(42), + "bigint", + typ_for("bigint"), + Type::INT8, + ); + assert_convert_val_consistent( + "Python decl text / String", + json!("hello"), + "text", + typ_for("text"), + Type::TEXT, + ); + assert_convert_val_consistent( + "Python decl jsonb / Object", + json!({"k": [1, 2]}), + "jsonb", + typ_for("jsonb"), + Type::JSONB, + ); + + // Mismatched-but-coercible JSON shape: JSON int 0/1 into a bool col + // still works because tokio_postgres encodes Number as INT8 and + // postgres has int→bool cast at the SQL site. Uses the inferred + // path (parser-default text otyp). + assert_convert_val_consistent_inferred( + "Number(0)/bool (parser default)", + json!(0), + "text", + typ_for("text"), + Type::INT8, + ); + } + + /// `decimal_fits_f64_losslessly` returns true for values that round-trip + /// through f64 and false for values that don't. This is the predicate + /// behind the one-shot precision-loss warning. + #[test] + fn decimal_fits_f64_losslessly_predicate() { + use std::str::FromStr; + // Values that fit f64 cleanly: + for s in &["0", "1", "-1", "3.14", "1234.5", "-0.5", "10000000000"] { + let d = Decimal::from_str(s).unwrap(); + assert!( + decimal_fits_f64_losslessly(&d), + "expected `{s}` to fit f64 losslessly" + ); + } + // Values past f64's ~15 significant-digit window lose precision: + for s in &[ + "12345678901234.56789", // 19 sig digits + "0.123456789012345678", // 18 sig digits past the decimal + "99999999999999999999", // 20-digit integer + ] { + let d = Decimal::from_str(s).unwrap(); + assert!( + !decimal_fits_f64_losslessly(&d), + "expected `{s}` to NOT fit f64 losslessly" + ); + } + } + + /// `should_check_precision` returns `true` exactly + /// `NUMERIC_PRECISION_CHECK_BUDGET` times, then `false` forever — and + /// `false` immediately once the precision-loss flag has been set, so the + /// hot path on a numeric-heavy result set is one cheap atomic load after + /// the first lossy value is observed. + #[test] + fn precision_check_budget_caps_per_query_overhead() { + use std::sync::atomic::Ordering; + let state = ResultFormatState::default(); + let mut allowed = 0u32; + let mut denied = 0u32; + for _ in 0..(NUMERIC_PRECISION_CHECK_BUDGET + 100) { + if state.should_check_precision() { + allowed += 1; + } else { + denied += 1; + } + } + assert_eq!(allowed, NUMERIC_PRECISION_CHECK_BUDGET); + assert_eq!(denied, 100); + // The flag short-circuits the budget — once set, no more checks run + // even if the budget hadn't been spent. + let state = ResultFormatState::default(); + state.numeric_precision_loss.store(true, Ordering::Relaxed); + for _ in 0..10 { + assert!(!state.should_check_precision()); + } + // Budget untouched. + assert_eq!( + state.numeric_precision_check_budget.load(Ordering::Relaxed), + NUMERIC_PRECISION_CHECK_BUDGET + ); + } + + /// Sparse positional placeholders renumber to a contiguous 1..=N without + /// substring collisions OR mangling string-literal/comment occurrences. + /// The pre-existing `String::replace` chain turned `$50` into `$10` when + /// oidx=5 was processed first; even the regex-with-greedy-digits approach + /// (a regression of its own) walked through string literals. The current + /// position-aware rewrite uses the parser's tokenizer to skip those. + #[test] + fn renumber_sparse_placeholders_no_collision_no_string_mangling() { + fn renumber(input: &str, mapping: &HashMap) -> String { + let mut out = input.to_owned(); + let mut positions = windmill_parser_sql::parse_pg_statement_arg_positions(input); + positions.sort_by_key(|(_, range)| std::cmp::Reverse(range.start)); + for (oidx, range) in positions { + if let Some(new_i) = mapping.get(&oidx) { + if oidx as usize != *new_i { + out.replace_range(range, &new_i.to_string()); + } + } + } + out + } + + let mapping: HashMap = [(5, 1), (50, 2)].into_iter().collect(); + let cases = &[ + // Two placeholders, full rewrite (greedy-digit collision check). + ("SELECT $5, $50", "SELECT $1, $2"), + // Same input flipped — order independence. + ("SELECT $50, $5", "SELECT $2, $1"), + // Repeat use of an index — every site gets rewritten. + ( + "SELECT $5 FROM t WHERE id = $5 OR ref = $50", + "SELECT $1 FROM t WHERE id = $1 OR ref = $2", + ), + // Index outside the mapping is left intact. + ("SELECT $5, $99", "SELECT $1, $99"), + // String literal containing the same `$N` syntax must not be + // rewritten — the tokenizer marks it as inside a string. + ( + "SELECT 'price: $5' AS lbl, $5 FROM t", + "SELECT 'price: $5' AS lbl, $1 FROM t", + ), + // Single-line comment must not be rewritten either. + ("-- mention $5\nSELECT $5", "-- mention $5\nSELECT $1"), + // Dollar-quoted block ($$ … $$) must not be rewritten. + ("SELECT $$body with $5$$, $5", "SELECT $$body with $5$$, $1"), + ]; + for (input, expected) in cases { + assert_eq!( + renumber(input, &mapping).as_str(), + *expected, + "input={input}" + ); + } + } + + /// Drift-prevention: `otyp_to_pg_type` and `convert_val` must agree on the + /// Type for every recognised arg_t when the JSON value matches the arg_t's + /// "natural" Rust kind. Fails if someone adds a new arg_t to one but not + /// the other, or changes the Type returned by either. + #[test] + fn otyp_to_pg_type_and_convert_val_agree_for_recognised_types() { + // (arg_t, natural-value, expected scalar Type) + let cases: &[(&str, Value, Type)] = &[ + ("bool", json!(true), Type::BOOL), + ("boolean", json!(false), Type::BOOL), + ("char", json!(65), Type::CHAR), + ("smallint", json!(1), Type::INT2), + ("smallserial", json!(1), Type::INT2), + ("int2", json!(1), Type::INT2), + ("serial2", json!(1), Type::INT2), + ("int", json!(1), Type::INT4), + ("integer", json!(1), Type::INT4), + ("int4", json!(1), Type::INT4), + ("serial", json!(1), Type::INT4), + ("bigint", json!(1), Type::INT8), + ("int8", json!(1), Type::INT8), + ("bigserial", json!(1), Type::INT8), + ("serial8", json!(1), Type::INT8), + ("real", json!(1.5), Type::FLOAT4), + ("float4", json!(1.5), Type::FLOAT4), + ("double", json!(1.5), Type::FLOAT8), + ("double precision", json!(1.5), Type::FLOAT8), + ("float8", json!(1.5), Type::FLOAT8), + ("numeric", json!(1), Type::NUMERIC), + ("decimal", json!(1), Type::NUMERIC), + ("oid", json!(1), Type::OID), + ( + "uuid", + json!("550e8400-e29b-41d4-a716-446655440000"), + Type::UUID, + ), + ("date", json!("2024-01-15"), Type::DATE), + ("time", json!("10:30:00"), Type::TIME), + // chrono::NaiveTime can only encode TIME — see the timetz arm. + ("timetz", json!("10:30:00"), Type::TIME), + ("timestamp", json!("2024-01-15T10:30:00"), Type::TIMESTAMP), + ( + "timestamptz", + json!("2024-01-15T10:30:00Z"), + Type::TIMESTAMPTZ, + ), + ("json", json!({"k": 1}), Type::JSON), + ("jsonb", json!({"k": 1}), Type::JSONB), + ("bytea", json!("aGVsbG8="), Type::BYTEA), + ("text", json!("hello"), Type::TEXT), + ("varchar", json!("hello"), Type::VARCHAR), + ("character varying", json!("hello"), Type::VARCHAR), + ]; + for (arg_t, value, expected) in cases { + // 1. The dispatch's "is recognised" gate must accept this arg_t. + // `timetz` is the one exception where we deliberately return + // TIME from convert_val (chrono limitation), but otyp_to_pg_type + // returns TIMETZ. + let from_otyp = otyp_to_pg_type(arg_t) + .unwrap_or_else(|e| panic!("otyp_to_pg_type lost arg_t `{arg_t}`: {e}")); + if *arg_t != "timetz" { + assert_eq!( + from_otyp, *expected, + "otyp_to_pg_type({arg_t}) drift: expected {expected}, got {from_otyp}" + ); + } + // 2. convert_val must produce a binding whose Type matches + // `expected`, AND whose Rust type successfully encodes against + // that Type (the to_sql_checked round-trip). + assert_convert_val_consistent( + &format!("meta/{arg_t}"), + value.clone(), + arg_t, + typ_for(arg_t), + expected.clone(), + ); + } + } } diff --git a/backend/windmill-worker/src/universal_pkg_installer.rs b/backend/windmill-worker/src/universal_pkg_installer.rs index 47b8401184..47b3d95358 100644 --- a/backend/windmill-worker/src/universal_pkg_installer.rs +++ b/backend/windmill-worker/src/universal_pkg_installer.rs @@ -390,7 +390,9 @@ pub async fn par_install_language_dependencies_seq< _platform_agnostic: bool, concurrent_downloads: usize, callback: impl Fn(RequiredDependency) -> Result + Send + Sync + 'static, - post_install: Option) -> anyhow::Result<()> + Send + Sync + 'static>>, + post_install: Option< + Arc) -> anyhow::Result<()> + Send + Sync + 'static>, + >, job_id: &'a Uuid, w_id: &'a str, worker_name: &'a str, @@ -448,13 +450,8 @@ pub async fn par_install_language_dependencies_seq< } if is_layered && offset > 0 { - windmill_queue::append_logs( - job_id, - w_id, - format!("\n\n--- Layer {} ---", i + 1), - conn, - ) - .await; + windmill_queue::append_logs(job_id, w_id, format!("\n\n--- Layer {} ---", i + 1), conn) + .await; } let layer_size = layer_deps.len(); @@ -625,7 +622,9 @@ async fn spawn_wrapped_installation_threads< _platform_agnostic: bool, counter_offset: Option, total_override: Option, - post_install: Option) -> anyhow::Result<()> + Send + Sync + 'static>>, + post_install: Option< + Arc) -> anyhow::Result<()> + Send + Sync + 'static>, + >, ) -> anyhow::Result<( Vec>>, tokio::sync::broadcast::Sender<()>, @@ -772,7 +771,9 @@ async fn try_install_one_detached<'a, T: Clone + std::marker::Send + Sync + 'a + // If dropped the entire installation fails and all installation threads are being stopped // That's why we just pass it to return so it is not being dropped kill_all_tasks: TaskKiller, - post_install: Option) -> anyhow::Result<()> + Send + Sync + 'static>>, + post_install: Option< + Arc) -> anyhow::Result<()> + Send + Sync + 'static>, + >, ) -> anyhow::Result { let start = std::time::Instant::now(); diff --git a/typescript-client/sqlUtils.ts b/typescript-client/sqlUtils.ts index 26af4aff92..6eac9893f5 100644 --- a/typescript-client/sqlUtils.ts +++ b/typescript-client/sqlUtils.ts @@ -153,6 +153,94 @@ function ducklakeProvider(name: string): SqlProvider { // Shared template function builder // --------------------------------------------------------------------------- +// Build a ready-to-execute SqlStatement. Used by both the template-tag +// path (which builds `content` from strings/values) and `.query()` (which +// gets a hand-written SQL string with positional placeholders). +function buildSqlStatement( + provider: SqlProvider, + content: string, + contentBody: string, + args: Record +): SqlStatement { + async function fetch({ + resultCollection, + }: FetchParams = {}) { + let finalContent = content; + if (resultCollection) + finalContent = `-- result_collection=${resultCollection}\n${finalContent}`; + try { + let result; + if (workerHasInternalServer()) { + result = await JobService.runScriptPreviewInline({ + workspace: getWorkspace(), + requestBody: { args, content: finalContent, language: provider.language }, + }); + } else { + result = await JobService.runScriptPreviewAndWaitResult({ + workspace: getWorkspace(), + requestBody: { args, content: finalContent, language: provider.language }, + }); + } + return result as SqlResult; + } catch (e: any) { + let err = e; + if ( + e && + typeof e.body == "string" && + e.statusText == "Internal Server Error" + ) { + let body = e.body; + if (body.startsWith("Internal:")) body = body.slice(9).trim(); + if (body.startsWith("Error:")) body = body.slice(6).trim(); + if (body.startsWith("datatable")) body = body.slice(9).trim(); + err = Error(`${provider.providerName} ${body}`); + err.query = contentBody; + err.request = e.request; + } + throw err; + } + } + + return { + content, + args, + fetch, + fetchOne: (params) => + fetch({ ...params, resultCollection: "last_statement_first_row" }), + fetchOneScalar: (params) => + fetch({ + ...params, + resultCollection: "last_statement_first_row_scalar", + }), + execute: (params) => fetch(params), + } satisfies SqlStatement; +} + +// JSON-encode a JS value into something the executor can deserialize. The +// JSON.stringify-friendly representation of a JS value before sending it to +// the executor: +// - `bigint` → string. JSON.stringify on a bigint throws; the +// executor accepts numeric strings into BIGINT +// slots via `Value::String → INT8`. +// - `Date` → ISO-8601 string. inferSqlType maps these to +// `TIMESTAMPTZ`; the executor's `Value::String` +// arm parses ISO strings into `chrono::DateTime`. +// - non-finite `number` → string ("NaN" / "Infinity" / "-Infinity"). +// JSON.stringify renders these as `null`, which +// silently became NULL in the database. The +// executor accepts these literals via +// `Value::String → FLOAT8` (`f64::from_str`). +// - everything else → passed through unchanged. +function serializeArgValue(v: any): any { + if (typeof v === "bigint") return v.toString(); + if (v instanceof Date) return v.toISOString(); + if (typeof v === "number" && !Number.isFinite(v)) { + if (Number.isNaN(v)) return "NaN"; + return v > 0 ? "Infinity" : "-Infinity"; + } + return v; +} + function buildSqlTemplateFunction(provider: SqlProvider): SqlTemplateFunction { let sqlFn = ((strings: TemplateStringsArray, ...values: any[]) => { // Separate raw vs parameterized values, assigning arg indices only to params @@ -212,62 +300,12 @@ function buildSqlTemplateFunction(provider: SqlProvider): SqlTemplateFunction { ...Object.fromEntries( valueInfos .filter((info): info is Extract<(typeof valueInfos)[number], { raw: false }> => !info.raw) - .map((info) => [`arg${info.argNum}`, info.value]) + .map((info) => [`arg${info.argNum}`, serializeArgValue(info.value)]) ), ...provider.extraArgs, }; - async function fetch({ - resultCollection, - }: FetchParams = {}) { - if (resultCollection) - content = `-- result_collection=${resultCollection}\n${content}`; - try { - let result; - if (workerHasInternalServer()) { - result = await JobService.runScriptPreviewInline({ - workspace: getWorkspace(), - requestBody: { args, content, language: provider.language }, - }); - } else { - result = await JobService.runScriptPreviewAndWaitResult({ - workspace: getWorkspace(), - requestBody: { args, content, language: provider.language }, - }); - } - return result as SqlResult; - } catch (e: any) { - let err = e; - if ( - e && - typeof e.body == "string" && - e.statusText == "Internal Server Error" - ) { - let body = e.body; - if (body.startsWith("Internal:")) body = body.slice(9).trim(); - if (body.startsWith("Error:")) body = body.slice(6).trim(); - if (body.startsWith("datatable")) body = body.slice(9).trim(); - err = Error(`${provider.providerName} ${body}`); - err.query = contentBody; - err.request = e.request; - } - throw err; - } - } - - return { - content, - args, - fetch, - fetchOne: (params) => - fetch({ ...params, resultCollection: "last_statement_first_row" }), - fetchOneScalar: (params) => - fetch({ - ...params, - resultCollection: "last_statement_first_row_scalar", - }), - execute: (params) => fetch(params), - } satisfies SqlStatement; + return buildSqlStatement(provider, content, contentBody, args); }) as SqlTemplateFunction; sqlFn.raw = (value: string) => new RawSql(value); @@ -293,12 +331,33 @@ function buildSqlTemplateFunction(provider: SqlProvider): SqlTemplateFunction { */ export function datatable(name: string = "main"): DatatableSqlTemplateFunction { let { name: n, schema } = parseName(name); - let sqlFn = buildSqlTemplateFunction( - datatableProvider(n, schema) - ) as DatatableSqlTemplateFunction; + let provider = datatableProvider(n, schema); + let sqlFn = buildSqlTemplateFunction(provider) as DatatableSqlTemplateFunction; + // `.query(sql, ...params)` is for SQL strings that already contain + // positional placeholders ($1, $2, ...). We DON'T go through the template + // builder here — that would re-emit each value as `$N::TYPE` and append + // them after the user's literal SQL, which is the bug previous versions of + // this method shipped. Instead we build the executor-shaped content + // directly: a `-- $N argN (TYPE)` declaration block (the parser picks + // these up as explicitly typed args) followed by the user's SQL verbatim. + // Note: we hand-roll the decl format here rather than calling + // `provider.formatArgDecl`, because the datatable formatter intentionally + // omits the type (the template-tag path emits `$N::TYPE` inline instead); + // for `.query()` we have no inline cast to fall back on. sqlFn.query = (sqlString: string, ...params: any[]) => { - let arr = Object.assign([sqlString], { raw: [sqlString] }); - return sqlFn(arr, ...params); + let argDecls = params + .map((v, i) => `-- $${i + 1} arg${i + 1} (${inferSqlType(v)})`) + .join("\n"); + let contentBody = sqlString; + let content = + (argDecls ? argDecls + "\n" : "") + provider.preamble() + sqlString; + let args = { + ...Object.fromEntries( + params.map((v, i) => [`arg${i + 1}`, serializeArgValue(v)]) + ), + ...provider.extraArgs, + }; + return buildSqlStatement(provider, content, contentBody, args); }; return sqlFn; } @@ -329,13 +388,27 @@ export function ducklake(name: string = "main"): SqlTemplateFunction { // These types exist in both DuckDB and Postgres // Check that the types exist if you plan to extend this function for other SQL engines. function inferSqlType(value: any): string { - if (typeof value === "number" || typeof value === "bigint") { + if (typeof value === "bigint") return "BIGINT"; + if (typeof value === "number") { if (Number.isInteger(value)) return "BIGINT"; return "DOUBLE PRECISION"; } else if (value === null || value === undefined) { return "TEXT"; } else if (typeof value === "string") { return "TEXT"; + } else if (Array.isArray(value)) { + // Homogeneous-primitive arrays auto-tag as `TYPE[]` so that values like + // `${[1,2,3]}` against an `int[]` column work without an explicit + // `${arr}::int[]` cast. For non-homogeneous or nested arrays we fall + // back to JSON, which works for jsonb columns. + return inferSqlArrayType(value); + } else if (value instanceof Date) { + // JS `Date` carries an absolute instant in UTC; map to TIMESTAMPTZ so + // `${someDate}` works against a `timestamptz` column without the user + // needing an explicit cast. Without this the typeof check above falls + // through to "object" → JSON, which only works by accident via + // PG's `json → text → timestamptz` implicit cast chain. + return "TIMESTAMPTZ"; } else if (typeof value === "object") { return "JSON"; } else if (typeof value === "boolean") { @@ -345,11 +418,45 @@ function inferSqlType(value: any): string { } } +function inferSqlArrayType(value: any[]): string { + if (value.length === 0) return "JSON"; + // Detect a single shared scalar JS type across all elements. Mixed types + // or any non-primitive element forces the JSON fallback. + let scalarType: string | undefined = undefined; + for (const elem of value) { + let elemType: string; + if (typeof elem === "bigint") elemType = "BIGINT"; + else if (typeof elem === "number") + elemType = Number.isInteger(elem) ? "BIGINT" : "DOUBLE PRECISION"; + else if (typeof elem === "string") elemType = "TEXT"; + else if (typeof elem === "boolean") elemType = "BOOLEAN"; + else return "JSON"; + if (scalarType === undefined) scalarType = elemType; + else if (scalarType === "BIGINT" && elemType === "DOUBLE PRECISION") + scalarType = "DOUBLE PRECISION"; + else if (scalarType === "DOUBLE PRECISION" && elemType === "BIGINT") { + // already widened + } else if (scalarType !== elemType) { + return "JSON"; + } + } + return `${scalarType}[]`; +} + // The goal is to detect if the user added a type annotation manually // // untyped : sql`SELECT ${x} = 0` => ['SELECT ', ' = 0'] // typed : sql`SELECT ${x}::int = 0` => ['SELECT ', '::int = 0'] // typed : sql`SELECT CAST ( ${x} AS int ) = 0` => ['SELECT CAST ( ', ' AS int ) = 0'] +// +// Caveat: the returned string is only meaningful as a *presence* signal — +// the only consumer (`formatArgUsage`) just checks `explicitType !== undefined` +// to decide whether to emit `$N` (user already wrote a cast) vs `$N::TYPE` +// (SDK injects the inferred cast). The returned string itself can be +// imprecise — e.g. `${x}::DOUBLE PRECISION` returns `"DOUBLE"` (split on +// whitespace), and `CAST(${x} AS int)` returns `"int)"` (no paren stripping). +// Don't rely on the returned string as a parsed PG type; only on whether +// it's defined. function parseTypeAnnotation( prevTemplateString: string | undefined, nextTemplateString: string | undefined diff --git a/typescript-client/tests/sqlUtils.test.ts b/typescript-client/tests/sqlUtils.test.ts new file mode 100644 index 0000000000..8fde3c14e8 --- /dev/null +++ b/typescript-client/tests/sqlUtils.test.ts @@ -0,0 +1,626 @@ +/** + * Standalone tests for `wmill.datatable()` / `wmill.ducklake()` SQL template + * functions. + * + * The real `sqlUtils.ts` imports `./services.gen` (auto-generated, not in + * the repo) so we can't import it here. Instead we re-implement the same + * type-inference / template-building / `.query()` pipeline inline (sans the + * network calls) and assert the `content` + `args` shapes the executor + * would receive. Each test maps 1:1 to a behaviour this PR introduces or + * fixes (BigInt, homogeneous arrays, `.query()` positional, etc.) so they + * also serve as a regression backstop. + * + * Run with: bun test typescript-client/tests/sqlUtils.test.ts + */ +import { expect, test, describe } from "bun:test"; + +// ============================================================================= +// Pure SDK logic (mirror of typescript-client/sqlUtils.ts — kept minimal, +// only the parts that decide content / args). +// ============================================================================= + +class RawSql { + readonly __brand = "RawSql" as const; + constructor(public readonly value: string) {} +} + +interface SqlProvider { + formatArgDecl(argNum: number, argType: string): string; + formatArgUsage( + argNum: number, + explicitType: string | undefined, + inferredType: string + ): string; + preamble(): string; + language: "postgresql" | "duckdb"; + extraArgs: Record; + providerName: string; +} + +function datatableProvider(name: string, schema?: string): SqlProvider { + return { + providerName: "datatable", + language: "postgresql", + extraArgs: { database: `datatable://${name}` }, + formatArgDecl: (argNum) => `-- $${argNum} arg${argNum}`, + formatArgUsage: (argNum, explicitType, inferredType) => + explicitType !== undefined + ? `$${argNum}` + : `$${argNum}::${inferredType}`, + preamble: () => (schema ? `SET search_path TO "${schema}";\n` : ""), + }; +} + +function ducklakeProvider(name: string): SqlProvider { + return { + providerName: "ducklake", + language: "duckdb", + extraArgs: {}, + formatArgDecl: (argNum, argType) => `-- $arg${argNum} (${argType})`, + formatArgUsage: (argNum) => `$arg${argNum}`, + preamble: () => `ATTACH 'ducklake://${name}' AS dl;USE dl;\n`, + }; +} + +function inferSqlType(value: any): string { + if (typeof value === "bigint") return "BIGINT"; + if (typeof value === "number") { + if (Number.isInteger(value)) return "BIGINT"; + return "DOUBLE PRECISION"; + } else if (value === null || value === undefined) { + return "TEXT"; + } else if (typeof value === "string") { + return "TEXT"; + } else if (Array.isArray(value)) { + return inferSqlArrayType(value); + } else if (value instanceof Date) { + return "TIMESTAMPTZ"; + } else if (typeof value === "object") { + return "JSON"; + } else if (typeof value === "boolean") { + return "BOOLEAN"; + } else { + return "TEXT"; + } +} + +function inferSqlArrayType(value: any[]): string { + if (value.length === 0) return "JSON"; + let scalarType: string | undefined = undefined; + for (const elem of value) { + let elemType: string; + if (typeof elem === "bigint") elemType = "BIGINT"; + else if (typeof elem === "number") + elemType = Number.isInteger(elem) ? "BIGINT" : "DOUBLE PRECISION"; + else if (typeof elem === "string") elemType = "TEXT"; + else if (typeof elem === "boolean") elemType = "BOOLEAN"; + else return "JSON"; + if (scalarType === undefined) scalarType = elemType; + else if (scalarType === "BIGINT" && elemType === "DOUBLE PRECISION") + scalarType = "DOUBLE PRECISION"; + else if (scalarType === "DOUBLE PRECISION" && elemType === "BIGINT") { + // already widened + } else if (scalarType !== elemType) { + return "JSON"; + } + } + return `${scalarType}[]`; +} + +function parseTypeAnnotation( + prevTemplateString: string | undefined, + nextTemplateString: string | undefined +): string | undefined { + if (!nextTemplateString) return; + nextTemplateString = nextTemplateString.trimStart(); + if (nextTemplateString.startsWith("::")) { + return nextTemplateString.substring(2).trimStart().split(/\s+/)[0]; + } + prevTemplateString = prevTemplateString?.trimEnd(); + if ( + prevTemplateString?.endsWith("(") && + prevTemplateString + .substring(0, prevTemplateString.length - 1) + .trim() + .toUpperCase() + .endsWith("CAST") && + nextTemplateString.toUpperCase().startsWith("AS ") + ) { + return nextTemplateString.substring(2).trimStart().split(/\s+/)[0]; + } +} + +function serializeArgValue(v: any): any { + if (typeof v === "bigint") return v.toString(); + if (v instanceof Date) return v.toISOString(); + if (typeof v === "number" && !Number.isFinite(v)) { + if (Number.isNaN(v)) return "NaN"; + return v > 0 ? "Infinity" : "-Infinity"; + } + return v; +} + +function buildContentAndArgs( + provider: SqlProvider, + strings: TemplateStringsArray | string[], + values: any[] +): { content: string; args: Record } { + let argIndex = 0; + const valueInfos = values.map((v, i) => { + if (v instanceof RawSql) + return { raw: true as const, value: v.value, originalIndex: i }; + argIndex++; + return { + raw: false as const, + value: v, + originalIndex: i, + argNum: argIndex, + }; + }); + + let argDecls = valueInfos + .filter((info): info is Extract => !info.raw) + .map((info) => { + let argType = + parseTypeAnnotation( + strings[info.originalIndex], + strings[info.originalIndex + 1] + ) || inferSqlType(info.value); + return provider.formatArgDecl(info.argNum, argType); + }); + + let content = argDecls.length ? argDecls.join("\n") + "\n" : ""; + content += provider.preamble(); + + let contentBody = ""; + for (let i = 0; i < strings.length; i++) { + contentBody += strings[i]; + if (i < valueInfos.length) { + let info = valueInfos[i]; + if (info.raw) { + contentBody += info.value; + } else { + let explicitType = parseTypeAnnotation( + strings[info.originalIndex], + strings[info.originalIndex + 1] + ); + let inferredType = inferSqlType(info.value); + contentBody += provider.formatArgUsage( + info.argNum, + explicitType, + inferredType + ); + } + } + } + content += contentBody; + + const args = { + ...Object.fromEntries( + valueInfos + .filter((info): info is Extract => !info.raw) + .map((info) => [`arg${info.argNum}`, serializeArgValue(info.value)]) + ), + ...provider.extraArgs, + }; + return { content, args }; +} + +function buildDatatableQuery( + provider: SqlProvider, + sqlString: string, + params: any[] +): { content: string; args: Record } { + let argDecls = params + .map((v, i) => `-- $${i + 1} arg${i + 1} (${inferSqlType(v)})`) + .join("\n"); + let content = + (argDecls ? argDecls + "\n" : "") + provider.preamble() + sqlString; + let args = { + ...Object.fromEntries( + params.map((v, i) => [`arg${i + 1}`, serializeArgValue(v)]) + ), + ...provider.extraArgs, + }; + return { content, args }; +} + +function templateTag(provider: SqlProvider) { + return (strings: TemplateStringsArray, ...values: any[]) => + buildContentAndArgs(provider, strings, values); +} + +const dt = (name = "main") => templateTag(datatableProvider(name)); +const dl = (name = "main") => templateTag(ducklakeProvider(name)); +const datatableQuery = (name = "main") => { + const provider = datatableProvider(name); + return (sql: string, ...params: any[]) => + buildDatatableQuery(provider, sql, params); +}; + +// ============================================================================= +// inferSqlType — exhaustive coverage +// ============================================================================= + +describe("inferSqlType — primitives", () => { + test("integer Number → BIGINT", () => { + expect(inferSqlType(0)).toBe("BIGINT"); + expect(inferSqlType(42)).toBe("BIGINT"); + expect(inferSqlType(-7)).toBe("BIGINT"); + expect(inferSqlType(Number.MAX_SAFE_INTEGER)).toBe("BIGINT"); + }); + + test("non-integer Number → DOUBLE PRECISION", () => { + expect(inferSqlType(0.5)).toBe("DOUBLE PRECISION"); + expect(inferSqlType(-3.14)).toBe("DOUBLE PRECISION"); + expect(inferSqlType(Number.EPSILON)).toBe("DOUBLE PRECISION"); + }); + + test("BigInt → BIGINT (not DOUBLE PRECISION)", () => { + // Pre-fix this branch was unreachable because bigint was bundled with + // number and `Number.isInteger(BigInt)` returns false → would have + // returned DOUBLE PRECISION (wrong). The split-out check is the fix. + expect(inferSqlType(BigInt(0))).toBe("BIGINT"); + expect(inferSqlType(BigInt("9007199254740993"))).toBe("BIGINT"); + expect(inferSqlType(BigInt(-1))).toBe("BIGINT"); + }); + + test("string / null / undefined → TEXT", () => { + expect(inferSqlType("")).toBe("TEXT"); + expect(inferSqlType("hello")).toBe("TEXT"); + expect(inferSqlType(null)).toBe("TEXT"); + expect(inferSqlType(undefined)).toBe("TEXT"); + }); + + test("boolean → BOOLEAN", () => { + expect(inferSqlType(true)).toBe("BOOLEAN"); + expect(inferSqlType(false)).toBe("BOOLEAN"); + }); + + test("plain object → JSON", () => { + expect(inferSqlType({})).toBe("JSON"); + expect(inferSqlType({ a: 1, b: [1, 2] })).toBe("JSON"); + }); +}); + +describe("inferSqlType — arrays", () => { + test("empty array → JSON", () => { + expect(inferSqlType([])).toBe("JSON"); + }); + + test("homogeneous integer array → BIGINT[]", () => { + expect(inferSqlType([1, 2, 3])).toBe("BIGINT[]"); + expect(inferSqlType([0])).toBe("BIGINT[]"); + expect(inferSqlType([-1, 0, 1])).toBe("BIGINT[]"); + }); + + test("homogeneous float array → DOUBLE PRECISION[]", () => { + expect(inferSqlType([1.5, 2.5])).toBe("DOUBLE PRECISION[]"); + }); + + test("mixed int/float array widens to DOUBLE PRECISION[]", () => { + expect(inferSqlType([1, 2.5])).toBe("DOUBLE PRECISION[]"); + expect(inferSqlType([1.5, 2])).toBe("DOUBLE PRECISION[]"); + }); + + test("homogeneous string array → TEXT[]", () => { + expect(inferSqlType(["a", "b", "c"])).toBe("TEXT[]"); + expect(inferSqlType([""])).toBe("TEXT[]"); + }); + + test("homogeneous bool array → BOOLEAN[]", () => { + expect(inferSqlType([true, false, true])).toBe("BOOLEAN[]"); + }); + + test("homogeneous bigint array → BIGINT[]", () => { + expect(inferSqlType([BigInt(1), BigInt(2)])).toBe("BIGINT[]"); + }); + + test("non-homogeneous array → JSON", () => { + expect(inferSqlType([1, "x"])).toBe("JSON"); + expect(inferSqlType(["a", true])).toBe("JSON"); + expect(inferSqlType([1, null])).toBe("JSON"); + expect(inferSqlType([true, 1])).toBe("JSON"); + }); + + test("nested array → JSON (current limitation, no auto-tag for 2D)", () => { + expect(inferSqlType([[1], [2]])).toBe("JSON"); + expect(inferSqlType([{ a: 1 }, { a: 2 }])).toBe("JSON"); + }); +}); + +// ============================================================================= +// parseTypeAnnotation — used by the SDK to suppress its own ::TYPE injection +// when the user already wrote a cast. +// ============================================================================= + +describe("parseTypeAnnotation — user-supplied cast detection", () => { + test("`${x}::int` → 'int'", () => { + expect(parseTypeAnnotation("SELECT ", "::int FROM t")).toBe("int"); + }); + test("whitespace tolerance after ::", () => { + expect(parseTypeAnnotation("SELECT ", " :: bigint FROM t")).toBe( + "bigint" + ); + }); + test("`CAST(${x} AS int)` → first whitespace-delimited word after AS", () => { + // The SDK splits on whitespace and doesn't strip closing parens, so + // `AS int)` returns "int)". The exact value doesn't matter downstream + // because the SDK only checks `explicitType !== undefined` to skip its + // own ::cast injection — but we lock the behaviour in. + expect(parseTypeAnnotation("SELECT CAST(", " AS int)")).toBe("int)"); + }); + test("`CAST ( ${x} AS BOOL )` (whitespace + caps)", () => { + // Whitespace before `)` causes split to drop it, so this returns "BOOL". + expect(parseTypeAnnotation("SELECT CAST ( ", " AS BOOL )")).toBe("BOOL"); + }); + test("no cast adjacent → undefined", () => { + expect(parseTypeAnnotation("SELECT ", " FROM t")).toBeUndefined(); + expect(parseTypeAnnotation("SELECT ", "")).toBeUndefined(); + expect(parseTypeAnnotation(undefined, undefined)).toBeUndefined(); + }); +}); + +// ============================================================================= +// datatable() template tag — content + args round-trips +// ============================================================================= + +describe("datatable() — template tag", () => { + test("primitives auto-tag with ::TYPE inline; decls have no type", () => { + const sql = dt(); + const out = sql`SELECT ${42}, ${3.14}, ${true}, ${"x"}, ${null}`; + // datatable provider's formatArgDecl ignores the type, so we get bare + // decls + the casts in the SQL body. + expect(out.content).toContain("-- $1 arg1\n"); + expect(out.content).toContain("$1::BIGINT"); + expect(out.content).toContain("$2::DOUBLE PRECISION"); + expect(out.content).toContain("$3::BOOLEAN"); + expect(out.content).toContain("$4::TEXT"); + expect(out.content).toContain("$5::TEXT"); + expect(out.args).toMatchObject({ + arg1: 42, + arg2: 3.14, + arg3: true, + arg4: "x", + arg5: null, + }); + }); + + test("user `${x}::int` suppresses SDK's auto-cast (parser sees user's cast)", () => { + const sql = dt(); + const out = sql`SELECT ${42}::int`; + expect(out.content).toContain("SELECT $1::int"); + expect(out.content).not.toContain("$1::BIGINT"); + }); + + test("CAST(${x} AS T) syntax → bare $N in SQL (regression #8988)", () => { + const sql = dt(); + const out = sql`SELECT CAST(${true} AS bool)`; + expect(out.content).toContain("CAST($1 AS bool)"); + expect(out.content).not.toContain("$1::BOOLEAN"); + }); + + test("BigInt is stringified for JSON transport, tagged as ::BIGINT", () => { + const sql = dt(); + const out = sql`SELECT ${BigInt("9007199254740993")}`; + expect(out.content).toContain("$1::BIGINT"); + expect(out.args.arg1).toBe("9007199254740993"); + // Round-trip through JSON without throwing — the original bug. + expect(() => JSON.stringify(out.args)).not.toThrow(); + }); + + test("BigInt zero / negative / large", () => { + const sql = dt(); + expect(sql`SELECT ${BigInt(0)}`.args.arg1).toBe("0"); + expect(sql`SELECT ${BigInt(-1)}`.args.arg1).toBe("-1"); + expect(sql`SELECT ${BigInt("99999999999999999999")}`.args.arg1).toBe( + "99999999999999999999" + ); + }); + + test("Date is auto-tagged ::TIMESTAMPTZ and ISO-stringified", () => { + // Pre-fix: typeof Date === "object" → ::JSON, then PG cast chain + // worked accidentally for `${date}::timestamptz`. Now: explicit + // ::TIMESTAMPTZ + Date.toISOString() so plain `${date}` against a + // timestamptz column doesn't need a cast. + const sql = dt(); + const d = new Date("2024-01-15T10:30:00.000Z"); + const out = sql`SELECT ${d} AS t`; + expect(out.content).toContain("$1::TIMESTAMPTZ"); + expect(out.args.arg1).toBe("2024-01-15T10:30:00.000Z"); + expect(() => JSON.stringify(out.args)).not.toThrow(); + }); + + test("non-finite Number is stringified for the executor", () => { + // JSON.stringify(NaN) and JSON.stringify(Infinity) both produce `null`, + // which silently became NULL in the database. The executor accepts + // "NaN" / "Infinity" / "-Infinity" via `Value::String → FLOAT8` + // (`f64::from_str`), so we send the special values as strings. + const sql = dt(); + expect(sql`SELECT ${NaN}`.args.arg1).toBe("NaN"); + expect(sql`SELECT ${Infinity}`.args.arg1).toBe("Infinity"); + expect(sql`SELECT ${-Infinity}`.args.arg1).toBe("-Infinity"); + // Tag stays DOUBLE PRECISION (these are floats). + expect(sql`SELECT ${NaN}`.content).toContain("$1::DOUBLE PRECISION"); + }); + + test("homogeneous arrays auto-tag with TYPE[]", () => { + const sql = dt(); + expect(sql`SELECT ${[1, 2, 3]}`.content).toContain("$1::BIGINT[]"); + expect(sql`SELECT ${[1.5, 2.5]}`.content).toContain( + "$1::DOUBLE PRECISION[]" + ); + expect(sql`SELECT ${["a", "b"]}`.content).toContain("$1::TEXT[]"); + expect(sql`SELECT ${[true, false]}`.content).toContain("$1::BOOLEAN[]"); + }); + + test("non-homogeneous and empty arrays fall back to JSON", () => { + const sql = dt(); + expect(sql`SELECT ${[1, "x"]}`.content).toContain("$1::JSON"); + expect(sql`SELECT ${[]}`.content).toContain("$1::JSON"); + expect(sql`SELECT ${[[1], [2]]}`.content).toContain("$1::JSON"); + }); + + test("mixed-numeric array widens to DOUBLE PRECISION[]", () => { + const sql = dt(); + expect(sql`SELECT ${[1, 2.5]}`.content).toContain( + "$1::DOUBLE PRECISION[]" + ); + }); + + test("multiple args get distinct decls + numbered placeholders", () => { + const sql = dt(); + const out = sql`INSERT INTO t VALUES (${1}, ${"x"}, ${[true, false]})`; + expect(out.content).toContain("-- $1 arg1"); + expect(out.content).toContain("-- $2 arg2"); + expect(out.content).toContain("-- $3 arg3"); + expect(out.content).toContain("$1::BIGINT"); + expect(out.content).toContain("$2::TEXT"); + expect(out.content).toContain("$3::BOOLEAN[]"); + expect(out.args).toMatchObject({ + arg1: 1, + arg2: "x", + arg3: [true, false], + }); + }); + + test("RawSql is inlined verbatim, doesn't consume an arg index", () => { + const sql = dt(); + const col = new RawSql("name"); + const out = sql`SELECT ${col} FROM t WHERE id = ${42}`; + // Only one decl, only one arg in args dict. + expect(out.content.match(/^-- \$\d+/gm)?.length).toBe(1); + expect(out.content).toContain("SELECT name FROM t WHERE id = $1::BIGINT"); + expect(Object.keys(out.args).filter((k) => k.startsWith("arg")).length).toBe( + 1 + ); + expect(out.args).toMatchObject({ arg1: 42 }); + }); + + test("schema name is propagated as SET search_path preamble", () => { + const sql = dt("main"); + const out = sql`SELECT 1`; + expect(out.args.database).toBe("datatable://main"); + }); + + test("database extra arg is always present", () => { + const sql = dt("custom_db"); + const out = sql`SELECT ${1}`; + expect(out.args.database).toBe("datatable://custom_db"); + }); +}); + +// ============================================================================= +// datatable().query() — positional placeholders (the previously-broken path) +// ============================================================================= + +describe("datatable().query() — positional placeholders", () => { + test("emits typed declarations + SQL verbatim, no appended placeholders", () => { + const q = datatableQuery(); + const out = q("SELECT $1, $2", 42, "hello"); + expect(out.content).toContain("-- $1 arg1 (BIGINT)"); + expect(out.content).toContain("-- $2 arg2 (TEXT)"); + // Crucially: SQL must end with the user's SQL, NOT have placeholders + // appended after it (the pre-fix bug). + expect(out.content.endsWith("SELECT $1, $2")).toBe(true); + expect(out.args).toMatchObject({ arg1: 42, arg2: "hello" }); + }); + + test("BigInt args are stringified", () => { + const q = datatableQuery(); + const out = q("SELECT $1", BigInt("100")); + expect(out.args.arg1).toBe("100"); + expect(out.content).toContain("-- $1 arg1 (BIGINT)"); + }); + + test("array args auto-tag homogeneously in the decl block", () => { + const q = datatableQuery(); + const out = q("SELECT $1, $2", [1, 2], ["a", "b"]); + expect(out.content).toContain("-- $1 arg1 (BIGINT[])"); + expect(out.content).toContain("-- $2 arg2 (TEXT[])"); + }); + + test("zero params → no decl block, just SQL + extras", () => { + const q = datatableQuery(); + const out = q("SELECT 1"); + expect(out.content).not.toContain("-- $"); + expect(out.content).toContain("SELECT 1"); + // database extra still injected. + expect(out.args.database).toBe("datatable://main"); + }); + + test("ten params number contiguously", () => { + const q = datatableQuery(); + const params = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + const out = q("SELECT " + params.map((_, i) => `$${i + 1}`).join(","), ...params); + for (let i = 1; i <= 10; i++) { + expect(out.content).toContain(`-- $${i} arg${i} (BIGINT)`); + expect(out.args[`arg${i}`]).toBe(i); + } + }); + + test(".query()'s decl format matches what the executor parses (regression)", () => { + // The PG parser's RE_ARG_PGSQL is: + // ^-- \$(\d+) (\w+)(?: \(([A-Za-z0-9_\[\]]+)\))?(?: ?\= ?(.+))? *$ + // We assert our decl line matches that grammar so the executor doesn't + // fall back to "inferred default text". + const q = datatableQuery(); + const out = q("SELECT $1", 42); + const declRe = /^-- \$\d+ \w+ \([A-Za-z0-9_\[\]]+\) *$/m; + expect(out.content).toMatch(declRe); + }); +}); + +// ============================================================================= +// ducklake() template tag — DuckDB declares types in the comment (different +// shape from datatable). Same auto-tag rules apply for inferSqlType. +// ============================================================================= + +describe("ducklake() — DuckDB shape", () => { + test("declarations carry the type", () => { + const sql = dl("main"); + const out = sql`SELECT ${42}, ${"hello"}, ${true}`; + expect(out.content).toContain("-- $arg1 (BIGINT)"); + expect(out.content).toContain("-- $arg2 (TEXT)"); + expect(out.content).toContain("-- $arg3 (BOOLEAN)"); + // Preamble attaches the ducklake. + expect(out.content).toContain("ATTACH 'ducklake://main' AS dl;USE dl;"); + // Args are referenced via $argN syntax in the SQL body. + expect(out.content).toContain("$arg1"); + }); + + test("BigInt + homogeneous arrays propagate to ducklake too", () => { + const sql = dl("main"); + const out = sql`SELECT ${BigInt(9)}, ${[1, 2, 3]}, ${["a", "b"]}`; + expect(out.content).toContain("(BIGINT)"); + expect(out.content).toContain("(BIGINT[])"); + expect(out.content).toContain("(TEXT[])"); + expect(out.args.arg1).toBe("9"); + expect(out.args.arg2).toEqual([1, 2, 3]); + }); + + test("ducklake doesn't carry a database extra arg", () => { + const sql = dl(); + const out = sql`SELECT 1`; + expect(out.args).not.toHaveProperty("database"); + }); +}); + +// ============================================================================= +// Cross-cutting: the args dict must always be JSON-serialisable. +// ============================================================================= + +describe("args dict is JSON-serialisable for every supported value shape", () => { + test("BigInt, primitives, arrays, objects, raw — none throw", () => { + const sql = dt(); + const out = sql` + SELECT ${BigInt(1)}, ${1}, ${1.5}, ${"x"}, ${true}, ${null}, + ${[1, 2]}, ${["a", "b"]}, ${[true, false]}, + ${{ k: 1 }}, ${[1, "x"]} + `; + const json = JSON.stringify(out.args); + expect(typeof json).toBe("string"); + // BigInt got stringified, not thrown. + expect(json).toContain('"arg1":"1"'); + }); +});