Repository navigation
sql: add a jq function behind enable_jq - #39838
Conversation
b0b780e to
4f159d5
Compare
antiguru
left a comment
There was a problem hiding this comment.
Posted by Claude Code on behalf of @antiguru.
Suggestion: convert between jsonb datums and jaq values directly instead of through JSON text. Today each row is serialized to text, parsed by hifijson, evaluated, printed again, and reparsed by serde_json, which is two encodes and two parses per row. The direct walk keeps the observable semantics of the text path:
- Input numbers become
Num::Int/Num::BigIntfor integers andNum::Decotherwise, as jaq's reader does, so decimal precision survives until arithmetic. - Output numbers go through
strconv::parse_numeric, the parserjsonbtext uses. Non-finite floats are rejected. - Object keys are sorted with last-wins on duplicates. This matches
pack_dict, including keys that collide after invalid UTF-8 is replaced. - Output nesting is capped at 128 levels, the depth at which serde_json's recursion limit rejects
jsonbtext. - Byte strings and non-string keys get explicit errors instead of failing as unparsable text. The errors also no longer embed the whole output value.
The jaq-json serde feature is not a shortcut here. Val only implements Deserialize, visit_f64 turns decimals into floats, and the workspace's arbitrary_precision feature makes numbers arrive as $serde_json::private::Number maps that visit_map would turn into objects.
The proptests keep the text path as a test-only oracle and compare against it:
jsonbto jaq value, compared viaDebugbecauseVal'sPartialEqequates numbers acrossNumvariants.- Round trip from
jsonbthrough a jaq value back tojsonb. - Arbitrary jaq values to
jsonb. These include NaN, infinities, bigints, exponent decimals, byte strings, non-string keys, and invalid UTF-8. - Program outputs over arbitrary inputs.
- Depth at 127, 128, and 129.
I checked that mutating the duplicate-key handling and the integer detection each fails a test. I ran the unit tests and proptests for this change, but not the slt suite or clippy, and I have not measured the speedup.
Suggested change, against 4f159d5
The first part moves jq.rs to jq/mod.rs and its tests to jq/tests.rs.
diff --git a/src/expr/src/scalar/func/jq.rs b/src/expr/src/scalar/func/jq/mod.rs
similarity index 63%
rename from src/expr/src/scalar/func/jq.rs
rename to src/expr/src/scalar/func/jq/mod.rs
index 60b4b31e1d..a47990460c 100644
--- a/src/expr/src/scalar/func/jq.rs
+++ b/src/expr/src/scalar/func/jq/mod.rs
@@ -21,17 +21,17 @@
use std::cmp::Ordering;
use std::fmt;
use std::hash::{Hash, Hasher};
-use std::str::FromStr;
use std::sync::Arc;
use jaq_core::compile::Undefined;
use jaq_core::load::{self, Arena, File, Loader};
use jaq_core::native::{Fun, run, v};
use jaq_core::{Compiler, Ctx, Exn, Vars, data};
-use jaq_json::Val;
+use jaq_json::{Num, Val};
use mz_expr_derive::sqlfunc;
use mz_ore::str::StrExt;
use mz_repr::adt::jsonb::{Jsonb, JsonbRef};
+use mz_repr::{Datum, Row, RowPacker, strconv};
use serde::{Deserialize, Serialize};
use crate::EvalError;
@@ -83,9 +83,7 @@ impl JqProgram {
/// produces more than one value is an error, as jq's `[...]` collects a
/// stream into a single array.
pub fn eval(&self, input: JsonbRef<'_>) -> Result<Option<Jsonb>, EvalError> {
- // Values cross between Materialize and jaq as JSON text.
- let input = jaq_json::read::parse_single(input.to_string().as_bytes())
- .map_err(|e| error(format!("cannot read input: {e:?}")))?;
+ let input = to_val(input.into_datum());
let ctx = Ctx::<Data>::new(&self.filter.lut, Vars::new([]));
let mut outputs = self.filter.id.run((ctx, input));
let output = match outputs.next() {
@@ -98,10 +96,9 @@ impl JqProgram {
"program produced more than one value; collect them with [...]",
));
}
- let output = output.to_string();
- let output = Jsonb::from_str(&output)
- .map_err(|e| error(format!("output {output} is not JSON: {e}")))?;
- Ok(Some(output))
+ let mut row = Row::default();
+ pack_val(&mut row.packer(), &output, 0).map_err(error)?;
+ Ok(Some(Jsonb::from_row(row)))
}
}
@@ -254,60 +251,103 @@ fn error(msg: impl std::fmt::Display) -> EvalError {
EvalError::InvalidParameterValue(format!("jq: {msg}").into())
}
-#[cfg(test)]
-mod tests {
- use super::*;
+/// Nesting depth at which output fails to convert. Parsing `jsonb` text
+/// rejects the same depth, through serde_json's recursion limit.
+const MAX_OUTPUT_DEPTH: usize = 128;
- fn run(input: &str, program: &str) -> Result<Option<String>, String> {
- let input: Jsonb = input.parse().unwrap();
- let program = JqProgram::new(program).map_err(|e| format!("jq: {e}"))?;
- program
- .eval(input.as_ref())
- .map(|out| out.map(|out| out.to_string()))
- .map_err(|e| e.to_string())
- }
-
- #[mz_ore::test]
- #[cfg_attr(miri, ignore)] // too slow
- fn jq_eval() {
- assert_eq!(
- run(r#"{"a": [1, 2.5, 3]}"#, ".a | map(. * 2)"),
- Ok(Some("[2,5,6]".into()))
- );
- assert_eq!(
- run(
- r#"{"b": 2, "a": 1}"#,
- r#"to_entries | map(.key) | join(",")"#
- ),
- Ok(Some(r#""a,b""#.into()))
- );
- assert_eq!(
- run(r#"{"x": 123456789012345678901234567890.123}"#, ".x"),
- Ok(Some("123456789012345678901234567890.123".into()))
- );
- assert_eq!(run("[1, 2, 3]", ".[] | select(. > 5)"), Ok(None));
- assert_eq!(
- run("[1, 2, 3]", "[.[] | select(. > 1)]"),
- Ok(Some("[2,3]".into()))
- );
- // Logging filters are kept, as no-ops.
- assert_eq!(run("[1, 2]", "debug"), Ok(Some("[1,2]".into())));
- for (program, error) in [
- (
- ".[]",
- "jq: program produced more than one value; collect them with [...]",
- ),
- (". +", "jq: expected term at end of program"),
- ("now", "jq: undefined filter now/0"),
- ("env", "jq: undefined filter env/0"),
- (
- r#"import "x" as x; ."#,
- "jq: cannot import x: module loading not supported",
- ),
- (r#"error("boom")"#, r#"jq: "boom""#),
- ("halt_error(3)", "jq: program halted with exit code 3"),
- ] {
- assert_eq!(run("[1, 2]", program), Err(error.into()), "{program}");
+/// Converts a `jsonb` datum into a jaq value.
+///
+/// Numbers convert as jaq's own JSON reader converts their text: integers to
+/// `Num::Int` or `Num::BigInt`, and everything else to `Num::Dec`, which keeps
+/// the decimal text until arithmetic turns it into a float.
+fn to_val(datum: Datum<'_>) -> Val {
+ mz_ore::stack::maybe_grow(|| match datum {
+ Datum::JsonNull => Val::Null,
+ Datum::True => Val::Bool(true),
+ Datum::False => Val::Bool(false),
+ Datum::Numeric(n) => {
+ // Standard notation has no exponent, so an integer has no `.`.
+ let text = n.into_inner().to_standard_notation_string();
+ let num = if text.contains('.') {
+ None
+ } else {
+ Num::from_str_radix(&text, 10)
+ };
+ Val::Num(num.unwrap_or_else(|| Num::Dec(jaq_json::Rc::new(text))))
}
- }
+ Datum::String(s) => Val::utf8_str(s.as_bytes().to_vec()),
+ Datum::List(list) => Val::Arr(jaq_json::Rc::new(list.iter().map(to_val).collect())),
+ Datum::Map(dict) => Val::obj(
+ dict.iter()
+ .map(|(k, v)| (Val::utf8_str(k.as_bytes().to_vec()), to_val(v)))
+ .collect(),
+ ),
+ d => unreachable!("not a jsonb datum: {d:?}"),
+ })
}
+
+/// Packs a jaq value as a `jsonb` datum, or describes why it is not JSON.
+///
+/// `depth` counts the arrays and objects enclosing `val`. Invalid UTF-8 in
+/// strings is replaced, and among object keys that are equal after
+/// replacement the last one wins, matching how `jsonb` text parses.
+fn pack_val(packer: &mut RowPacker, val: &Val, depth: usize) -> Result<(), String> {
+ let nested = || {
+ let depth = depth + 1;
+ if depth >= MAX_OUTPUT_DEPTH {
+ Err(format!(
+ "output nests {MAX_OUTPUT_DEPTH} or more levels deep"
+ ))
+ } else {
+ Ok(depth)
+ }
+ };
+ mz_ore::stack::maybe_grow(|| {
+ match val {
+ Val::Null => packer.push(Datum::JsonNull),
+ Val::Bool(true) => packer.push(Datum::True),
+ Val::Bool(false) => packer.push(Datum::False),
+ Val::Num(Num::Float(f)) if !f.is_finite() => {
+ return Err(format!("output {} is not JSON", Num::Float(*f)));
+ }
+ Val::Num(n) => {
+ let n = strconv::parse_numeric(&n.to_string()).map_err(|e| e.to_string())?;
+ packer.push(Datum::Numeric(n));
+ }
+ Val::TStr(s) => packer.push(Datum::String(&String::from_utf8_lossy(s))),
+ Val::BStr(_) => return Err("output contains a byte string, which is not JSON".into()),
+ Val::Arr(vals) => {
+ let depth = nested()?;
+ packer.push_list_with(|packer| {
+ vals.iter().try_for_each(|v| pack_val(packer, v, depth))
+ })?;
+ }
+ Val::Obj(entries) => {
+ let depth = nested()?;
+ let mut entries = entries
+ .iter()
+ .map(|(k, v)| match k {
+ Val::TStr(k) => Ok((String::from_utf8_lossy(k), v)),
+ _ => Err("output contains an object key that is not a string"),
+ })
+ .collect::<Result<Vec<_>, _>>()?;
+ // Stable, so the last of several equal keys sorts last.
+ entries.sort_by(|(a, _), (b, _)| a.cmp(b));
+ packer.push_dict_with(|packer| {
+ for (i, (k, v)) in entries.iter().enumerate() {
+ if entries.get(i + 1).is_some_and(|(next, _)| next == k) {
+ continue;
+ }
+ packer.push(Datum::String(k));
+ pack_val(packer, v, depth)?;
+ }
+ Ok::<_, String>(())
+ })?;
+ }
+ }
+ Ok(())
+ })
+}
+
+#[cfg(test)]
+mod tests;
diff --git a/src/expr/src/scalar/func/jq/tests.rs b/src/expr/src/scalar/func/jq/tests.rs
new file mode 100644
index 0000000000..3644cb78ed
--- /dev/null
+++ b/src/expr/src/scalar/func/jq/tests.rs
@@ -0,0 +1,246 @@
+// Copyright Materialize, Inc. and contributors. All rights reserved.
+//
+// Use of this software is governed by the Business Source License
+// included in the LICENSE file.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0.
+
+use std::str::FromStr;
+
+use mz_repr::{SqlScalarType, arb_datum_for_scalar};
+use proptest::prelude::*;
+
+use super::*;
+
+fn run(input: &str, program: &str) -> Result<Option<String>, String> {
+ let input: Jsonb = input.parse().unwrap();
+ let program = JqProgram::new(program).map_err(|e| format!("jq: {e}"))?;
+ program
+ .eval(input.as_ref())
+ .map(|out| out.map(|out| out.to_string()))
+ .map_err(|e| e.to_string())
+}
+
+#[mz_ore::test]
+#[cfg_attr(miri, ignore)] // too slow
+fn jq_eval() {
+ assert_eq!(
+ run(r#"{"a": [1, 2.5, 3]}"#, ".a | map(. * 2)"),
+ Ok(Some("[2,5,6]".into()))
+ );
+ assert_eq!(
+ run(
+ r#"{"b": 2, "a": 1}"#,
+ r#"to_entries | map(.key) | join(",")"#
+ ),
+ Ok(Some(r#""a,b""#.into()))
+ );
+ assert_eq!(
+ run(r#"{"x": 123456789012345678901234567890.123}"#, ".x"),
+ Ok(Some("123456789012345678901234567890.123".into()))
+ );
+ assert_eq!(run("[1, 2, 3]", ".[] | select(. > 5)"), Ok(None));
+ assert_eq!(
+ run("[1, 2, 3]", "[.[] | select(. > 1)]"),
+ Ok(Some("[2,3]".into()))
+ );
+ // Logging filters are kept, as no-ops.
+ assert_eq!(run("[1, 2]", "debug"), Ok(Some("[1,2]".into())));
+ for (program, error) in [
+ (
+ ".[]",
+ "jq: program produced more than one value; collect them with [...]",
+ ),
+ (". +", "jq: expected term at end of program"),
+ ("now", "jq: undefined filter now/0"),
+ ("env", "jq: undefined filter env/0"),
+ (
+ r#"import "x" as x; ."#,
+ "jq: cannot import x: module loading not supported",
+ ),
+ (r#"error("boom")"#, r#"jq: "boom""#),
+ ("halt_error(3)", "jq: program halted with exit code 3"),
+ ] {
+ assert_eq!(run("[1, 2]", program), Err(error.into()), "{program}");
+ }
+}
+
+/// Converts input as JSON text, the reference for [`to_val`].
+fn to_val_via_text(input: JsonbRef<'_>) -> Val {
+ jaq_json::read::parse_single(input.to_string().as_bytes()).expect("jsonb text is JSON")
+}
+
+/// Converts output as JSON text, the reference for [`pack_val`].
+fn from_val_via_text(val: &Val) -> Result<Jsonb, String> {
+ Jsonb::from_str(&val.to_string()).map_err(|e| e.to_string())
+}
+
+fn from_val(val: &Val) -> Result<Jsonb, String> {
+ let mut row = Row::default();
+ pack_val(&mut row.packer(), val, 0)?;
+ Ok(Jsonb::from_row(row))
+}
+
+/// Asserts that direct conversion agrees with the text reference: equal
+/// `jsonb`, or an error from both.
+fn assert_from_val_agrees(val: &Val) -> Result<(), TestCaseError> {
+ match (from_val(val), from_val_via_text(val)) {
+ (Ok(direct), Ok(text)) => prop_assert_eq!(direct, text, "{:?}", val),
+ (Err(_), Err(_)) => {}
+ (direct, text) => prop_assert!(false, "{val:?}: direct {direct:?}, text {text:?}"),
+ }
+ Ok(())
+}
+
+fn arb_jsonb() -> impl Strategy<Value = Jsonb> {
+ arb_datum_for_scalar(SqlScalarType::Jsonb)
+ .prop_map(|d| Jsonb::from_row(Row::pack_slice(&[Datum::from(&d)])))
+}
+
+/// Numbers in the full `numeric` range, including negatives, which
+/// [`arb_jsonb`] does not generate.
+fn arb_numeric_jsonb() -> impl Strategy<Value = Jsonb> {
+ (any::<i128>(), -45i32..45).prop_filter_map("out of numeric range", |(m, e)| {
+ let n = strconv::parse_numeric(&format!("{m}e{e}")).ok()?;
+ Some(Jsonb::from_row(Row::pack_slice(&[Datum::Numeric(n)])))
+ })
+}
+
+/// A jaq value. Covers what jq programs produce but `jsonb` cannot hold.
+#[derive(Clone, Debug)]
+enum Tree {
+ Null,
+ Bool(bool),
+ Int(isize),
+ BigInt(String),
+ Float(f64),
+ Dec(String),
+ TStr(Vec<u8>),
+ BStr(Vec<u8>),
+ Arr(Vec<Tree>),
+ Obj(Vec<(Tree, Tree)>),
+}
+
+impl Tree {
+ fn to_val(&self) -> Val {
+ match self {
+ Tree::Null => Val::Null,
+ Tree::Bool(b) => Val::Bool(*b),
+ Tree::Int(i) => Val::Num(Num::Int(*i)),
+ Tree::BigInt(s) => Val::Num(Num::from_str_radix(s, 10).expect("integer")),
+ Tree::Float(f) => Val::Num(Num::Float(*f)),
+ Tree::Dec(s) => Val::Num(Num::Dec(jaq_json::Rc::new(s.clone()))),
+ Tree::TStr(b) => Val::utf8_str(b.clone()),
+ Tree::BStr(b) => Val::byte_str(b.clone()),
+ Tree::Arr(vs) => Val::Arr(jaq_json::Rc::new(vs.iter().map(Tree::to_val).collect())),
+ Tree::Obj(es) => Val::obj(es.iter().map(|(k, v)| (k.to_val(), v.to_val())).collect()),
+ }
+ }
+}
+
+fn arb_tree() -> impl Strategy<Value = Tree> {
+ let string = prop_oneof![
+ ".*".prop_map(String::into_bytes),
+ // Includes invalid UTF-8.
+ prop::collection::vec(any::<u8>(), 0..8),
+ ];
+ let leaf = prop_oneof![
+ Just(Tree::Null),
+ any::<bool>().prop_map(Tree::Bool),
+ any::<isize>().prop_map(Tree::Int),
+ "-?[1-9][0-9]{18,60}".prop_map(Tree::BigInt),
+ // Includes NaN and the infinities.
+ any::<f64>().prop_map(Tree::Float),
+ Just(Tree::Float(f64::NAN)),
+ Just(Tree::Float(-0.0)),
+ "-?(0|[1-9][0-9]{0,45})(\\.[0-9]{1,45})?([eE][+-]?[0-9]{1,3})?".prop_map(Tree::Dec),
+ string.clone().prop_map(Tree::TStr),
+ string.clone().prop_map(Tree::BStr),
+ ];
+ leaf.prop_recursive(4, 32, 8, move |inner| {
+ let key = prop_oneof![
+ 8 => string.clone().prop_map(Tree::TStr),
+ // Few, so that keys collide after invalid UTF-8 is replaced.
+ 4 => prop::collection::vec(prop_oneof![Just(b'a'), Just(0xff)], 0..3).prop_map(Tree::TStr),
+ 1 => inner.clone(),
+ ];
+ prop_oneof![
+ prop::collection::vec(inner.clone(), 0..8).prop_map(Tree::Arr),
+ prop::collection::vec((key, inner), 0..8).prop_map(Tree::Obj),
+ ]
+ })
+}
+
+proptest! {
+ #![proptest_config(ProptestConfig::with_cases(1000))]
+
+ #[mz_ore::test]
+ #[cfg_attr(miri, ignore)] // too slow
+ fn jq_to_val_matches_text(input in prop_oneof![arb_jsonb(), arb_numeric_jsonb()]) {
+ // `Val`'s `PartialEq` equates numbers across `Num` variants, which
+ // would hide `Dec` turning into `Float`.
+ let direct = to_val(input.as_ref().into_datum());
+ let text = to_val_via_text(input.as_ref());
+ prop_assert_eq!(format!("{direct:?}"), format!("{text:?}"));
+ }
+
+ #[mz_ore::test]
+ #[cfg_attr(miri, ignore)] // too slow
+ fn jq_val_roundtrip(input in prop_oneof![arb_jsonb(), arb_numeric_jsonb()]) {
+ let val = to_val(input.as_ref().into_datum());
+ prop_assert_eq!(from_val(&val), Ok(input));
+ }
+
+ #[mz_ore::test]
+ #[cfg_attr(miri, ignore)] // too slow
+ fn jq_from_val_matches_text(tree in arb_tree()) {
+ assert_from_val_agrees(&tree.to_val())?;
+ }
+
+ #[mz_ore::test]
+ #[cfg_attr(miri, ignore)] // too slow
+ fn jq_eval_matches_text(
+ input in prop_oneof![arb_jsonb(), arb_numeric_jsonb()],
+ program in prop::sample::select(vec![
+ ".",
+ "[..]",
+ "map_values(.)?",
+ "[paths]",
+ "to_entries? // . | from_entries? // .",
+ "walk(.)",
+ "tojson | fromjson",
+ "[.. | numbers | . * 1]",
+ ".. |= (if type == \"number\" then . / 3 else . end)",
+ "{b: 1, a: .}",
+ ]),
+ ) {
+ let program = JqProgram::new(program).expect("valid program");
+ let ctx = Ctx::<Data>::new(&program.filter.lut, Vars::new([]));
+ let input_val = to_val(input.as_ref().into_datum());
+ let mut outputs = program.filter.id.run((ctx, input_val));
+ if let Some(Ok(output)) = outputs.next() {
+ assert_from_val_agrees(&output)?;
+ }
+ }
+}
+
+#[mz_ore::test]
+fn jq_from_val_depth_matches_text() {
+ for depth in [MAX_OUTPUT_DEPTH - 1, MAX_OUTPUT_DEPTH, MAX_OUTPUT_DEPTH + 1] {
+ let mut arr = Val::Null;
+ let mut obj = Val::Null;
+ for _ in 0..depth {
+ arr = Val::Arr(jaq_json::Rc::new(vec![arr]));
+ obj = Val::obj([(Val::utf8_str(b"k".to_vec()), obj)].into_iter().collect());
+ }
+ for val in [arr, obj] {
+ assert_eq!(
+ from_val(&val).is_ok(),
+ from_val_via_text(&val).is_ok(),
+ "depth {depth}"
+ );
+ }
+ }
+}
antiguru
left a comment
There was a problem hiding this comment.
Posted by Claude Code on behalf of @antiguru.
Review findings, in addition to the conversion suggestion above. For a spike behind a default-off flag, the scope is right. The first two points should still be addressed or documented before the flag is enabled anywhere outside CI.
Constant folding runs jaq in environmentd
reduce/unary.rs folds every unary function over a literal except Panic. As a result, jq(<literal>, <literal>) evaluates in environmentd's optimizer, not on a replica. A local unit test confirms that def f: 1 + f; f overflows the stack, and the result is fatal runtime error: stack overflow, aborting (SIGABRT). In environmentd, SELECT jq('1', 'def f: 1 + f; f') would therefore take down the whole environment, not stall one worker. Loops that run inside jaq-core's tail-call loop (def_run in filter.rs) and programs like [range(1e12)] would hang or exhaust the optimizer's memory the same way. The NOTE in the module doc only mentions the worker. Options are to exclude Jq from constant folding, to evaluate it in environmentd under a budget, or at least to document that folding runs it there.
strptime depends on the host's tz database
jaq-std's strptime resolves %Q zone names through jiff. With the tzdb-zoneinfo feature, which is enabled in this build, jiff reads /usr/share/zoneinfo on Linux. The resulting array drops the offset, so successful lookups agree across machines. Whether a lookup succeeds depends on the host, though. Locally, "2024-07-01 12:00 Foo/Bar" | strptime("%Y-%m-%d %H:%M %Q") fails with failed to find time zone, and a zone missing from one image's tzdata would fail there while succeeding on another. That breaks the guarantee that every replica computes the same answer. Either withhold strptime or switch jiff to a bundled tz database, and add an slt case with a zone name.
Catalog compatibility
Items are re-planned from their create_sql at boot, and the program compiles during planning. If a jaq upgrade drops or renames a filter, or if a name is later added to WITHHELD, an existing materialized view using it no longer plans and the catalog fails to open. That makes WITHHELD and the jaq version part of the catalog compatibility surface, which is worth stating on WITHHELD.
Smaller points
- The text-path error
output {output} is not JSONembeds the whole output value, which can be arbitrarily large and is persisted for materialized views. The conversion suggestion above removes this. - Only
nowandenvhave tests among the withheld filters. A table-driven slt over all ofWITHHELD, plus$ENV, would catch a jaq upgrade that re-exposes one. - "program produced more than one value; collect them with [...]" puts a hint into the primary error message.
InvalidParameterValuehas no hint slot, so this needs a dedicatedEvalErrorvariant or a shorter message. syntax_errorcomputesoffseteven whenatis empty, so the computation can move into theelsebranch.JqProgram::sourcehas no doc comment.- Redacted
EXPLAINshows the program text. This matchesIsRegexpMatch, but a program may embed literal user data.
A spike of user-defined logic with a minimal surface: `jq(jsonb, text)` applies a literal jq program, run on jaq, to a jsonb value. The program is compiled at planning time and carried in the plan as its source text. Filters that depend on the clock, environment, or extra input are withheld so replicas agree. jaq has no evaluation budget, so the function stays behind a default-off feature flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moves the function to `mz_unsafe.jq`, gated by `unsafe_enable_unsafe_functions`, and drops the `enable_jq` flag. Constant folding no longer evaluates `jq`, so programs never run in environmentd. Withholds `strptime`, whose zone lookups depend on the host. Converts between jsonb datums and jaq values directly instead of through JSON text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reverts the move to `mz_unsafe`, which taints catalogs that use it, in favor of the `enable_jq` feature flag. Output now goes through the canonical `jsonb` text parser instead of a hand-written packer, since jq values are a superset of JSON and the parser is what guarantees a valid `jsonb`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a2da407 to
c04c003
Compare
QA LLM Review1. HIGH -- jq programs still run inside environmentd, so any user can abort it or wedge the coordinator
Skipping DetailsEach of these was reproduced on the PR's CI image:
From reading the code, the same applies to EXECUTE parameters ( Fix: listing the call sites cannot close this, because any |
The `jq` function (#39838) is now enabled in CI, and SQLsmith fails both nightly SQLsmith jobs on errors it creates itself. It passes arbitrary strings as jq programs, which fail to parse or evaluate (`jq: ...`). It also passes non-literal programs, which the planner rejects. This adds both messages to `known_errors`, which parallel workload also uses. Seen in nightly 18509 and 18510, and in release qualification 1400. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A spike of user-defined logic with the smallest implementation and surface we could find: one SQL function,
jq(jsonb, text) -> jsonb, which applies a jq program to ajsonbvalue. It is behind theenable_jqfeature flag, off by default and on in CI. The point is to support more testing, not to ship it: the function may change or go away.The aim is to see how much of the user-defined-function ask a small, familiar language covers before taking on an ABI, a module catalog object, or DDL. There is none of that here.
How it works
jsonbto jaq values. Output is printed and parsed by the canonicaljsonbtext parser, because jq values are a superset of JSON (byte strings, non-string keys, NaN, invalid UTF-8) and the parser is what guarantees a validjsonb.NULL. More than one output is an error.now,env,localtime,strptime,input, ...) are removed, so every replica computes the same answer.jqwhile constant folding, so programs only run on clusters, never inenvironmentd.Nearly all of the code is the new files under
src/expr/src/scalar/func/jq. The rest is registration, the feature flag, a new OID, the constant-folding exclusion, and regenerated snapshots, which add one unary function,jq, whose payload is a string.Cost
Compiling a program takes about 0.2 ms, once per dataflow per worker. Evaluation per row, natively in release mode: about 2 µs for a
mapover a three-element array, 20 µs over 100 elements.Open questions
Tests
test/sqllogictest/jq.sltcovers results, NULLs, errors, the removed filters, literal-only rejection, that literal calls are not folded, a materialized view, and a longer program (Jaro-Winkler similarity).jq/tests.rscover evaluation, error messages, every removed filter, non-JSON output, and a round-trip property test of thejsonbconversion.Release notes
None while the feature is behind a default-off flag.
🤖 Generated with Claude Code