Skip to content

Commit

Permalink
environmentd: convert json numbers to strings
Browse files Browse the repository at this point in the history
This used to happen but regressed at some point.

Fixes #21659
  • Loading branch information
maddyblue committed Sep 10, 2023
1 parent 838bc69 commit e55f9f2
Show file tree
Hide file tree
Showing 10 changed files with 189 additions and 155 deletions.
1 change: 1 addition & 0 deletions doc/user/content/integrations/http-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ ultimately rolled back because of an error in a later part of the transaction.
You must parse the results to understand which statements ultimately reflect
the resultant state.

Numeric results are converted to strings to avoid possible JavaScript number inaccuracy.
Column descriptions contain the name, oid, data type size and type modifier of a returned column.

#### TypeScript definition
Expand Down
1 change: 1 addition & 0 deletions doc/user/content/integrations/websocket-api.md
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,7 @@ The inner object's various `type_X` fields are lower-level details that can be u
A single row result.
Will only occur after a `Rows` message.
The payload is an array of JSON values corresponding to the columns from the `Rows` message.
Numeric results are converted to strings to avoid possible JavaScript number inaccuracy.

#### `ParameterStatus`

Expand Down
4 changes: 4 additions & 0 deletions redshift.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
mjibson-redshift
username: admin
password: AENSTUHNTHUEuoentuhoenstuh342342
mjibson-fivetran-2.cdqsoxo4spgz.us-east-1.redshift.amazonaws.com:5439/dev
1 change: 1 addition & 0 deletions run
11 changes: 7 additions & 4 deletions src/environmentd/src/http/sql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use mz_adapter::{
SessionClient, Severity,
};
use mz_interchange::encode::TypedDatum;
use mz_interchange::json::ToJson;
use mz_interchange::json::{JsonNumberPolicy, ToJson};
use mz_ore::result::ResultExt;
use mz_repr::{Datum, RelationDesc, RowArena};
use mz_sql::ast::display::AstDisplay;
Expand Down Expand Up @@ -665,7 +665,10 @@ impl ResultSender for WebSocket {
datums
.iter()
.enumerate()
.map(|(i, d)| TypedDatum::new(*d, &types[i]).json())
.map(|(i, d)| {
TypedDatum::new(*d, &types[i])
.json(&JsonNumberPolicy::ConvertNumberToString)
})
.collect(),
),
)
Expand Down Expand Up @@ -1065,7 +1068,7 @@ async fn execute_stmt<S: ResultSender>(
let types = &desc.typ().column_types;
for row in rows {
let datums = datum_vec.borrow_with(&row);
sql_rows.push(datums.iter().enumerate().map(|(i, d)| TypedDatum::new(*d, &types[i]).json()).collect());
sql_rows.push(datums.iter().enumerate().map(|(i, d)| TypedDatum::new(*d, &types[i]).json(&JsonNumberPolicy::ConvertNumberToString)).collect());
}
let tag = format!("SELECT {}", sql_rows.len());
SqlResult::rows(client, tag, sql_rows, desc).into()
Expand All @@ -1077,7 +1080,7 @@ async fn execute_stmt<S: ResultSender>(
let types = &desc.typ().column_types;
for row in rows {
let datums = datum_vec.borrow_with(&row);
sql_rows.push(datums.iter().enumerate().map(|(i, d)| TypedDatum::new(*d, &types[i]).json()).collect());
sql_rows.push(datums.iter().enumerate().map(|(i, d)| TypedDatum::new(*d, &types[i]).json(&JsonNumberPolicy::ConvertNumberToString)).collect());
}
let tag = format!("SELECT {}", sql_rows.len());
SqlResult::rows(client, tag, sql_rows, desc).into()
Expand Down
6 changes: 3 additions & 3 deletions src/environmentd/tests/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1823,7 +1823,7 @@ fn test_max_connections_on_all_interfaces() {
assert_eq!(status, StatusCode::OK);
let result: HttpResponse<HttpRows> = res.json().unwrap();
assert_eq!(result.results.len(), 1);
assert_eq!(result.results[0].rows, vec![vec![1]]);
assert_eq!(result.results[0].rows, vec![vec!["1"]]);
Ok(())
}).unwrap();

Expand All @@ -1847,7 +1847,7 @@ fn test_max_connections_on_all_interfaces() {
);
assert_eq!(
ws.read().unwrap(),
Message::Text("{\"type\":\"Row\",\"payload\":[1]}".to_string())
Message::Text("{\"type\":\"Row\",\"payload\":[\"1\"]}".to_string())
);
tracing::info!("data: {:?}", ws.read().unwrap());
}
Expand Down Expand Up @@ -2415,7 +2415,7 @@ fn test_github_20262() {
r#"{"type":"ReadyForQuery","payload":"I"}"#,
r#"{"type":"CommandStarting","payload":{"has_rows":true,"is_streaming":false}}"#,
r#"{"type":"Rows","payload":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]}}"#,
r#"{"type":"Row","payload":[1]}"#,
r#"{"type":"Row","payload":["1"]}"#,
r#"{"type":"CommandComplete","payload":"SELECT 1"}"#,
r#"{"type":"ReadyForQuery","payload":"I"}"#,
]);
Expand Down
50 changes: 25 additions & 25 deletions src/environmentd/tests/testdata/http/post
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,28 @@ http
{"query":"select 1+2 as col"}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[3]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["3"]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

