fix(plugins): correct plugin-storage query/count/index/order on Postgres - #1
Open
vedanshujain wants to merge 2 commits into
Open
Conversation
This was referenced Jul 17, 2026
added 2 commits
July 19, 2026 13:07
…nb + numeric cast)
Plugin storage keeps documents in `_plugin_storage.data`, a plain `text`
column. On Postgres the query/count/index paths extracted fields with the
JSON operator `->>` and no cast, which broke in two ways:
1. Parse failure: `text ->> 'x'` has no operator in Postgres, so `query()`,
`count()`, and the UNIQUE/expression index DDL raised
"operator does not exist: text ->> unknown".
2. Lexical comparison: even once parseable, `(data::jsonb)->>'field'` is
still `text`, so a numeric guard like `stock >= 10` compared lexically
(`'9' >= '10'` is TRUE) and silently over-counted / oversold.
Fix, scoped to the `_plugin_storage.data` path only (the generic
`jsonExtractExpr` used by real json/jsonb content columns is untouched):
- Add `pluginDataExtractExpr(db, field, { numeric })`: on Postgres emits
`(data::jsonb)->>'field'`, and `((data::jsonb)->>'field')::numeric` when
the bound operand is numeric; SQLite keeps `json_extract` (already typed).
- `buildCondition` decides numeric vs text per condition from the JS type of
the bound value (number, all-number `in` list, per-bound RangeFilter);
string/boolean/startsWith stay text. Index expressions extract as text only
(uniqueness is on the textual value).
- The raw-WHERE splice in `query()`/`count()` no longer wraps the condition in
`= 1`: Postgres parses `<cond> = 1` as a chained comparison (syntax error)
and rejects `boolean = integer`. The condition is passed to `.where()` as a
bare boolean, valid on both dialects; the `?`/params contract is preserved.
- `count()` coerces the result to a number (Postgres returns bigint COUNT as a
string), honouring its `Promise<number>` contract on both dialects.
Unblocks dialect-parameterized plugin-storage tests; adds a
`describeEachDialect` suite covering the numeric-range boundary, `in`,
`startsWith`, equality, count, and unique/expression index creation on both
SQLite and Postgres.
…ally on Postgres
Addresses review findings on the query()/count() path this PR fixes:
1. orderBy sorted numeric fields lexically on Postgres. `ORDER BY
(data::jsonb)->>'field'` sorts [10, 100, 9] on PG while SQLite's
json_extract sorts [9, 10, 100]. Add `pluginDataOrderExpr`, which orders
over the jsonb-native value `(data::jsonb)->'field'` — numeric among
numbers, lexical among strings, and total across heterogeneous data (never
throws). SQLite keeps json_extract (already numeric). orderBy in query() and
buildOrderByClause now route through it.
2. The numeric guard threw on a single non-number stored value and diverged
from SQLite: `((data::jsonb)->>'field')::numeric` raises
"invalid input syntax for type numeric" the moment any scanned row stores a
non-number, aborting the whole query, whereas SQLite silently compared.
Guard the cast with a type check on both dialects so a non-number yields
NULL (no match), never an error, with identical results:
PG: CASE WHEN jsonb_typeof((data::jsonb)->'f')='number'
THEN ((data::jsonb)->>'f')::numeric END
SQLite: CASE WHEN json_type(data,'$.f') IN ('integer','real')
THEN json_extract(data,'$.f') END
This intentionally changes the numeric-comparison SQL on both dialects (for
parity); the exact-SQL unit assertions are updated accordingly.
3. Document that numeric predicates fall back to a sequential scan on Postgres
(the per-field expression index is text-typed; field types aren't known at
index-creation) — comment in storage-indexes.ts and a note in the changeset.
Tests (describeEachDialect, both dialects): numeric orderBy asserts [9,10,100]
without .toSorted() masking; heterogeneous field (5 vs "abc") returns only the
numeric row with no throw on either dialect; plus boolean equality (PG — bound
JS boolean vs text 'true'/'false'; better-sqlite3 can't bind booleans),
mixed-type `in` text fallback, negative/zero/float numeric guards, and a
null-field IS NULL match.
vedanshujain
force-pushed
the
fix/plugin-storage-postgres-jsonb
branch
from
July 19, 2026 13:09
d7d732d to
8b89c14
Compare
vedanshujain
changed the base branch from
main
to
test/plugin-storage-postgres-regression
July 19, 2026 13:10
18 tasks
Overlapping PRsThis PR modifies files that are also changed by other open PRs:
This may cause merge conflicts or duplicated work. A maintainer will coordinate. |
github-actions
Bot
added
overlap
review/needs-review
|
This PR has been inactive for 14 days. It will be closed automatically in 7 days if there is no further activity. If you're still working on this, please push an update or leave a comment. |
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
1 participant
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.
Summary
Fixes plugin storage
query(),count(), unique/expression index creation, andorderByon Postgres. The_plugin_storage.datacolumn istext, but the query path extracted JSON fields with the Postgres->>operator and no cast, so plugin storage was effectively broken and untested on Postgres (only SQLite was exercised). This is a prerequisite for atomic conditional writes (insert/updateIf), which need a correct numeric guard on Postgres.The bugs (all verified on live Postgres 16)
text ->> 'field'has no operator in Postgres →operator does not exist: text ->> unknown. Every plugin-storagequery()/count()and theCREATE [UNIQUE] INDEX … ((data->>'field'))DDL errored on PG.query()/count()where-splice wrapped each condition as<cond> = 1, which isboolean = integeron PG →syntax error at or near "=". No plugin-storage query ran on PG regardless of extraction.(data::jsonb)->>'field'istext, so a numeric range guard likestock >= 10compared lexically ('9' >= '10'is TRUE) → silent over-counting / oversell.::numerictotality: a naive cast throws on any row storing a non-number in that field, and diverged from SQLite.orderBy: numeric fields sorted lexically on PG ([10,100,9]).The fix (scoped to the
_plugin_storage.datapath only — genericjsonExtractExpruntouched)pluginDataExtractExpr(db, field, { numeric }): PG(data::jsonb)->>'field'; for numeric comparison operands a type-guarded cast —CASE WHEN jsonb_typeof((data::jsonb)->'field')='number' THEN ((data::jsonb)->>'field')::numeric END(SQLite:CASE WHEN json_type(data,'$.field') IN ('integer','real') THEN json_extract(…) END). Non-number values evaluate toNULL→ excluded, no throw, identical on both dialects.pluginDataOrderExpr:orderByuses jsonb-native ordering(data::jsonb)->'field'on PG (numeric-among-numbers, total, never throws); SQLite keepsjson_extract.buildRawWhereExpression: replaces the= 1wrapper with a bare-booleanRawBuilder<SqlBool>.where(...)(same?/param interleave; SQLite SQL semantically unchanged), DRYing the query/count splice.count()coerces PG's bigintCOUNT(*)string withNumber(...).SQLite behavior is unchanged except that a string-stored value like
"10"no longer satisfies a numeric guard (previously allowed via storage-class ordering) — now standardized to match Postgres. Numeric predicates fall back to a sequential scan on Postgres (the text expression index can't satisfy a::numericpredicate; field types aren't known at index-creation) — documented instorage-indexes.ts.Tests
New dialect-parameterized suite
storage-postgres-query.test.ts(runs on SQLite and Postgres viadescribeEachDialect+EMDASH_TEST_PG): equality, numeric RangeFilter across the multi-digit boundary (9/10/100,gte:10→ {10,100}),in,startsWith,countwith numeric range, unique-index creation + enforcement on PG, numericorderBy([9,10,100]), heterogeneous-field totality (number vs"abc"→ numeric row only, no throw, parity), boolean-eq, mixedin, negative/zero/float, null-field. Existing SQLite unit exact-SQL assertions updated to the newCASEform. Full plugin + dialect-compat run green on both dialects; typecheck / lint / format clean.Notes
emdash: patchchangeset included.