Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions .agents/skills/testing-fixtures/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,27 @@ This generates two tests per case (pretty and non-pretty) and uses Jest snapshot
3. Run `npx jest` to verify all tests pass
4. Commit the `.sql` file, `generated.json`, and any new generated test files in `__tests__/kitchen-sink/`

## PostgreSQL Regression Corpus (`__fixtures__/regress/`)

A ratchet over the SQL that libpg_query vendors from PostgreSQL (`test/sql/postgres_regress`,
`test/sql/plpgsql_regress`) plus its own test SQL (`test/sql/deparse*`, `test/plpgsql_samples.sql`):

- `statements.json.gz`: unique parseable statements keyed like `generated.json`
(`postgres_regress/arrays-12.sql`), plus the libpg_query commit they came from. Built by
`cd packages/deparser && npm run fixtures:regress -- /path/to/libpg_query`. Check out the
`x-publish.libpgQueryCommit` of the pinned `libpg-query` release first.
- `known-failures.deparser.json` / `known-failures.plpgsql-deparser.json`: statements that do not
round-trip yet.
- Tests: `packages/deparser/__tests__/regress/regress-corpus.test.ts` (one test per source file,
pretty and non-pretty, via `TestUtils.expectAstMatch`) and
`packages/plpgsql-deparser/__tests__/regress-corpus.test.ts`. They run in the normal `test` script.

A new failure fails the test, and so does a known failure that now passes. After fixing deparser
bugs (or bumping `libpg-query` and regenerating the corpus), refresh the lists with
`UPDATE_KNOWN_FAILURES=1 npx jest __tests__/regress` (deparser) and
`UPDATE_KNOWN_FAILURES=1 npx jest __tests__/regress-corpus` (plpgsql-deparser), then review the diff.
Bug fixes still get a kitchen-sink fixture; the corpus only guards against regressions.

## PL/pgSQL Fixtures (`plpgsql-deparser`)

The PL/pgSQL deparser has its own fixture pipeline:
Expand Down
690 changes: 690 additions & 0 deletions __fixtures__/regress/known-failures.deparser.json

Large diffs are not rendered by default.