# Multiple queries are ok.
http
{"query":"select 1; select 2"}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"tag":"SELECT 1","rows":[[2]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"tag":"SELECT 1","rows":[["2"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

# Arrays + lists work
http
{"query":"select array[1], list[2]"}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[[1],[2]]],"desc":{"columns":[{"name":"array","type_oid":1007,"type_len":-1,"type_mod":-1},{"name":"list","type_oid":16384,"type_len":-1,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[[["1"],["2"]]],"desc":{"columns":[{"name":"array","type_oid":1007,"type_len":-1,"type_mod":-1},{"name":"list","type_oid":16384,"type_len":-1,"type_mod":-1}]},"notices":[]}]}

# Succeeding and failing queries can mix and match.
http
{"query":"select 1; select * from noexist;"}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"error":{"message":"unknown catalog item 'noexist'","code":"XX000"},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"error":{"message":"unknown catalog item 'noexist'","code":"XX000"},"notices":[]}]}

# CREATEs should work when provided alone.
http
Expand Down Expand Up @@ -80,7 +80,7 @@ http
{"query":"select * from t;"}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

http
{"query":"delete from t"}
Expand All @@ -99,19 +99,19 @@ http
{"query":"begin; select 1; commit"}
----
200 OK
{"results":[{"ok":"BEGIN","notices":[]},{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"COMMIT","notices":[]}]}
{"results":[{"ok":"BEGIN","notices":[]},{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"COMMIT","notices":[]}]}

