Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: remove json_typeof #3316

Merged
merged 2 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- #3061, Apply all function settings as transaction-scoped settings - @taimoorzaeem
- #3171, #3046, Log schema cache stats to stderr - @steve-chavez
- #3210, Dump schema cache through admin API - @taimoorzaeem
- #2676, Performance improvement on bulk json inserts, around 10% increase on requests per second by removing `json_typeof` from write queries - @steve-chavez

### Fixed

Expand Down
31 changes: 17 additions & 14 deletions src/PostgREST/Query/SqlFragment.hs
Original file line number Diff line number Diff line change
Expand Up @@ -300,36 +300,39 @@ fromJsonBodyF :: Maybe LBS.ByteString -> [CoercibleField] -> Bool -> Bool -> Boo
fromJsonBodyF body fields includeSelect includeLimitOne includeDefaults =
(if includeSelect then "SELECT " <> namedCols <> " " else mempty) <>
"FROM (SELECT " <> jsonPlaceHolder <> " AS json_data) pgrst_payload, " <>
-- convert a json object into a json array, this way we can use json_to_recordset for all json payloads
-- Otherwise we'd have to use json_to_record for json objects and json_to_recordset for json arrays
-- We do this in SQL to avoid processing the JSON in application code
"LATERAL (SELECT CASE WHEN " <> jsonTypeofF <> "(pgrst_payload.json_data) = 'array' THEN pgrst_payload.json_data ELSE " <> jsonBuildArrayF <> "(pgrst_payload.json_data) END AS val) pgrst_uniform_json, " <>
(if includeDefaults
then "LATERAL (SELECT jsonb_agg(jsonb_build_object(" <> defsJsonb <> ") || elem) AS val from jsonb_array_elements(pgrst_uniform_json.val) elem) pgrst_json_defs, "
then if isJsonObject
then "LATERAL (SELECT " <> defsJsonb <> " || pgrst_payload.json_data AS val) pgrst_json_defs, "
else "LATERAL (SELECT jsonb_agg(" <> defsJsonb <> " || elem) AS val from jsonb_array_elements(pgrst_payload.json_data) elem) pgrst_json_defs, "
else mempty) <>
"LATERAL (SELECT " <> parsedCols <> " FROM " <>
(if null fields
-- When we are inserting no columns (e.g. using default values), we can't use our ordinary `json_to_recordset`
-- because it can't extract records with no columns (there's no valid syntax for the `AS (colName colType,...)`
-- part). But we still need to ensure as many rows are created as there are array elements.
then SQL.sql $ jsonArrayElementsF <> "(" <> finalBodyF <> ") _ "
(if null fields -- when json keys are empty, e.g. when payload is `{}` or `[{}, {}]`
then SQL.sql $
if isJsonObject
then "(values(1)) _ " -- only 1 row for an empty json object '{}'
else jsonArrayElementsF <> "(" <> finalBodyF <> ") _ " -- extract rows of a json array of empty objects `[{}, {}]`
else jsonToRecordsetF <> "(" <> SQL.sql finalBodyF <> ") AS _(" <> typedCols <> ") " <> if includeLimitOne then "LIMIT 1" else mempty
) <>
") pgrst_body "
where
namedCols = intercalateSnippet ", " $ fromQi . QualifiedIdentifier "pgrst_body" . cfName <$> fields
parsedCols = intercalateSnippet ", " $ pgFmtCoerceNamed <$> fields
typedCols = intercalateSnippet ", " $ pgFmtIdent . cfName <> const " " <> SQL.sql . encodeUtf8 . cfIRType <$> fields
defsJsonb = SQL.sql $ BS.intercalate "," fieldsWDefaults
defsJsonb = SQL.sql $ "jsonb_build_object(" <> BS.intercalate "," fieldsWDefaults <> ")"
fieldsWDefaults = mapMaybe (\case
CoercibleField{cfName=nam, cfDefault=Just def} -> Just $ encodeUtf8 (pgFmtLit nam <> ", " <> def)
CoercibleField{cfDefault=Nothing} -> Nothing
) fields
(finalBodyF, jsonTypeofF, jsonBuildArrayF, jsonArrayElementsF, jsonToRecordsetF) =
(finalBodyF, jsonArrayElementsF, jsonToRecordsetF) =
if includeDefaults
then ("pgrst_json_defs.val", "jsonb_typeof", "jsonb_build_array", "jsonb_array_elements", "jsonb_to_recordset")
else ("pgrst_uniform_json.val", "json_typeof", "json_build_array", "json_array_elements", "json_to_recordset")
then ("pgrst_json_defs.val", "jsonb_array_elements", if isJsonObject then "jsonb_to_record" else "jsonb_to_recordset")
else ("pgrst_payload.json_data", "json_array_elements", if isJsonObject then "json_to_record" else "json_to_recordset")
jsonPlaceHolder = SQL.encoderAndParam (HE.nullable $ if includeDefaults then HE.jsonbLazyBytes else HE.jsonLazyBytes) body
isJsonObject = -- light validation as pg's json_to_record(set) already validates that the body is valid JSON. We just need to know whether the body looks like an object or not.
let
insignificantWhitespace = [32,9,10,13] --" \t\n\r" [32,9,10,13] https://datatracker.ietf.org/doc/html/rfc8259#section-2
in
LBS.take 1 (LBS.dropWhile (`elem` insignificantWhitespace) (fromMaybe mempty body)) == "{"