74 changes: 74 additions & 0 deletions __fixtures__/regress/known-failures.plpgsql-deparser.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
[
"plpgsql_samples-28.sql",
"plpgsql_samples-31.sql",
"plpgsql_samples-33.sql",
"plpgsql_samples-34.sql",
"plpgsql_samples-36.sql",
"postgres_regress/case-43.sql",
"postgres_regress/case-46.sql",
"postgres_regress/case-53.sql",
"postgres_regress/case-54.sql",
"postgres_regress/copy2-2.sql",
"postgres_regress/copy2-3.sql",
"postgres_regress/create_index-535.sql",
"postgres_regress/create_index-536.sql",
"postgres_regress/create_operator-17.sql",
"postgres_regress/event_trigger-79.sql",
"postgres_regress/explain-46.sql",
"postgres_regress/plpgsql-22.sql",
"postgres_regress/plpgsql-24.sql",
"postgres_regress/plpgsql-28.sql",
"postgres_regress/plpgsql-30.sql",
"postgres_regress/plpgsql-34.sql",
"postgres_regress/plpgsql-38.sql",
"postgres_regress/plpgsql-40.sql",
"postgres_regress/plpgsql-44.sql",
"postgres_regress/plpgsql-46.sql",
"postgres_regress/plpgsql-48.sql",
"postgres_regress/plpgsql-54.sql",
"postgres_regress/plpgsql-60.sql",
"postgres_regress/plpgsql-64.sql",
"postgres_regress/plpgsql-66.sql",
"postgres_regress/plpgsql-68.sql",
"postgres_regress/plpgsql-70.sql",
"postgres_regress/plpgsql-72.sql",
"postgres_regress/plpgsql-74.sql",
"postgres_regress/plpgsql-76.sql",
"postgres_regress/plpgsql-80.sql",
"postgres_regress/plpgsql-81.sql",
"postgres_regress/plpgsql-82.sql",
"postgres_regress/plpgsql-88.sql",
"postgres_regress/plpgsql-89.sql",
"postgres_regress/plpgsql-90.sql",
"postgres_regress/plpgsql-91.sql",
"postgres_regress/plpgsql-92.sql",
"postgres_regress/plpgsql-283.sql",
"postgres_regress/plpgsql-286.sql",
"postgres_regress/plpgsql-289.sql",
"postgres_regress/plpgsql-291.sql",
"postgres_regress/plpgsql-293.sql",
"postgres_regress/plpgsql-295.sql",
"postgres_regress/plpgsql-298.sql",
"postgres_regress/plpgsql-364.sql",
"postgres_regress/plpgsql-365.sql",
"postgres_regress/plpgsql-387.sql",
"postgres_regress/plpgsql-424.sql",
"postgres_regress/plpgsql-504.sql",
"postgres_regress/plpgsql-768.sql",
"postgres_regress/plpgsql-854.sql",
"postgres_regress/plpgsql-857.sql",
"postgres_regress/plpgsql-875.sql",
"postgres_regress/plpgsql-880.sql",
"postgres_regress/privileges-766.sql",
"postgres_regress/rangefuncs-128.sql",
"postgres_regress/rangefuncs-137.sql",
"postgres_regress/rangefuncs-168.sql",
"postgres_regress/rowsecurity-23.sql",
"postgres_regress/rowsecurity-1068.sql",
"postgres_regress/select_views-5.sql",
"postgres_regress/stats_ext-741.sql",
"postgres_regress/temp-49.sql",
"postgres_regress/transactions-239.sql",
"postgres_regress/transactions-244.sql",
"postgres_regress/triggers-261.sql"
]
Binary file added __fixtures__/regress/statements.json.gz
Binary file not shown.
68 changes: 68 additions & 0 deletions packages/deparser/__tests__/regress/regress-corpus.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* Round-trips the PostgreSQL regression corpus (__fixtures__/regress/statements.json.gz,
* built by `npm run fixtures:regress`) through parse -> deparse -> parse, pretty and not.
*
* known-failures.deparser.json is a ratchet: a new failure fails the test, and so does a listed
* statement that now passes. After a deliberate change, refresh it with
* UPDATE_KNOWN_FAILURES=1 npx jest __tests__/regress
*/
import * as fs from 'fs';
import * as path from 'path';
import * as zlib from 'zlib';

import { TestUtils } from '../../test-utils';

const FIXTURE_DIR = path.join(__dirname, '../../../../__fixtures__/regress');
const KNOWN_FAILURES_FILE = path.join(FIXTURE_DIR, 'known-failures.deparser.json');
const UPDATE = process.env.UPDATE_KNOWN_FAILURES === '1';

const { statements } = JSON.parse(
zlib.gunzipSync(fs.readFileSync(path.join(FIXTURE_DIR, 'statements.json.gz'))).toString('utf8')
) as { statements: Record<string, string> };
const knownFailures: Record<string, string[]> = JSON.parse(fs.readFileSync(KNOWN_FAILURES_FILE, 'utf8'));

const byFile = new Map<string, string[]>();
for (const key of Object.keys(statements)) {
const file = key.replace(/-\d+\.sql$/, '.sql');
byFile.set(file, [...(byFile.get(file) ?? []), key]);
}

const observed: Record<string, string[]> = {};
const ran = new Set<string>();
const utils = new TestUtils();

