Skip to content

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

Open
AbhilashG12 wants to merge 7 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 7 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.

@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.

@AbhilashG12

AbhilashG12 commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the PASS review . I have manually tested them :

Python:

from pmxt import Router, SqlResult, SqlMeta, SqlColumn
router = Router(pmxt_api_key='test')

✅ Router created
✅ Has sql method? True

Types imported successfully
✅ SqlResult: <class 'pmxt.router.SqlResult'>
✅ SqlMeta: <class 'pmxt.router.SqlMeta'>
✅ SqlColumn: <class 'pmxt.router.SqlColumn'>

Typescript

import { Router } from 'pmxtjs';
const router = new Router({ pmxtApiKey: 'test' });

✅ Router created
✅ Has sql method? true
✅ SQL types are available at compile time (TypeScript interfaces)

Types

Router exported = working
router.sql() exists = working
SqlResult undefined at runtime = expected (it's a TypeScript interface)

@AbhilashG12 AbhilashG12 force-pushed the feat/add-sql-route-sdk branch from c497de8 to 91ec8d9 Compare July 9, 2026 12:38
- Add sql(query: string) method to Router class
- Add SqlResult, SqlMeta, SqlColumn types
- Export types from both SDKs

Fixes pmxt-dev#1032
@AbhilashG12 AbhilashG12 force-pushed the feat/add-sql-route-sdk branch from 4028530 to 7a54c84 Compare July 9, 2026 13:09
- Restore full __init__.py from upstream
- Add SQL exports (SqlResult, SqlMeta, SqlColumn)

Fixes CI for pmxt-dev#1032
- Add SqlResult, SqlMeta, SqlColumn imports
- Add SQL exports to __all__

Fixes CI for pmxt-dev#1032
@realfishsam

Copy link
Copy Markdown
Contributor

PR Review: FAIL

What This Does

Intends to expose router SQL query support through both SDKs and generated docs/references.

Blast Radius

Python SDK package root/router, TypeScript SDK client/router, and generated API docs. This is SDK-publishing critical because it touches package files shipped to PyPI/npm.

Consumer Verification

Before (base branch):
SDK router/client files are normal UTF-8 source files and can be imported/compiled by consumers.

After (PR branch):
sdks/typescript/pmxt/router.ts, sdks/python/pmxt/router.py, and sdks/python/pmxt/__init__.py are UTF-16LE/BOM-style binary files in git (diff shows Bin ...; byte check begins with ff fe and embedded NUL bytes). TypeScript/Python consumers and build tools expect UTF-8 text source and will fail or mis-parse these files.

Test Results

  • Build: PASS for current main core build during scheduled run
  • Unit tests: PASS for current main core Jest (699 passed, 3 skipped)
  • Server starts: NOT VERIFIED for this PR head
  • E2E smoke: FAIL by source encoding check on shipped SDK files

Findings

  1. sdks/typescript/pmxt/router.ts — stored as UTF-16LE with embedded NUL bytes rather than UTF-8 TypeScript source. This is visible in git as a binary diff and confirmed by the leading bytes ff fe ....
  2. sdks/python/pmxt/router.py and sdks/python/pmxt/__init__.py — also show as binary diffs, indicating generated Python source was committed in a non-text encoding. These files are part of the published SDK surface.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A for SQL route surface
  • OpenAPI sync: NOT VERIFIED
  • Financial precision: N/A
  • Type safety: ISSUE (source files are not valid normal TS/Python text in repo)
  • Auth safety: N/A

Semver Impact

minor -- new SDK feature, but current artifact encoding is blocking.

Risk

Very high: publishing this head can ship unreadable SDK source files to npm/PyPI consumers.

- Fix router.ts, router.py, __init__.py encoding
- Use UTF-8 with BOM instead of UTF-16LE
- Add SQL exports to client.ts

Fixes pmxt-dev#1032
@realfishsam

Copy link
Copy Markdown
Contributor

PR Review: FAIL

What This Does

Adds/export SQL result types and router sql() support in the SDK surfaces. This matters to SDK consumers because it is a new hosted/router API method intended to query PMXT's ClickHouse-backed SQL endpoint.

Blast Radius

Python package root exports, Python router module, TypeScript router/client exports, and generated API reference docs.

Consumer Verification

Before (base branch):
The SDK router modules do not expose SQL result helper types or a TypeScript Router.sql() helper.

After (PR branch):
Python files still compile syntactically:

python3 -m py_compile sdks/python/pmxt/router.py sdks/python/pmxt/__init__.py
pycompile=0

But the changed source files contain mojibake/encoding corruption in user-facing docs/comments immediately at the top of both router modules:

sdks/python/pmxt/router.py:2  Router ΓÇö cross-venue intelligence layer.
sdks/typescript/pmxt/router.ts:2  Router ΓÇö cross-venue intelligence layer.

The diff also shows a UTF-8 BOM inserted at the beginning of the Python package root/router and TypeScript router files. I did not verify a live /v0/sql call because this hosted endpoint requires service availability/auth; the static source regression is already blocking for a published SDK PR.

Test Results

  • Build: NOT VERIFIED (TypeScript SDK build is blocked in this review environment by missing generated SDK artifacts: ../generated/src/index.js)
  • Unit tests: NOT RUN
  • Server starts: N/A (SDK-only change)
  • E2E smoke: FAIL for source/artifact sanity due visible mojibake in published SDK source/docs.

Findings

  1. sdks/python/pmxt/router.py:1-2 and sdks/typescript/pmxt/router.ts:1-2 -- the files have been rewritten with an encoding mismatch/BOM and corrupted em-dashes (ΓÇö). This is a published SDK source/API-doc quality regression and indicates the large generated API reference rewrites may also contain encoding artifacts.
  2. sdks/python/pmxt/__init__.py:274-276 -- SqlResult, SqlMeta, and SqlColumn are imported after the existing __all__ list is closed, but they are not added to __all__. from pmxt import SqlResult works by attribute import, but from pmxt import * and documented public-export introspection will omit the newly advertised SQL types.

PMXT Pipeline Check

  • Field propagation (3-layer): N/A for type-only SDK exports; live SQL response shape was not verified.
  • OpenAPI sync: N/A
  • Financial precision: N/A
  • Type safety: OK for the local TypeScript interfaces, but source encoding/artifact sanity fails.
  • Auth safety: N/A

Semver Impact

minor -- adds a new SDK router SQL helper/type surface.

Risk

I could not verify the hosted SQL endpoint behavior from this environment. Before merge, the source files should be regenerated/edited with clean UTF-8, SQL types added to Python __all__, and a focused hosted-client test should verify the /v0/sql response shape.

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