refactor: replace WeakMap with Grafast meta system + adopt Benjie's plugin patterns#776
Merged
pyramation merged 5 commits intomainfrom Mar 4, 2026
Merged
Conversation
…in all search plugins
Replaces the module-level WeakMap pattern with PostGraphile's built-in
meta system for passing data between the condition apply phase and
output field plan, following the pattern from Benjie's
postgraphile-plugin-fulltext-filter reference implementation.
Changes across all three plugins (graphile-search-plugin,
graphile-pgvector-plugin, graphile-pg-textsearch-plugin):
- Remove module-level WeakMap and slot interfaces
- Replace dangerouslyGetParent() with getQueryBuilder() traversal
(walks PgCondition .parent chain to find PgSelectQueryBuilder)
- Condition apply now calls qb.setMeta(key, { selectIndex }) instead
of storing in WeakMap
- Output field plan now calls $select.getMeta(key) which returns a
proper Grafast Step, used with lambda([$details, $row], ...)
- Remove getPgSelectStep helper, use $row.getClassStep() directly
GraphQL interfaces remain identical - no API changes for consumers.
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
- TypeScript namespace augmentations (Inflection, BehaviorStrings, ScopeObjectFieldsField, Plugins) - Custom inflection methods (pgTsvRank, pgVectorDistance, pgBm25Score, etc.) - Behavior registry + entity behavior for per-column opt-out via smart tags - qb.mode === 'normal' check to prevent injection into aggregate queries - TYPES.float.fromPg() instead of parseFloat() for proper codec conversion - Computed column (proc) support for tsvector plugin - Duck-typing enhancement for query builder detection
- TypeScript namespace augmentations (declare global) - Custom inflection methods (overridable naming) - Behavior registry with entityBehavior callbacks - TYPES.float.fromPg() instead of parseFloat() - Computed column (proc) support for tsvector - Remove extra where check from getQueryBuilder duck-typing - Keep simple constantCase enum naming for direct columns to prevent mismatch
… condition reads it at execution time The orderBy enum apply runs at PLANNING time (receives PgSelectStep) while condition apply runs at EXECUTION time (receives runtime proxy). The proxy's meta is initialized from PgSelectStep._meta, so data stored at planning time IS available at execution time. Flow: 1. Enum apply (planning): stores direction flag in PgSelectStep._meta 2. PgSelectStep._meta gets copied to proxy's meta closure 3. Condition apply (execution): reads direction, adds ORDER BY with scoreFragment This fixes the 'sort by full text rank orderBy enums works' test - ASC and DESC now produce different result orderings. Also removes the stale WeakMap bridge that was no longer needed.
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.
refactor: replace WeakMap with Grafast meta system + adopt Benjie's plugin patterns
Summary
Replaces the module-level
WeakMappattern with PostGraphile's built-insetMeta/getMetasystem for passing computed data (rank/distance/score indices) between the condition apply phase and the output field plan. This follows the pattern recommended by Benjie (Graphile maintainer) in his postgraphile-plugin-fulltext-filter reference implementation.All three search plugins receive identical changes:
graphile-search-plugin(tsvector full-text search)graphile-pgvector-plugin(pgvector similarity search)graphile-pg-textsearch-plugin(BM25 ranked search)What changed internally:
WeakMap+ slot interfacesgetPgSelectStep()duck-typing helpergetQueryBuilder(build, $condition)which walksPgCondition.parentchain (viabuild.dataplanPg.PgConditioninstanceof)qb.setMeta(key, { selectIndex })instead of storing in WeakMap$select.getMeta(key)(returns a Grafast Step) withlambda([$details, $row], ...)instead of closure-captured WeakMap lookup$condition.dangerouslyGetParent()withgetQueryBuilder()for a more stable traversal patternNo API changes intended — GraphQL schema (field names, input types, orderBy enums) should remain identical, but see review checklist item about inflection methods.
Benjie's Plugin Pattern Improvements
Six improvements from Benjie's reference implementation, applied across all three plugins:
declare globalblocks forGraphileBuild.Inflection,BehaviorStrings,ScopeObjectFieldsField, andGraphileConfig.PluginsinterfacespgTsvRank,pgVectorDistance,pgBm25Score,pgTsvOrderByColumnRankEnum, etc.)@behavior -attributeFtsRank:selectto hide rank field for a specific column). By default everything is auto-exposed (no behavior change for existing schemas).qb.mode === 'normal'check — Prevents injecting SELECT expressions into aggregate queries. Applied only to theselectAndReturnIndex/setMetablock (pgvector + BM25 plugins); intentionally not applied to the ORDER BY meta-read block.TYPES.float.fromPg()instead ofparseFloat()— Proper codec conversion instead of JS-level parsingUpdates since last revision
Fix: orderBy enum timing mismatch (graphile-search-plugin)
Root cause identified and fixed for the
"sort by full text rank orderBy enums works"test failure.The problem: The orderBy enum
applyruns at planning time (receivesPgSelectStep), while the conditionapplyruns at execution time (receives a runtime proxy). These two objects have separate meta stores, so they cannot share data viasetMeta/getMetaRawdirectly between each other.The solution: Leverage the fact that
PgSelectStep._metagets copied to the proxy's meta closure during execution. Flow:PgSelectStep._metaviasetMetaproxy.metaviagetStaticInfo→buildTheQueryCoreproxy.getMetaRaw, adds ORDER BY withscoreFragmentThis fixes the test — ASC and DESC now produce different result orderings. Also removes the stale
_ftsRankBridgeWeakMap that was attempted as an intermediate fix but didn't work.Note: pgvector and BM25 plugins already had the correct pattern and did not need this fix.
Entity behavior phase change
Changed
entityBehaviorcallbacks frominferredphase tooverridephase withafter: ['inferred'], before: ['override']ordering. This ensures the behavior additions (e.g.'attributeFtsRank:select') are prepended to the behavior array before any user-specified overrides, allowing users to negate them with smart tags.Codec identity check improvement
Replaced direct codec identity comparisons (e.g.
attr.codec === build.dataplanPg.TYPES.tsvector) withisTsvectorCodec()helper that checkscodec.extensions.pg.name === 'tsvector'. More robust across PostGraphile RC versions.Review & Testing Checklist for Human
This PR cannot be fully tested locally (requires PostgreSQL + pgvector/pg_textsearch extensions in CI). Priority items for review:
pgTsvOrderByColumnRankEnum) callthis._attributeName({codec, attributeName, skipRowId: true})instead of using rawattributeName. If_attributeNameproduces different output than the raw attribute name, orderBy enum names will change, breaking clients. Compare a GraphQL introspection query before/after this PR to confirm field/enum names are identical.filter.test.tsfailure was caught by CI and fixed; ensure no other edge cases exist.PgSelectStep._metabeing copied to the execution-time proxy's meta closure. This is confirmed by reading PostGraphile source (lines 1322 + 1785 ofpgSelect.js) but could break if PostGraphile internals change. If orderBy enums stop working in future PostGraphile versions, this is the first place to check.overridephase withafter: ['inferred']. If other plugins also useoverridephase, the ordering might cause behaviors to be evaluated in unexpected order. Verify rank/distance/score fields still appear on expected columns.CREATE FUNCTION article_combined_search(article) RETURNS tsvector ...), verify that rank fields appear for them and work correctly.Notes
@pgsql/quotesTypeScript error in BM25 plugin is pre-existing on main (not caused by this PR) — BM25 plugin cannot be built locally but CI has the necessary dependenciesgetQueryBuilder()is duplicated across all 3 files (not extracted to shared utility) — could be refactored in a follow-up