describe('regress corpus round-trip', () => {
beforeAll(() => {
for (const level of ['error', 'warn', 'log'] as const) jest.spyOn(console, level).mockImplementation(() => {});
});
afterAll(() => {
jest.restoreAllMocks();
if (UPDATE) {
// Only replace entries for files that ran, so a filtered run (-t, --shard) keeps the rest
const merged = { ...knownFailures };
for (const file of ran) delete merged[file];
Object.assign(merged, observed);
for (const file of Object.keys(merged)) if (!byFile.has(file)) delete merged[file];
const sorted = Object.fromEntries(Object.entries(merged).sort(([a], [b]) => a.localeCompare(b)));
fs.writeFileSync(KNOWN_FAILURES_FILE, JSON.stringify(sorted, null, 2) + '\n');
}
Comment thread
tenki-reviewer[bot] marked this conversation as resolved.
});

it('lists only files that are in the corpus', () => {
expect(Object.keys(knownFailures).filter((file) => !byFile.has(file))).toEqual([]);
});

it.each([...byFile.keys()])('%s', async (file) => {
const failing: string[] = [];
for (const key of byFile.get(file)!) {
try {
await utils.expectAstMatch(key, statements[key]);
} catch {
failing.push(key);
}
}
ran.add(file);
if (failing.length) observed[file] = failing;
if (!UPDATE) expect(failing).toEqual(knownFailures[file] ?? []);
});
});
1 change: 1 addition & 0 deletions packages/deparser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"fixtures:sql": "ts-node scripts/make-fixtures-sql.ts",
"fixtures": "ts-node scripts/make-fixtures.ts",
"fixtures:upstream-diff": "ts-node scripts/make-upstream-diff.ts",
"fixtures:regress": "ts-node scripts/make-regress-fixtures.ts",
"lint": "eslint . --fix",
"test": "jest",
"test:watch": "jest --watch",
Expand Down
110 changes: 110 additions & 0 deletions packages/deparser/scripts/make-regress-fixtures.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
#!/usr/bin/env ts-node
/**
* Builds __fixtures__/regress/statements.json.gz from a libpg_query checkout:
* the PostgreSQL regression SQL it vendors (test/sql/postgres_regress, plpgsql_regress)
* plus its own test SQL (test/sql/deparse*, test/plpgsql_samples.sql).
*
* npm run fixtures:regress -- /path/to/libpg_query
*
* Check out the libpg_query commit that the pinned libpg-query release was built from
* (x-publish.libpgQueryCommit in libpg-query-node versions/18/package.json).
*/
import { execFileSync } from 'child_process';
import * as fs from 'fs';
import { sync as globSync } from 'glob';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 bug · medium

glob undeclared in deparser package

packages/deparser/scripts/make-regress-fixtures.ts:14 imports { sync as globSync } from 'glob', but glob is only declared in the root package.json and packages/pgsql-cli/package.json — not in packages/deparser/package.json. Under pnpm's default isolated node_modules layout, the new fixtures:regress script fails at import time with "Cannot find module 'glob'", so the corpus cannot be regenerated from a fresh clone.

📋 Prompt for AI Agents

In packages/deparser/package.json, add a devDependencies entry for glob matching the version already used at the repo root (^11.0.0), because packages/deparser/scripts/make-regress-fixtures.ts line 14 imports globSync from glob and pnpm will not resolve an undeclared transitive dependency from a package's own scripts.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glob comes from the root package.json (^11.0.0) and is hoisted. The existing make-fixtures.ts, make-fixtures-ast.ts and make-fixtures-sql.ts scripts import it the same way, so I kept the convention.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The candidate diff for 'packages/deparser/package.json' only shows the addition of the 'fixtures:regress' script entry — it does not include a 'glob' entry under 'devDependencies'. The fix described (adding 'glob': '^11.0.0' to devDependencies) is not visible in any of the evidence provided. The user's claim that hoisting from the root covers this is unreliable under pnpm's isolated node_modules layout, and the other scripts cited as precedent have the same latent issue. The original finding remains unresolved.

import { loadModule, parseSync, scanSync } from 'libpg-query';
import * as path from 'path';
import * as zlib from 'zlib';

import { generateStatementKey } from './statement-splitter';

const OUT_FILE = path.join(__dirname, '../../../__fixtures__/regress/statements.json.gz');
const SOURCES = [
'test/sql/postgres_regress/*.sql',
'test/sql/plpgsql_regress/*.sql',
'test/sql/deparse/*.sql',
'test/sql/deparse-depesz/**/*.{sql,psql}',
'test/plpgsql_samples.sql',
];

