Skip to content

Commit

Permalink
Fix case for array of jsons (#5523)
Browse files Browse the repository at this point in the history
## Problem

Currently proxy doesn't handle array of json parameters correctly.

## Summary of changes

Added one more level of quotes escaping for the array of jsons case.
Resolves: #5515
  • Loading branch information
khanova authored Oct 12, 2023
1 parent dbb21d6 commit 21deb81
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 3 deletions.
31 changes: 29 additions & 2 deletions proxy/src/http/sql_over_http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ fn json_array_to_pg_array(value: &Value) -> Result<Option<String>, serde_json::E
// convert to text with escaping
Value::Bool(_) => serde_json::to_string(value).map(Some),
Value::Number(_) => serde_json::to_string(value).map(Some),
Value::Object(_) => serde_json::to_string(value).map(Some),

// here string needs to be escaped, as it is part of the array
Value::Object(_) => json_array_to_pg_array(&Value::String(serde_json::to_string(value)?)),
Value::String(_) => serde_json::to_string(value).map(Some),

// recurse into array
Expand Down Expand Up @@ -613,7 +613,7 @@ fn _pg_array_parse(
}
}
}
'}' => {
'}' if !quote => {
level -= 1;
if level == 0 {
push_checked(&mut entry, &mut entries, elem_type)?;
Expand Down Expand Up @@ -697,6 +697,14 @@ mod tests {
"{{true,false},{NULL,42},{\"foo\",\"bar\\\"-\\\\\"}}".to_owned()
)]
);
// array of objects
let json = r#"[{"foo": 1},{"bar": 2}]"#;
let json: Value = serde_json::from_str(json).unwrap();
let pg_params = json_to_pg_text(vec![json]).unwrap();
assert_eq!(
pg_params,
vec![Some(r#"{"{\"foo\":1}","{\"bar\":2}"}"#.to_owned())]
);
}

#[test]
Expand Down Expand Up @@ -824,4 +832,23 @@ mod tests {
json!([[[1, 2, 3], [4, 5, 6]]])
);
}
#[test]
fn test_pg_array_parse_json() {
fn pt(pg_arr: &str) -> Value {
pg_array_parse(pg_arr, &Type::JSONB).unwrap()
}
assert_eq!(pt(r#"{"{}"}"#), json!([{}]));
assert_eq!(
pt(r#"{"{\"foo\": 1, \"bar\": 2}"}"#),
json!([{"foo": 1, "bar": 2}])
);
assert_eq!(
pt(r#"{"{\"foo\": 1}", "{\"bar\": 2}"}"#),
json!([{"foo": 1}, {"bar": 2}])
);
assert_eq!(
pt(r#"{{"{\"foo\": 1}", "{\"bar\": 2}"}}"#),
json!([[{"foo": 1}, {"bar": 2}]])
);
}
}
8 changes: 7 additions & 1 deletion test_runner/regress/test_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ def q(sql: str, params: Optional[List[Any]] = None) -> Any:
headers={"Content-Type": "application/sql", "Neon-Connection-String": connstr},
verify=str(static_proxy.test_output_dir / "proxy.crt"),
)
assert response.status_code == 200
assert response.status_code == 200, response.text
return response.json()

rows = q("select 42 as answer")["rows"]
Expand All @@ -206,6 +206,12 @@ def q(sql: str, params: Optional[List[Any]] = None) -> Any:
rows = q("select $1::json->'a' as answer", [{"a": {"b": 42}}])["rows"]
assert rows == [{"answer": {"b": 42}}]

rows = q("select $1::jsonb[] as answer", [[{}]])["rows"]
assert rows == [{"answer": [{}]}]

rows = q("select $1::jsonb[] as answer", [[{"foo": 1}, {"bar": 2}]])["rows"]
assert rows == [{"answer": [{"foo": 1}, {"bar": 2}]}]

rows = q("select * from pg_class limit 1")["rows"]
assert len(rows) == 1

Expand Down

1 comment on commit 21deb81

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2348 tests run: 2226 passed, 0 failed, 122 skipped (full report)


Flaky tests (1)

Postgres 16

  • test_pageserver_recovery: debug

Code coverage (full report)

  • functions: 52.4% (8174 of 15604 functions)
  • lines: 81.1% (47788 of 58951 lines)

The comment gets automatically updated with the latest test results
21deb81 at 2023-10-12T13:16:40.149Z :recycle:

Please sign in to comment.