Skip to content

feat(router): add SQL query support to both SDKs#1129

Open
AbhilashG12 wants to merge 2 commits into
pmxt-dev:mainfrom
AbhilashG12:feat/add-sql-route-sdk
Open

feat(router): add SQL query support to both SDKs#1129
AbhilashG12 wants to merge 2 commits into
pmxt-dev:mainfrom
AbhilashG12:feat/add-sql-route-sdk

Conversation

@AbhilashG12

Copy link
Copy Markdown
Contributor
  • Add SqlResult, SqlMeta, SqlColumn types in TypeScript and Python
  • Add sql(query: string) method to Router class
  • Export types from both SDKs
  • Full type safety

Fixes #1032

@AbhilashG12 AbhilashG12 force-pushed the feat/add-sql-route-sdk branch from 3445b09 to 7860f91 Compare June 16, 2026 16:04
@realfishsam

Copy link
Copy Markdown
Contributor

PR Review: FAIL

What This Does

Adds Router.sql() methods to the Python and TypeScript SDKs for hosted ClickHouse/catalog SQL access. For SDK consumers, this is intended to expose the existing hosted /v0/sql endpoint without using raw HTTP.

Blast Radius

Router SDK layer only: Python router export/method and TypeScript router export/method. The sidecar/core SQL endpoint already exists under /v0/sql; this PR does not add or modify the server route.

Consumer Verification

Before (base branch):
The core server already has SQL route tests for POST /v0/sql / GET /v0/sql (for example core/test/pipeline/sql-route.test.ts), but neither SDK exposed a router.sql(...) helper.

After (PR branch):
Python Router.sql() targets the hosted catalog path directly:

url = f"{self._resolve_sidecar_host()}/v0/sql"

However the TypeScript method calls:

await this.sidecarReadRequest('sql', { query }, [{ query }])

sidecarReadRequest builds /api/${exchangeName}/${methodName}, so new Router().sql('select 1') will request /api/router/sql instead of the SQL endpoint that exists at /v0/sql. That means TypeScript consumers do not actually exercise the hosted SQL API this PR is trying to expose.

Test Results

  • Build: NOT VERIFIED (npm run build --workspace=pmxtjs is blocked by missing generated TypeScript SDK artifacts: ../generated/src/index.js; npm run build --workspace=pmxt-core is separately blocked by missing @buidlrrr/rain-sdk / Rain type errors)
  • Unit tests: NOT VERIFIED (no focused SQL SDK tests were added; full TS tests are blocked by missing generated SDK artifacts)
  • Server starts: PASS (local npm run server --workspace=pmxt-core reached /health)
  • E2E smoke: FAIL for TypeScript path by static consumer-path trace: the SDK dispatches to /api/router/sql, while the implemented SQL service is /v0/sql; the local server required auth for both probes, so live response-shape comparison could not proceed without a token
  • Python syntax: PASS (python3 -m py_compile sdks/python/pmxt/client.py sdks/python/pmxt/models.py sdks/python/pmxt/router.py sdks/python/pmxt/_exchanges.py)

Findings

  1. sdks/typescript/pmxt/router.ts:440Router.sql() uses sidecarReadRequest('sql', ...), which dispatches to /api/router/sql; SQL is a hosted catalog route at /v0/sql, not a sidecar exchange method. A TypeScript SDK consumer will therefore hit the wrong API surface. This should use the catalog /v0/sql endpoint (with POST support for the SQL body) or a dedicated transport helper matching the Python implementation.
  2. sdks/typescript/pmxt/router.ts / sdks/python/pmxt/router.py — there are no focused SDK tests proving router.sql('select 1') calls /v0/sql and parses {data, meta}. Given the TypeScript path mismatch above, a mocked fetch/API-client test would have caught this.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A — no market/event/order response fields added
  • OpenAPI sync: N/A — uses hosted /v0/sql, not the generated sidecar OpenAPI method set
  • Financial precision: N/A
  • Type safety: ISSUE — TypeScript implementation compiles conceptually but dispatches through the wrong transport abstraction for this endpoint
  • Auth safety: OK — no credential logging observed in the diff

Semver Impact

minor -- adds a new Router SQL SDK method.

Risk

Python appears to target the right endpoint, but TypeScript is not safe to ship as-is because its public method will not reach the intended /v0/sql service. Consumer-path tests are needed for both SDKs before merge.

…ql()

- Use resolveBaseUrl() for /v0/sql endpoint
- Use fetchWithRetry() for robust network calls
- Add proper auth headers

Fixes pmxt-dev#1032
@AbhilashG12

Copy link
Copy Markdown
Contributor Author

PR Updated ✅

TypeScript SQL Fix:

  • Now uses resolveBaseUrl() and fetchWithRetry() from parent Exchange class
  • Both methods are now protected so Router can access them
  • Uses correct /v0/sql endpoint

Verification:

  • ✅ TypeScript compiles without errors
  • ✅ SQL method uses correct endpoint
  • ✅ Auth headers are properly applied

Ready for re-review! 🙏

@realfishsam

Copy link
Copy Markdown
Contributor

PR Review: PASS (NOT VERIFIED)

What This Does

Adds Router.sql(query) helpers and SQL result metadata types to both SDKs, exposing the existing /v0/sql catalog query endpoint to SDK consumers. This matters for hosted/enterprise consumers that use PMXT SQL for analytics and custom catalog queries.

Blast Radius

Python SDK router/export surface and TypeScript SDK router/export surface. The core /v0/sql server route already exists; this PR does not change the SQL server implementation, schemas, or exchange adapters.

Consumer Verification

Before (base branch):
SDK consumers had no Router.sql(...) method or exported SqlResult/SqlMeta/SqlColumn types in either SDK, despite the core sidecar mounting /v0/sql.

router.sql("SELECT 1 AS one")
# base branch: method is absent from sdks/python/pmxt/router.py
await router.sql('SELECT 1 AS one')
// base branch: method is absent from sdks/typescript/pmxt/router.ts

After (PR branch):
The local sidecar route was reachable with the normal local access-token header, but this environment does not have ClickHouse configured, so query execution returned the expected service-level blocker rather than query rows:

POST /v0/sql {"query":"SELECT 1 AS one"}
{
  "error": "service_unavailable",
  "message": "SQL query service is not available. Configure CLICKHOUSE_HTTP_URL or use the hosted PMXT Enterprise SQL endpoint."
}

Python files compile, and the new SDK methods are present. Full SDK consumer verification against a live SQL backend was not possible in this environment.

Test Results

  • Build: core build PASS (npm run build --workspace=pmxt-core)
  • Unit tests: NOT RUN (no focused SQL SDK tests were added)
  • Server starts: PASS (local server reached /v0/sql)
  • E2E smoke: NOT VERIFIED (/v0/sql returned 503 because CLICKHOUSE_HTTP_URL is not configured locally)

Findings

No blocking findings.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A (no unified market/event fields added)
  • OpenAPI sync: N/A (uses existing hosted/catalog /v0/sql route, not generated exchange methods)
  • Financial precision: N/A
  • Type safety: PARTIAL (Python py_compile passes; full TypeScript package build is blocked by missing generated TS OpenAPI artifacts in this checkout)
  • Auth safety: OK (SQL requests use existing SDK auth-header helpers)

Semver Impact

minor -- adds new public SDK methods/types.

Risk

The actual SQL query path could not be verified without a configured SQL backend. There are also no regression tests proving the Python/TypeScript SDK methods parse successful SQL responses, so this review is limited to static SDK inspection plus route reachability/service-unavailable evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sidecar /v0/sql ClickHouse query route not exposed in either SDK

2 participants