Repository navigation
Dispatch plan reuse and JSON model spellings (0.1.1) - #8
Merged
Merged
Conversation
streamed_list_objects passed the search straight to unnest(). The planner's row estimate for unnest() evaluates a stable argument while the statement is planned, so every call ran the whole search twice: once to count rows for the planner, once for the answer. The search is now held in a PL/pgSQL variable and unnest() reads the variable. TestStreamedListObjectsSearchesOnce counts fga._list_objects calls per streamed call; with the old body it counts 2.
list_objects dispatches to a generated __objects function through a dynamic statement, which PostgreSQL plans on every execution. The generated function carries no SET clause so that callers can inline it, and dispatch inlined it too: its whole body was planned on every call. On a wide model (a four-level hierarchy, 5,000 leaves) planning took 10.8 ms against 0.85 ms of execution. Asking for the rows WITH ORDINALITY keeps the function out of the dynamic plan, since the planner never inlines a set-returning function asked for its row numbers. It then runs as a function and its own plan is cached for the session. The same list call went from 1.65 ms to 0.18 ms. Callers that query the generated function directly still inline it; only dispatch changes. TestCompiledDispatchCallsTheFunction counts the generated function's calls through both entry points; with the old dispatch it counts 0.
Upstream's HTTP API decodes a model with protojson, which takes a field under its proto name and under its JSON name. Three model fields are named differently the two ways: the rewrite's computedUserset and tupleToUserset, and the computedUserset inside a tupleToUserset. The engine stored a camelCase model as written and the resolver, which reads only snake_case, answered false where upstream answers true. Tools that convert the DSL to JSON, such as the syntax transformer, emit the camelCase form. The model write now renames those three fields to snake_case before storing, and refuses a node holding both spellings of one, as protojson refuses a duplicate field. No other key is renamed: every other model field has one name, and upstream discards the camelCase form of it as an unknown key, so renaming it would accept a model upstream reads differently. The field list comes from the descriptors of openfga/api at the pinned version. TestModelJSONSpellings sends the same JSON to the oracle's HTTP API and to the engine and compares the outcome, which is the only place the spelling exists (the gRPC client sends protos); TestCamelCaseModelAnswers compares check answers through every rewrite operator. Without the rename the camelCase checks answer false and the duplicate-field models are accepted.
A condition parameter's type was stored as written and read at evaluation, where a missing or unrecognised type_name fell back to TYPE_NAME_ANY. Upstream decodes it at model write and refuses what it cannot use. The difference shows up with camelCase: protojson knows these fields only as type_name and generic_types, so upstream discards a typeName and refuses the model for having no type, while the engine accepted it and evaluated the parameter as any value. The enum given as a number, which protojson accepts, went the other way: upstream reads 3 as a string, the engine read an unknown name and accepted any value. The model write now decodes each parameter as upstream does: a type by name or by enum number, LIST and MAP with exactly one generic type and every other type with none, and stores the type by name, the only form evaluation reads. Refusals carry upstream's codes, 2000 for a shape protojson cannot decode and 2056 with upstream's message for a type it cannot use. Every case in TestConditionParamTypes was measured against the oracle's HTTP API; without the decoder the engine accepted all twenty that upstream refuses, and answered false where upstream refuses an int given for a parameter typed 3.
Records what plan reuse saves a dispatched list call on the tenant 100k fixture, alternating the two installs in one session, and what the streamed function's double search cost and when: counted, the first five calls in a session searched twice before PostgreSQL's plan cache moved to a generic plan, so warm medians hide it while a short-lived connection paid it on every call.
The release fixes how dispatch and the streamed list plan their work and closes two conformance gaps in model writes: the camelCase rewrite keys upstream accepts, and condition parameter types decoded as upstream decodes them. No public function is added or removed, so it is a patch. A model that names a parameter type upstream refuses is now refused too; one written by an earlier version keeps working until it is rewritten.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release 0.1.1: dispatch reuses the generated plan, and model writes accept
and refuse what upstream's JSON API accepts and refuses.
What changes
Dispatch reuses the generated plan.
fga.list_objectsandfga.streamed_list_objectsreach a compiled__objectsfunction through adynamic statement, which PostgreSQL plans on every execution. The function
carries no
SETclause so callers can inline it, and dispatch inlined ittoo, so its whole body was planned again on every call. Dispatch now reads
the rows
WITH ORDINALITY, which the planner never inlines: the functionruns as a function and its own plan is cached for the session.
list_objectsfew 2.1 → 0.32 ms,many 2.4 → 0.52 ms, all 4.2 → 2.7 ms (
docs/BENCHMARKS.md).flattened body) planning was 10.8 ms against 0.85 ms of execution; the
same call went from 1.65 to 0.18 ms.
TestCompiledDispatchCallsTheFunctioncounts the generated function'scalls through both entry points (0 before the fix).
streamed_list_objectssearches once. It passed the search straight tounnest(), whose row estimate evaluates a stable argument at plan time, sothe first five calls in each session ran the whole search twice (a
short-lived connection paid it on every call). The search is now held in a
PL/pgSQL variable (
TestStreamedListObjectsSearchesOnce).camelCase rewrite keys. protojson takes a field under its proto name
and its JSON name; three model fields differ between the two
(
computedUserset,tupleToUserset, andcomputedUsersetinside atupleToUserset). The engine stored a camelCase model as written andanswered
falsewhere upstream answerstrue. The model write now renamesthose three to snake_case and refuses a node with both spellings (2000, as
protojson's duplicate field). No other key is renamed: every other model
field has a single name and upstream discards its camelCase form, so
renaming more would accept models upstream reads differently. The field
list comes from the openfga/api descriptors at the pin.
Condition parameter types decoded at write. A parameter type was stored
as written, and a missing or unknown
type_namefell back toTYPE_NAME_ANYat evaluation. Upstream decodes it at write: by name orenum number, LIST/MAP with exactly one generic type, the rest with none.
The engine now does the same, stores the type by name, and refuses with
upstream's codes (2000 for a shape protojson cannot decode, 2056 with
upstream's message otherwise). This is also what a camelCase
typeNamehits upstream: discarded, so the model has no type and is refused.
Tests
TestModelJSONSpellings,TestConditionParamTypes(27 cases) and the twoanswer tests send the same JSON to the oracle's HTTP API (the only
place a spelling exists; the gRPC client sends protos) and to the engine,
and compare.
oracle.URL()readsOPENFGA_URL, which the containerisedsuite already set, or
OPENFGA_HTTP_PORT.again with the old
070reinstalled.runner tests,
./scripts/build-release.shand./scripts/smoke-artifacts.sh. The full suite runs here.Compatibility
Patch release: no public function added or removed. A model naming a
parameter type upstream refuses is now refused at write; a model stored by
an earlier version keeps working until rewritten.