// Split on top-level semicolons using the real scanner, so string/dollar quoting and
// comments are handled. CREATE RULE (...; ...) and BEGIN ATOMIC ... END bodies stay whole.
function splitWithScanner(sql: string): string[] {
const buf = Buffer.from(sql, 'utf8');
const tokens = scanSync(sql).tokens;
const out: string[] = [];
let start = 0;
let parens = 0;
let atomic = 0;
let cases = 0;
let prev = '';
for (const tok of tokens) {
const word = tok.text.toUpperCase();
if (tok.text === '(') parens++;
else if (tok.text === ')') parens = Math.max(0, parens - 1);
else if (word === 'ATOMIC' && prev === 'BEGIN') atomic++;
else if (atomic > 0 && word === 'CASE') cases++;
else if (atomic > 0 && word === 'END') {
if (cases > 0) cases--;
else atomic--;
} else if (tok.text === ';' && parens === 0 && atomic === 0) {
Comment thread
tenki-reviewer[bot] marked this conversation as resolved.
out.push(buf.subarray(start, tok.start).toString('utf8'));
start = tok.end;
}
if (tok.tokenName !== 'SQL_COMMENT' && tok.tokenName !== 'C_COMMENT') prev = word;
}
out.push(buf.subarray(start).toString('utf8'));
return out;
}

function splitStatements(sql: string): string[] {
let pieces: string[];
try {
pieces = splitWithScanner(sql);
} catch {
// e.g. strings.sql has intentionally invalid escapes the scanner rejects
pieces = sql.split(/;[ \t]*\r?\n/);
}
return pieces
.map((s) => s.replace(/^(\s*--[^\n]*\n|\s+)+/, '').trim())
.filter((s) => s.length > 0);
Comment on lines +64 to +70

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 bug · medium

Fallback regex split corrupts dollar-quoted strings

When scanSync rejects a file (e.g. strings.sql with intentionally invalid escapes), splitStatements falls back to sql.split(/;[ \t]*\r? /) (make-regress-fixtures.ts:66), which is quote- and dollar-quote-blind. Any dollar-quoted body ($$...; ...$$, $_$...$_$) or a string literal containing ; is cut in half; each fragment then fails parseSync and is silently counted as 'skipped' (make-regress-fixtures.ts:92), so whole functions vanish from the corpus with no warning. The fragments also shift the per-file index i, so subsequent keys contain different statements than the generator's numbering implies.

📋 Prompt for AI Agents

In packages/deparser/scripts/make-regress-fixtures.ts, in splitStatements (lines 60-71), make the catch-block fallback dollar-quote aware: before sql.split(/;[ \t]*\r?\n/), replace every dollar-quoted span (regex /($[A-Za-z_]$)[\s\S]?\1/g) with a placeholder of identical byte length, split, then restore — or record the file as skipped with a log line instead of using the regex fallback. Otherwise fragments of a split dollar-quoted body fail parseSync and are silently dropped from the regress corpus, hiding entire functions from the fixture set.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback only runs for files the scanner rejects outright (just strings.sql at this pin). A piece is kept only if it parses as a full statement on its own, so a dollar-quoted body split in the middle gets dropped and never stored corrupted. Left as is.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback sql.split(/;[ \t]*\r?\n/) is still present unchanged in the current diff (lines 63-67 of the catch block). The argument that 'a split fragment gets dropped and never stored corrupted' does not address the finding: the problem is that whole statements (e.g. entire PL/pgSQL functions with dollar-quoted bodies) are silently lost from the fixture corpus, and the index i is shifted for all subsequent statements in that file, corrupting their keys. The fix requires either making the fallback dollar-quote-aware (placeholder-swap approach) or logging and skipping the file entirely — neither is present here.

}

