Repository navigation
fix(pg18): unpack array params for IN clauses; adopt core 0.3.1 list helper - #26
Conversation
Bun 1.3.13's bun:sql driver encodes JS arrays via Array.prototype.toString
("a,b,c"), which Postgres 18's stricter parser rejects for ANY($N::T[]).
Track upstream Bun PR #29552; until merged, replace the broken pattern
with explicit numbered placeholders and spread the array into params.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bun 1.3.13's bun:sql encodes JS arrays via Array.prototype.toString, which Postgres 18 rejects for ANY($N::T[]). Convert the skeleton-entries batch fetch to numbered placeholders + spread params. Tracking Bun PR #29552 upstream. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bun 1.3.13's bun:sql encodes JS arrays via Array.prototype.toString, which Postgres 18 rejects for ANY($N). Switch the three tagged-template queries to Bun's SQL value helper, which expands the array into individual placeholders. Cast the PgClient locally to access the value-helper call signature (PgClient from @easier-idx/core only types the tagged-template signature). Track upstream Bun PR #29552. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bun 1.3.13 bun:sql encodes JS arrays via Array.prototype.toString, which Postgres 18 rejects for ANY array parameters. Convert the five ANY/NOT(ANY) sites in global-store-pg.ts to numbered IN/NOT IN placeholders, spreading the array into the params list. Empty-array guards already exist for every call site (early return or distinct branch). Track upstream Bun PR #29552. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Bun 1.3.13 bun:sql encodes JS arrays via Array.prototype.toString, which Postgres 18 rejects for ANY($N). Convert the four ANY sites in the MCP file tools (traceImportChain, getCrossRepoEdges, findCallers, findImplementors) to numbered IN placeholders, spreading the scoped repo IDs into params. Preserve the original ANY-on-empty-array security semantics by short-circuiting to no rows when a non-null scoped list is empty (raw IN () is invalid SQL). Track upstream Bun PR #29552. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Core 0.3.1 (smart-knowledge-systems/easier-idx#5) adds a typed list helper: pg(arr) returns a PgList that buildTaggedQuery expands inline as IN ($1,$2,...). xref.ts was already using this idiom via a cast that bypassed the type system but failed at runtime ("Unknown object is not a valid PostgreSQL type") because the wrapper's tagged-template path didn't recognize the helper. With 0.3.1 the wrapper expands the helper itself, so the cast and the alias variable can go away. Note: bun install / bun run check are intentionally deferred until core 0.3.1 is merged and published. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
….3.1 StoreOps in core 0.3.x requires a transaction method that the inline ops in getStoreOps didn't provide, breaking typecheck after the dep bump. Rather than re-implement transaction here, switch to the upstream factories — they handle pg-style placeholder rewriting, BEGIN/COMMIT/ ROLLBACK semantics, and reserved-connection guarantees that hand-rolled ops would have to duplicate. Drops the now-unused pgUnsafe and pgToSqlite imports. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Both branches converged on the same fix (replace ANY($n) array params with IN ($1,$2,...) positional placeholders to work around Bun.SQL nested-array misserialisation). Resolved src/search/query.ts and src/search/search-pg.ts in favour of full positional parameterisation for both repo IDs and file paths, and kept the empty-resultRepoIds guard from main (PR #25). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Greptile SummaryThis PR addresses a
Confidence Score: 5/5Safe to merge — all changed code paths have been updated consistently and the fixes are mechanically correct. Every ANY($N) call site is correctly migrated to positional IN placeholders with matching param arrays. The large-hash dedup paths are now protected by a well-structured chunked temp-table strategy. The xref fix eliminates a latent runtime breakage (Promise-as-parameter) that predates this PR. The StoreOps refactor is a clean net reduction in custom code. No incorrectly constructed placeholder index offsets, no missed empty-array guards, and no new cross-cutting concerns were introduced. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant search-pg/query as search-pg / query.ts
participant files.ts as mcp/files.ts
participant global-store-pg as dedup/global-store-pg.ts
participant xref.ts
participant PG18 as PostgreSQL 18
Note over Caller,PG18: Before: ANY($N::int[]) bun:sql encodes as a,b,c PG18 rejects
Caller->>search-pg/query: searchPg / withSnippets
search-pg/query->>PG18: WHERE repo_id IN ($1,$2) AND file_path IN ($3,$4,...)
PG18-->>search-pg/query: rows
Caller->>files.ts: getFileDependencies / getCrossRepoEdges
files.ts->>files.ts: "scopedRepoIds.length===0 short-circuit []"
files.ts->>PG18: WHERE repo_id IN ($1,$2,...)
PG18-->>files.ts: rows
Caller->>global-store-pg as dedup/global-store-pg.ts: countBlobsExcept / deleteBlobsExcept
global-store-pg->>PG18: BEGIN
global-store-pg->>PG18: CREATE TEMP TABLE _live_hashes ON COMMIT DROP
loop chunks of 5000
global-store-pg->>PG18: INSERT INTO _live_hashes VALUES ($1),($2),...
end
global-store-pg->>PG18: SELECT/DELETE WHERE NOT EXISTS _live_hashes
PG18-->>global-store-pg: result
global-store-pg->>PG18: COMMIT drops _live_hashes
Caller->>xref.ts: xrefPg
xref.ts->>PG18: WHERE content_hash IN pg(definitionHashes)
PG18-->>xref.ts: rows
Reviews (2): Last reviewed commit: "refactor(search): reuse single placehold..." | Re-trigger Greptile |
| if (scopedRepoIds) { | ||
| const placeholders = scopedRepoIds.map((_, i) => `$${i + 1}`).join(","); | ||
| query = `SELECT sr.name AS source_repo, tr.name AS target_repo, | ||
| sf.file_path AS source_file, tf.file_path AS target_file, | ||
| e.imported_module | ||
| FROM cross_repo_edges e | ||
| JOIN repos sr ON sr.id = e.source_repo_id | ||
| JOIN repos tr ON tr.id = e.target_repo_id | ||
| JOIN files sf ON sf.id = e.source_file_id | ||
| JOIN files tf ON tf.id = e.target_file_id | ||
| WHERE (e.source_repo_id = ANY($1::int[]) OR e.target_repo_id = ANY($1::int[])) | ||
| ORDER BY sr.name, tr.name` | ||
| : `SELECT sr.name AS source_repo, tr.name AS target_repo, | ||
| WHERE (e.source_repo_id IN (${placeholders}) OR e.target_repo_id IN (${placeholders})) | ||
| ORDER BY sr.name, tr.name`; | ||
| params = [...scopedRepoIds]; |
There was a problem hiding this comment.
$N placeholder reuse is valid but inconsistent with sibling code
The getCrossRepoEdges query uses the same placeholders string ($1,$2,…$N) for both source_repo_id IN (…) and target_repo_id IN (…), passing a single copy of scopedRepoIds in params. PostgreSQL's wire protocol supports $N reuse within a query, so this is functionally correct. However, withCrossRepoEdges in src/search/query.ts handles the same pattern with separate placeholders/placeholders2 index ranges and two copies of the array — a style divergence that may confuse future readers. Consider aligning to one convention.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…sets Spreading every live hash into its own bind parameter risks tripping PostgreSQL's 65535 bind-parameter ceiling on large dedup sets. Load the live hashes into a session-scoped temp table in 5000-row chunks and let the filter join against that instead, so the IN-list size no longer caps how many blobs we can sweep. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Align withCrossRepoEdges with the style already used by getCrossRepoEdges in src/mcp/tools/files.ts: PostgreSQL's wire protocol lets the same \$1..\$N placeholders feed both IN clauses, so we only need to bind the repo-id list once instead of duplicating it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@greptile-apps Both P2 issues addressed; please re-review:
(Note: greptile's prior pass was on 546ee75; main has since been merged at 960d16d, which may need a fresh look.) Ready for re-review. |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
Summary
Fixes
codeindex search,codeindex xref, and the dedup/MCP code paths after the PG15→PG18 migration. The root cause is abun:sqldriver bug: it encodes JS arrays viaArray.prototype.toStringand ships"a,b,c"over the wire instead of a Postgres array literal. PG15 was lenient enough to occasionally tolerate the malformed encoding forint[]; PG18 rejects it cleanly withmalformed array literal/number of array dimensions (102) exceeds the maximum allowed (6)errors.Type casts (
$1::int[]) do not help — params must be unpacked into individual placeholders.What changed
Search/dedup/MCP — replaced every
ANY($N)withIN ($1, $2, ...), unpacking the array into separate bind vars. Empty-array short-circuits added in MCP files.ts to preserve the "no access" semantics that previously relied on[]being truthy +ANY(empty)matching no rows.Xref — uses the new
pg(arr)list helper from@easier-idx/core@0.3.1(smart-knowledge-systems/easier-idx#5). Tagged-template syntax:pg`WHERE id IN ${pg(fileIdArray)}`. The earlier cast workaround (pg as unknown as <T>(value: T) => unknown) was always broken at runtime — returned a Promise that got encoded as[object Promise]— soxrefwas unusable on either PG15 or PG18 since that workaround was added.Repo store ops —
StoreOpsin core 0.3.x added a requiredtransactionmethod. Switched from hand-rolled inline ops to the upstreamcreatePgStoreOps/createSqliteStoreOpsfactories instead of re-implementing transactions here.Commits
bdcfe51fix(search): replace ANY($N) with IN ($1,$2,...) for PG18 compat579eac1fix(search): replace ANY($N) with IN ($1,$2,...) in query.ts5335338fix(xref): replace ANY(${arr}) with IN ${sql(arr)} for PG18 compatc6f5b01fix(dedup): replace ANY array params with IN placeholders for PG182d5b00afix(mcp): replace ANY array params with IN placeholders for PG18d8b7132chore(deps): bump @easier-idx/core to 0.3.1, drop xref cast workaround546ee75refactor(repo): use createPgStoreOps/createSqliteStoreOps from core 0.3.1Test plan
bun run check— typecheck + lint + prettier all cleancodeindex search 'rate limiting' --top-n 3— returns ranked JSON results, no array-dimensions errorscodeindex xref getPg— returns cross-repo definitions and consumers (was broken before this PR even on PG15 due to the cast workaround returning a Promise)codeindex reindexsmoke against an updated working treeUpstream context
The Bun array encoding bug is tracked in oven-sh/bun#29552 (open since Apr 2026, has the actual fix, not yet merged). When that ships and we upgrade Bun, the
IN ($1,$2,...)workarounds remain valid and arguably more CockroachDB-portable thanANY($N::int[]).🤖 Generated with Claude Code