Skip to content

execute_sql makes a redundant PARSE_STATEMENT round trip on every call (~2x latency on remote systems) #151

Description

@ajshedivy

Summary

execute_sql runs a server-side QSYS2.PARSE_STATEMENT validation query on the wire before every real query, even though it has already classified the statement (read-only / statement type) in-process via the vscode-db2i parser. On remote or high-latency IBM i systems — where a single round trip to the database costs several seconds — this roughly doubles the latency of every execute_sql call.

Evidence

Server debug logs for a single warm SELECT 1 FROM SYSIBM.SYSDUMMY1 (pool already initialized and reused — not a cold start):

in-process validation (vscode-db2i parser)  →  statementTypes:["Select"], isReadOnly:true   (~0ms, no round trip)
QSYS2.PARSE_STATEMENT  (validateWithParseStatement)  →  executionTime 3,824ms  / ~5.7s wall   ← redundant
SELECT 1 …            (executeQueryWithPagination)   →  executionTime 1,587ms  / ~3.4s wall   ← the real query
total tool durationMs ≈ 9,100ms      ("Pool 'Symbol(ibmi-singleton-pool)' already initialized")

So ~60% of a warm execute_sql call is the redundant on-the-wire parse. The raw TCP connect time to the same host was ~100ms, so this is not the network or a cold pool — it is a second, avoidable IBM i round trip.

Where

packages/server/src/ibmi-mcp-server/tools/executeSql.tool.ts, executeSqlLogic:

  1. validateSqlSecurity()in-process AST/regex + vscode-db2i parser (~:328)
  2. validateWithParseStatement()on-the-wire QSYS2.PARSE_STATEMENT, runs unconditionally (~:332), gate at ~:258 (if (readOnly && statementType !== "QUERY"))
  3. executeQueryWithPagination() — the actual query (~:342)

The in-process SqlSecurityValidator (packages/server/src/ibmi-mcp-server/utils/security/sqlSecurityValidator.ts, validateReadOnlyRestrictions) already derives isReadOnly + statementTypes from the IBM i parser, with a regex fallback when the parser can't tokenize. So step 2 duplicates the read-only gate that step 1 already performed for free.

Possible solutions

These compose; 1 + 2 are the core fix and should ship together (see the safety note).

1. Make the wire PARSE_STATEMENT a fallback instead of unconditional.
Skip validateWithParseStatement() when the in-process parser successfully classified the statement; invoke it only when the in-process parser returns success:false (could not classify), or when an operator opts into a strict/audit mode via a new env var (e.g. IBMI_EXECUTE_SQL_PARSE_VALIDATION, defaulting to the fast path). Removes one IBM i round trip from the common read-only path.

2. Enforce read-only at the connection level.
Open the mapepire pool with the IBM i Toolbox JDBC access=read only property (via jdbcOptions) whenever read-only mode is configured. Db2 then rejects any write/DDL at execution, fail-closed — which makes the per-query parse unnecessary for safety, not merely redundant. There is currently no access default (.env.example leaves it empty), so this is also closing a gap. If read-only stored-procedure CALLs are required, access=read call is the looser option.

Safety note: solutions 1 and 2 should land in the same release — only drop the wire parse once the connection-level read-only backstop is the default, so no version weakens the read-only guarantee. A pre-merge smoke test (open a read-only pool, assert an UPDATE/INSERT/DDL is rejected by Db2) is worth gating on.

3. Return parse metadata inline in the execute_sql response.
Surface the already-computed in-process parse result (statement type, read-only flag, referenced objects) as an additive field on the execute_sql response, so callers that want validation feedback don't need a separate validate_query round trip before running. Map the parser's parse-failure case to an explicit "uncertain" state rather than a literal isReadOnly:false.

Secondary / related (lower priority)

  • /healthz pool visibility. execute_sql uses the singleton IBMiConnectionPool (keyed by Symbol("ibmi-singleton-pool"), services/connectionPool.ts), but /healthz only enumerates the named YAML-source pools (SourceManager). The actually-used pool never appears, so the source pools report initialized:false indefinitely even after many execute_sql calls — misleading when diagnosing "is the pool warm?". Consider including the singleton (and auth) pools in the /healthz pools map.
  • Pool prewarming / sizing. Pools initialize lazily on first query (baseConnectionPool.ts), so the first call after startup (or after MCP_POOL_IDLE_TIMEOUT_MS, default 5 min) pays full init. startingSize/maxSize (defaults 2/10) are not env-configurable on the singleton/YAML path — only the HTTP-auth endpoint can override them. An optional startup prewarm and MCP_POOL_*_SIZE env knobs would smooth cold-start spikes.

Impact

On a LAN the extra parse is minor. On remote/cloud IBM i systems, where each round trip is multiple seconds, dropping the redundant PARSE_STATEMENT roughly halves execute_sql latency on the common read-only path, and the connection-level read-only change makes the safety guarantee fail-closed at the database rather than dependent on a parser.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requesttoolsfeatures and enhancements to SQL tools

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions