fix(sqlserver): hoist leading CTEs out of row-limit derived-table wrap (#311)

* test(sql): cover leading CTE row-limit wrapping

* fix(sql): hoist leading CTEs before generic row limits

* fix(sqlserver): hoist leading CTEs before TOP row limits

* test(scan): note relationship limiter coverage boundary

* chore: sync uv.lock to ktx-daemon/ktx-sl 0.13.0
This commit is contained in:
Andrey Avtomonov 2026-06-23 15:03:46 +02:00 committed by GitHub
parent 9f715f93f1
commit c815e10fb3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 275 additions and 5 deletions

View file

@ -404,6 +404,50 @@ describe('KtxSqlServerScanConnector', () => {
await connector.cleanup();
});
it('hoists leading CTEs before applying the SQL Server TOP wrapper', async () => {
const queries: string[] = [];
const request = {
input: vi.fn((_name: string, _value: unknown) => request),
query: vi.fn(async (sql: string): Promise<KtxSqlServerQueryResult> => {
queries.push(sql);
return result([{ value: 1 }], ['value']);
}),
};
const poolFactory: KtxSqlServerPoolFactory = {
createPool: vi.fn(async () => ({
request: () => request,
close: vi.fn(async () => undefined),
})),
};
const connector = new KtxSqlServerScanConnector({
connectionId: 'warehouse',
connection: {
driver: 'sqlserver',
host: 'db.example.test',
database: 'analytics',
username: 'reader',
schema: 'dbo',
},
poolFactory,
});
await expect(
connector.executeReadOnly(
{
connectionId: 'warehouse',
sql: 'WITH child_values AS (SELECT 1 AS value) SELECT value FROM child_values',
maxRows: 1,
},
{ runId: 'scan-run-sqlserver-cte-limit' },
),
).resolves.toMatchObject({ headers: ['value'], rows: [[1]], rowCount: 1 });
expect(queries).toEqual([
'WITH child_values AS (SELECT 1 AS value) SELECT TOP 1 * FROM (SELECT value FROM child_values) AS ktx_query_result',
]);
expect(queries[0]).not.toContain('FROM (WITH');
});
it('limits introspection to tables in tableScope', async () => {
const queries: string[] = [];
const inputs: Array<{ name: string; value: unknown }> = [];

View file

@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest';
import { KtxExpectedError, KtxQueryError } from '../../../src/errors.js';
import {
assertReadOnlySql,
hoistLeadingCte,
limitSqlForExecution,
stripTrailingSqlNoise,
} from '../../../src/context/connections/read-only-sql.js';
@ -81,6 +82,60 @@ describe('assertReadOnlySql', () => {
});
});
describe('hoistLeadingCte', () => {
it('leaves non-CTE SQL untouched', () => {
expect(hoistLeadingCte('select * from orders')).toEqual({
withPrefix: '',
body: 'select * from orders',
});
});
it('splits a single leading CTE from the main query', () => {
expect(hoistLeadingCte('WITH paid AS (SELECT * FROM orders) SELECT * FROM paid')).toEqual({
withPrefix: 'WITH paid AS (SELECT * FROM orders) ',
body: 'SELECT * FROM paid',
});
});
it('splits multiple CTEs, recursive CTEs, column lists, and UNION bodies', () => {
expect(
hoistLeadingCte(
'WITH RECURSIVE nodes(id, parent_id) AS (SELECT id, parent_id FROM roots UNION ALL SELECT child.id, child.parent_id FROM child JOIN nodes ON child.parent_id = nodes.id), totals AS (SELECT count(*) AS total FROM nodes) SELECT total FROM totals',
),
).toEqual({
withPrefix:
'WITH RECURSIVE nodes(id, parent_id) AS (SELECT id, parent_id FROM roots UNION ALL SELECT child.id, child.parent_id FROM child JOIN nodes ON child.parent_id = nodes.id), totals AS (SELECT count(*) AS total FROM nodes) ',
body: 'SELECT total FROM totals',
});
});
it('ignores WITH, commas, and closing parens inside literals, identifiers, and comments', () => {
expect(
hoistLeadingCte(
`WITH tricky AS (
SELECT 'WITH x AS (SELECT 1), nope' AS "value, )"
FROM orders
WHERE note = ')'
/* comment ), next AS (SELECT 1) */
) SELECT * FROM tricky`,
),
).toEqual({
withPrefix: `WITH tricky AS (
SELECT 'WITH x AS (SELECT 1), nope' AS "value, )"
FROM orders
WHERE note = ')'
/* comment ), next AS (SELECT 1) */
) `,
body: 'SELECT * FROM tricky',
});
});
it('falls back to the legacy whole-query body when the CTE is malformed', () => {
const malformed = 'WITH broken AS (SELECT * FROM orders SELECT * FROM broken';
expect(hoistLeadingCte(malformed)).toEqual({ withPrefix: '', body: malformed });
});
});
describe('limitSqlForExecution', () => {
it('wraps compiled SQL and strips trailing semicolons', () => {
expect(limitSqlForExecution('select * from public.orders; ', 25)).toBe(
@ -98,6 +153,18 @@ describe('limitSqlForExecution', () => {
);
});
it('hoists leading CTEs before applying the generic LIMIT wrapper', () => {
expect(limitSqlForExecution('WITH paid AS (SELECT * FROM orders) SELECT * FROM paid', 25)).toBe(
'WITH paid AS (SELECT * FROM orders) select * from (SELECT * FROM paid) as ktx_query_result limit 25',
);
});
it('keeps the generic wrapper byte-identical for non-CTE SQL', () => {
expect(limitSqlForExecution('select id, status from public.orders', 25)).toBe(
'select * from (select id, status from public.orders) as ktx_query_result limit 25',
);
});
it('drops a trailing semicolon followed by a comment so the subquery stays valid', () => {
// The single-statement gate accepts `select 1; -- done`; without stripping
// the terminator the wrapper would embed `select 1; -- done` and comment out

View file

@ -8,6 +8,8 @@ import { profileKtxRelationshipSchema } from '../../../src/context/scan/relation
import { validateKtxRelationshipDiscoveryCandidates } from '../../../src/context/scan/relationship-validation.js';
import type { KtxQueryResult, KtxReadOnlyQueryInput, KtxScanContext } from '../../../src/context/scan/types.js';
// This harness runs SQL directly through SQLite; row-limit wrapper coverage lives
// in read-only-sql.test.ts and the SQL Server connector test.
class InMemorySqliteExecutor {
readonly db = new Database(':memory:');
queryCount = 0;