pgFmtOrderTerm :: QualifiedIdentifier -> CoercibleOrderTerm -> SQL.Snippet
pgFmtOrderTerm qi ot =
Expand Down
16 changes: 16 additions & 0 deletions test/pgbench/2676/new.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
WITH pgrst_source AS (
INSERT INTO "test"."complex_items"("arr_data", "field-with_sep", "id", "name")
SELECT "pgrst_body"."arr_data", "pgrst_body"."field-with_sep", "pgrst_body"."id", "pgrst_body"."name"
FROM (SELECT '[{"id": 4, "name": "Vier"}, {"id": 5, "name": "Funf", "arr_data": null}, {"id": 6, "name": "Sechs", "arr_data": [1, 2, 3], "field-with_sep": 6}]'::json AS json_data) pgrst_payload,
LATERAL (SELECT "arr_data", "field-with_sep", "id", "name" FROM json_to_recordset(pgrst_payload.json_data) AS _("arr_data" integer[], "field-with_sep" integer, "id" bigint, "name" text) ) pgrst_body
RETURNING "test"."complex_items".*
)
SELECT
'' AS total_result_set,
pg_catalog.count(_postgrest_t) AS page_total,
array[]::text[] AS header,
coalesce(json_agg(_postgrest_t), '[]') AS body,
nullif(current_setting('response.headers', true), '') AS response_headers,
nullif(current_setting('response.status', true), '') AS response_status,
'' AS response_inserted
FROM (SELECT "complex_items".* FROM "pgrst_source" AS "complex_items") _postgrest_t;
17 changes: 17 additions & 0 deletions test/pgbench/2676/old.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
WITH pgrst_source AS (
INSERT INTO "test"."complex_items"("arr_data", "field-with_sep", "id", "name")
SELECT "pgrst_body"."arr_data", "pgrst_body"."field-with_sep", "pgrst_body"."id", "pgrst_body"."name"
FROM (SELECT '[{"id": 4, "name": "Vier"}, {"id": 5, "name": "Funf", "arr_data": null}, {"id": 6, "name": "Sechs", "arr_data": [1, 2, 3], "field-with_sep": 6}]'::json AS json_data) pgrst_payload,
LATERAL (SELECT CASE WHEN json_typeof(pgrst_payload.json_data) = 'array' THEN pgrst_payload.json_data ELSE json_build_array(pgrst_payload.json_data) END AS val) pgrst_uniform_json,
LATERAL (SELECT "arr_data", "field-with_sep", "id", "name" FROM json_to_recordset(pgrst_uniform_json.val) AS _("arr_data" integer[], "field-with_sep" integer, "id" bigint, "name" text) ) pgrst_body
RETURNING "test"."complex_items".*
)
SELECT
'' AS total_result_set,
pg_catalog.count(_postgrest_t) AS page_total,
array[]::text[] AS header,
coalesce(json_agg(_postgrest_t), '[]') AS body,
nullif(current_setting('response.headers', true), '') AS response_headers,
nullif(current_setting('response.status', true), '') AS response_status,
'' AS response_inserted
FROM (SELECT "complex_items".* FROM "pgrst_source" AS "complex_items") _postgrest_t;
4 changes: 2 additions & 2 deletions test/pgbench/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
Can be used as:

```
postgrest-with-postgresql-15 -f test/pgbench/fixtures.sql pgbench -n -T 10 -f test/pgbench/1567/old.sql
postgrest-with-postgresql-15 -f test/pgbench/fixtures.sql pgbench -U postgres -n -T 10 -f test/pgbench/1567/old.sql

postgrest-with-postgresql-15 -f test/pgbench/fixtures.sql pgbench -n -T 10 -f test/pgbench/1567/new.sql
postgrest-with-postgresql-15 -f test/pgbench/fixtures.sql pgbench -U postgres -n -T 10 -f test/pgbench/1567/new.sql
```

## Directory structure
Expand Down
4 changes: 2 additions & 2 deletions test/spec/Feature/Query/PlanSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ spec actualPgVersion = do
liftIO $ do
resHeaders `shouldSatisfy` elem ("Content-Type", "application/vnd.pgrst.plan+json; for=\"application/json\"; charset=utf-8")
resStatus `shouldBe` Status { statusCode = 200, statusMessage="OK" }
totalCost `shouldBe` 3.27
totalCost `shouldBe` 0.06

it "outputs the total cost for an update" $ do
r <- request methodPatch "/projects?id=eq.3"
Expand All @@ -165,7 +165,7 @@ spec actualPgVersion = do
liftIO $ do
resHeaders `shouldSatisfy` elem ("Content-Type", "application/vnd.pgrst.plan+json; for=\"application/json\"; charset=utf-8")
resStatus `shouldBe` Status { statusCode = 200, statusMessage="OK" }
totalCost `shouldBe` 12.45
totalCost `shouldBe` 8.23

it "outputs the total cost for a delete" $ do
r <- request methodDelete "/projects?id=in.(1,2,3)"
Expand Down
Loading