http
{"query":"begin; select 1; commit; select 2;"}
----
200 OK
{"results":[{"ok":"BEGIN","notices":[]},{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"COMMIT","notices":[]},{"tag":"SELECT 1","rows":[[2]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"ok":"BEGIN","notices":[]},{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"COMMIT","notices":[]},{"tag":"SELECT 1","rows":[["2"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

http
{"query":"select 1; begin; select 2; commit;"}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"BEGIN","notices":[]},{"tag":"SELECT 1","rows":[[2]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"COMMIT","notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"BEGIN","notices":[]},{"tag":"SELECT 1","rows":[["2"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"COMMIT","notices":[]}]}

http
{"query":"begin; select 1/0; commit; select 2;"}
Expand All @@ -123,7 +123,7 @@ http
{"query":"begin; select 1; commit; select 1/0;"}
----
200 OK
{"results":[{"ok":"BEGIN","notices":[]},{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"COMMIT","notices":[]},{"error":{"message":"division by zero","code":"XX000"},"notices":[]}]}
{"results":[{"ok":"BEGIN","notices":[]},{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"COMMIT","notices":[]},{"error":{"message":"division by zero","code":"XX000"},"notices":[]}]}

http
{"query":"select 1/0; begin; select 2; commit;"}
Expand All @@ -135,7 +135,7 @@ http
{"query":"select 1; begin; select 1/0; commit;"}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"BEGIN","notices":[]},{"error":{"message":"division by zero","code":"XX000"},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"?column?","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"ok":"BEGIN","notices":[]},{"error":{"message":"division by zero","code":"XX000"},"notices":[]}]}

# Txns w/ writes

Expand Down Expand Up @@ -164,7 +164,7 @@ http
{"query":"select * from t;"}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

http
{"query":"delete from t;"}
Expand All @@ -188,7 +188,7 @@ http
{"query":"select * from t;"}
----
200 OK
{"results":[{"tag":"SELECT 3","rows":[[1],[2],[3]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 3","rows":[["1"],["2"],["3"]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

http
{"query":"delete from t;"}
Expand Down Expand Up @@ -258,47 +258,47 @@ http
{"queries":[{"query":"select $1+$2::int as col","params":["1","2"]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[3]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["3"]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

# Parameters can be present and empty
http
{"queries":[{"query":"select 3 as col","params":[]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[3]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["3"]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

# Multiple statements
http
{"queries":[{"query":"select 1 as col","params":[]},{"query":"select $1+$2::int as col","params":["1","2"]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"tag":"SELECT 1","rows":[[3]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"tag":"SELECT 1","rows":[["3"]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

http
{"queries":[{"query":"select $1+$2::int as col","params":["1","2"]},{"query":"select 1 as col","params":[]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[3]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["3"]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

http
{"queries":[{"query":"select $1+$2::int as col","params":["1","2"]},{"query":"select $1*$2::int as col","params":["2","3"]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[3]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"tag":"SELECT 1","rows":[[6]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["3"]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]},{"tag":"SELECT 1","rows":[["6"]],"desc":{"columns":[{"name":"col","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

# Quotes escaped
http
{"queries":[{"query":"select length($1), length($2)","params":["abc","'abc'"]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[3,5]],"desc":{"columns":[{"name":"length","type_oid":23,"type_len":4,"type_mod":-1},{"name":"length","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["3","5"]],"desc":{"columns":[{"name":"length","type_oid":23,"type_len":4,"type_mod":-1},{"name":"length","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

# All parameters values treated as strings
http
{"queries":[{"query":"select length($1), length($2)","params":["sum(a)","SELECT * FROM t;"]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[6,16]],"desc":{"columns":[{"name":"length","type_oid":23,"type_len":4,"type_mod":-1},{"name":"length","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["6","16"]],"desc":{"columns":[{"name":"length","type_oid":23,"type_len":4,"type_mod":-1},{"name":"length","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

# Too many parameters
http
Expand Down Expand Up @@ -360,7 +360,7 @@ http
{"queries":[{"query":"select count(*) from t","params":[]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[0]],"desc":{"columns":[{"name":"count","type_oid":20,"type_len":8,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["0"]],"desc":{"columns":[{"name":"count","type_oid":20,"type_len":8,"type_mod":-1}]},"notices":[]}]}

# Rolledback
http
Expand All @@ -374,7 +374,7 @@ http
{"queries":[{"query":"select count(*) from t","params":[]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[0]],"desc":{"columns":[{"name":"count","type_oid":20,"type_len":8,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["0"]],"desc":{"columns":[{"name":"count","type_oid":20,"type_len":8,"type_mod":-1}]},"notices":[]}]}

# Implicit txn
http
Expand All @@ -388,7 +388,7 @@ http
{"queries":[{"query":"select count(*) from t","params":[]},{"query":"delete from t","params":[]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[1]],"desc":{"columns":[{"name":"count","type_oid":20,"type_len":8,"type_mod":-1}]},"notices":[]},{"ok":"DELETE 1","notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["1"]],"desc":{"columns":[{"name":"count","type_oid":20,"type_len":8,"type_mod":-1}]},"notices":[]},{"ok":"DELETE 1","notices":[]}]}

# Errors prevent commit + further execution
http
Expand All @@ -402,7 +402,7 @@ http
{"queries":[{"query":"select count(*) from t","params":[]}]}
----
200 OK
{"results":[{"tag":"SELECT 1","rows":[[0]],"desc":{"columns":[{"name":"count","type_oid":20,"type_len":8,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 1","rows":[["0"]],"desc":{"columns":[{"name":"count","type_oid":20,"type_len":8,"type_mod":-1}]},"notices":[]}]}

# Requires explicit commit in explicit txn
http
Expand All @@ -428,7 +428,7 @@ http
{"queries":[{"query":"select * from t","params":[]}]}
----
200 OK
{"results":[{"tag":"SELECT 3","rows":[[1],[2],[3]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 3","rows":[["1"],["2"],["3"]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

# The first statement (insert) succeeds and commits because it's an implicit transaction.
http
Expand All @@ -442,7 +442,7 @@ http
{"queries":[{"query":"select * from t","params":[]}]}
----
200 OK
{"results":[{"tag":"SELECT 4","rows":[[1],[2],[3],[4]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}
{"results":[{"tag":"SELECT 4","rows":[["1"],["2"],["3"],["4"]],"desc":{"columns":[{"name":"a","type_oid":23,"type_len":4,"type_mod":-1}]},"notices":[]}]}

http
{"queries":[{"query":"subscribe (select * from t)","params":[]}]}
Expand Down
Loading

0 comments on commit e55f9f2

Please sign in to comment.