async function main() {
const root = path.resolve(process.argv[2] || process.env.LIBPG_QUERY_DIR || '');
if (!fs.existsSync(path.join(root, 'test/sql/postgres_regress'))) {
throw new Error('usage: npm run fixtures:regress -- /path/to/libpg_query');
}
await loadModule();
const commit = execFileSync('git', ['-C', root, 'rev-parse', 'HEAD']).toString().trim();
const files = SOURCES.flatMap((pattern) => globSync(pattern, { cwd: root })).sort();
const seen = new Set<string>();
const statements: Record<string, string> = {};
let skipped = 0;
for (const rel of files) {
const relKey = rel.replace(/^test\/(sql\/)?/, '').replace(/\.psql$/, '.sql');
splitStatements(fs.readFileSync(path.join(root, rel), 'utf8')).forEach((sql, i) => {
if (seen.has(sql)) return;
seen.add(sql);
try {
if (!parseSync(sql).stmts?.length) return;
} catch {
skipped++;
return;
}
statements[generateStatementKey(relKey, i)] = sql;
});
}
const json = JSON.stringify({ libpgQueryCommit: commit, statements });
fs.mkdirSync(path.dirname(OUT_FILE), { recursive: true });
fs.writeFileSync(OUT_FILE, zlib.gzipSync(json, { level: 9 }));
console.log(
`${files.length} files, ${Object.keys(statements).length} statements (${skipped} unparseable skipped) ` +
`-> ${path.relative(process.cwd(), OUT_FILE)} (${fs.statSync(OUT_FILE).size} bytes)`
);
}

main().catch((err) => {
console.error(err);
process.exit(1);
});
63 changes: 63 additions & 0 deletions packages/plpgsql-deparser/__tests__/regress-corpus.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/**
* Round-trips the PL/pgSQL functions and DO blocks of the PostgreSQL regression corpus
* (__fixtures__/regress/statements.json.gz, built by `npm run fixtures:regress` in
* packages/deparser) through parse -> deparse -> parse.
*
* known-failures.plpgsql-deparser.json is a ratchet: a new failure fails the test, and so
* does a listed statement that now passes. After a deliberate change, refresh it with
* UPDATE_KNOWN_FAILURES=1 npx jest __tests__/regress-corpus
*/
import * as fs from 'fs';
import { loadModule, parsePlPgSQLSync } from 'libpg-query';
import * as path from 'path';
import * as zlib from 'zlib';

import { PLpgSQLTestUtils } from '../test-utils';

const FIXTURE_DIR = path.join(__dirname, '../../../__fixtures__/regress');
const KNOWN_FAILURES_FILE = path.join(FIXTURE_DIR, 'known-failures.plpgsql-deparser.json');
const UPDATE = process.env.UPDATE_KNOWN_FAILURES === '1';

const { statements } = JSON.parse(
zlib.gunzipSync(fs.readFileSync(path.join(FIXTURE_DIR, 'statements.json.gz'))).toString('utf8')
) as { statements: Record<string, string> };
const knownFailures: string[] = JSON.parse(fs.readFileSync(KNOWN_FAILURES_FILE, 'utf8'));

const candidates = Object.keys(statements).filter((key) =>
/language\s+'?plpgsql|^\s*do\s/i.test(statements[key])
);

describe('regress corpus PL/pgSQL round-trip', () => {
const utils = new PLpgSQLTestUtils();

beforeAll(async () => {
await loadModule();
for (const level of ['error', 'warn', 'log'] as const) jest.spyOn(console, level).mockImplementation(() => {});
});
afterAll(() => jest.restoreAllMocks());

it('matches known failures', async () => {
const failing: string[] = [];
let tested = 0;
for (const key of candidates) {
try {
const parsed = parsePlPgSQLSync(statements[key]) as any;
if (!parsed.plpgsql_funcs?.length) continue;
} catch {
continue;
}
tested++;
try {
await utils.expectAstMatch(key, statements[key]);
} catch {
failing.push(key);
}
}
expect(tested).toBeGreaterThan(500);
if (UPDATE) {
fs.writeFileSync(KNOWN_FAILURES_FILE, JSON.stringify(failing, null, 2) + '\n');
} else {
expect(failing).toEqual(knownFailures);
}
});
});
Loading