From f5ef7fb88a3b54cf1bdd2c29a376681ebbed3134 Mon Sep 17 00:00:00 2001 From: Maoyin Sun Date: Wed, 1 Jul 2026 19:03:11 +0200 Subject: [PATCH] feat(grading): support in-memory PGlite backend for grading endpoints --- .../scripts/smoke-test.sh | 93 +++++++ .../comparators/execution-plan-comparator.ts | 24 +- .../src/grading/grading-controller.ts | 235 +++++++++++------- .../src/grading/query-grading-service.ts | 10 +- .../src/grading/result-set-comparator.ts | 54 +++- .../src/shared/i18n/messages.ts | 5 - .../src/shared/utils/database-utils.ts | 17 ++ .../grading/pglite-grading-controller.test.ts | 212 +++++++++++----- .../test/shared/pglite-row-query-fn.test.ts | 151 +++++++++++ 9 files changed, 621 insertions(+), 180 deletions(-) create mode 100644 services/sql-assessment-service/test/shared/pglite-row-query-fn.test.ts diff --git a/services/sql-assessment-service/scripts/smoke-test.sh b/services/sql-assessment-service/scripts/smoke-test.sh index 4e5a73e..00647b4 100755 --- a/services/sql-assessment-service/scripts/smoke-test.sh +++ b/services/sql-assessment-service/scripts/smoke-test.sh @@ -10,6 +10,8 @@ # # PGlite path (no external DB needed): # PGLITE_DB_ID databaseId to use (default: smoke-db) +# (also covers the grading endpoints — /api/grading/grade and +# /api/grading/compare/* now run against the in-process PGlite backend) # # Init-SQL-file feature (optional — tests the PGLITE_INIT_SQL_FILE / --init-sql-file feature): # SMOKE_TEST_INIT_SQL Set to '1' to enable (server must be started with PGLITE_INIT_SQL_FILE @@ -233,6 +235,96 @@ INSERT INTO orders (product_id, quantity) VALUES (1, 5), (2, 3);' '{connectionInfo:{type:"pglite",databaseId:$id},query:"SELECT * FROM categories"}')" } +# ---- PGlite grading tests -------------------------------------------------- +# Grading endpoints now run against the in-process PGlite backend (no external +# PostgreSQL required). These exercise /api/grading/grade and the three +# /api/grading/compare/* endpoints end-to-end. + +pglite_grading_tests() { + echo "" + echo "── PGlite grading path (${BASE_URL}) ──────────────────────────────────" + + local GRADE_DB_ID="smoke-grading-db" + local DDL + DDL='CREATE TABLE products (id SERIAL PRIMARY KEY, name TEXT NOT NULL, price NUMERIC(10,2)); +INSERT INTO products (name, price) VALUES ('\''Widget'\'', 9.99), ('\''Gadget'\'', 19.99), ('\''Gizmo'\'', 29.99);' + + local REF='SELECT name FROM products WHERE price > 15 ORDER BY name' + local CORRECT='SELECT name FROM products WHERE price > 15 ORDER BY name' + local WRONG='SELECT name FROM products ORDER BY name' + + # 1. Register the grading DB + post "grading — analyze-database (seed DDL)" "200" \ + "${BASE_URL}/api/database/analyze-database" \ + "$(jq -n --arg id "$GRADE_DB_ID" --arg sql "$DDL" \ + '{connectionInfo:{type:"pglite",databaseId:$id,sqlContent:$sql}}')" + + # 2. Full grade — identical query → equivalent:true, grade 7 + post_body_contains "grading — grade identical query (equivalent)" "200" \ + "${BASE_URL}/api/grading/grade" \ + "$(jq -n --arg id "$GRADE_DB_ID" --arg ref "$REF" --arg stu "$CORRECT" \ + '{connectionInfo:{type:"pglite",databaseId:$id},gradingRequest:{referenceQuery:$ref,studentQuery:$stu}}')" \ + '"equivalent":true' + + # 3. Full grade — wrong query → not equivalent + post_body_contains "grading — grade wrong query (not equivalent)" "200" \ + "${BASE_URL}/api/grading/grade" \ + "$(jq -n --arg id "$GRADE_DB_ID" --arg ref "$REF" --arg stu "$WRONG" \ + '{connectionInfo:{type:"pglite",databaseId:$id},gradingRequest:{referenceQuery:$ref,studentQuery:$stu}}')" \ + '"equivalent":false' + + # 4. compare/result-set — matching sets → match:true + post_body_contains "grading — compare/result-set (match)" "200" \ + "${BASE_URL}/api/grading/compare/result-set" \ + "$(jq -n --arg id "$GRADE_DB_ID" --arg ref "$REF" --arg stu "$CORRECT" \ + '{connectionInfo:{type:"pglite",databaseId:$id},referenceQuery:$ref,studentQuery:$stu}')" \ + '"match":true' + + # 5. compare/result-set — differing sets → match:false + post_body_contains "grading — compare/result-set (mismatch)" "200" \ + "${BASE_URL}/api/grading/compare/result-set" \ + "$(jq -n --arg id "$GRADE_DB_ID" --arg ref "$REF" --arg stu "$WRONG" \ + '{connectionInfo:{type:"pglite",databaseId:$id},referenceQuery:$ref,studentQuery:$stu}')" \ + '"match":false' + + # 6. compare/ast — same column list → columnsMatch:true + post_body_contains "grading — compare/ast (columns match)" "200" \ + "${BASE_URL}/api/grading/compare/ast" \ + "$(jq -n --arg id "$GRADE_DB_ID" --arg ref "$REF" --arg stu "$CORRECT" \ + '{connectionInfo:{type:"pglite",databaseId:$id},referenceQuery:$ref,studentQuery:$stu}')" \ + '"columnsMatch":true' + + # 7. compare/execution-plan — identical plans → plansMatch:true (proves EXPLAIN runs on PGlite) + post_body_contains "grading — compare/execution-plan (plans match)" "200" \ + "${BASE_URL}/api/grading/compare/execution-plan" \ + "$(jq -n --arg id "$GRADE_DB_ID" --arg ref "$REF" --arg stu "$CORRECT" \ + '{connectionInfo:{type:"pglite",databaseId:$id},referenceQuery:$ref,studentQuery:$stu}')" \ + '"plansMatch":true' + + # 8. compare/execution-plan — differing WHERE → plansMatch:false + post_body_contains "grading — compare/execution-plan (plans differ)" "200" \ + "${BASE_URL}/api/grading/compare/execution-plan" \ + "$(jq -n --arg id "$GRADE_DB_ID" --arg ref "$REF" --arg stu "$WRONG" \ + '{connectionInfo:{type:"pglite",databaseId:$id},referenceQuery:$ref,studentQuery:$stu}')" \ + '"plansMatch":false' + + # 9. The shared PGlite instance survives grading — a follow-up query still works + post "grading — DB still usable after grading (shared instance not closed)" "200" \ + "${BASE_URL}/api/query/execute" \ + "$(jq -n --arg id "$GRADE_DB_ID" \ + '{connectionInfo:{type:"pglite",databaseId:$id},query:"SELECT COUNT(*) AS c FROM products"}')" + + # 10. grade against an unregistered databaseId → 400 (only meaningful without init file) + if [[ -z "${SMOKE_TEST_INIT_SQL:-}" ]]; then + post "grading — grade unregistered databaseId (expect 400)" "400" \ + "${BASE_URL}/api/grading/grade" \ + "$(jq -n --arg ref "$REF" --arg stu "$CORRECT" \ + '{connectionInfo:{type:"pglite",databaseId:"__not_registered_grading__"},gradingRequest:{referenceQuery:$ref,studentQuery:$stu}}')" + else + green " SKIP grading — unregistered databaseId (init-SQL-file auto-initialises any databaseId)" + fi +} + # ---- Init-SQL-file tests (optional) --------------------------------------- pglite_init_sql_file_tests() { @@ -460,6 +552,7 @@ done echo " OK" pglite_tests +pglite_grading_tests pglite_init_sql_file_tests postgres_tests cli_tests diff --git a/services/sql-assessment-service/src/grading/comparators/execution-plan-comparator.ts b/services/sql-assessment-service/src/grading/comparators/execution-plan-comparator.ts index 689b39d..2e83ece 100644 --- a/services/sql-assessment-service/src/grading/comparators/execution-plan-comparator.ts +++ b/services/sql-assessment-service/src/grading/comparators/execution-plan-comparator.ts @@ -1,5 +1,5 @@ -import { DataSource } from 'typeorm'; import { AST, Join } from 'node-sql-parser'; +import { RowQueryFn } from '../../shared/utils/database-utils'; import { IParsedExecutionPlan, ParsedSubplan, @@ -60,27 +60,19 @@ export class ExecutionPlanComparator { referenceAST: AST, studentAliasMap: Record, referenceAliasMap: Record, - dataSource: DataSource, + runQuery: RowQueryFn, studentQuery: string, referenceQuery: string, lang: SupportedLanguage = 'en', ): Promise { const executionPlan: ExecutionPlanFeedback = {}; - const queryRunner = dataSource.createQueryRunner(); - let studentRawPlan: any; - let referenceRawPlan: any; - - try { - studentRawPlan = await queryRunner.query( - `EXPLAIN (FORMAT JSON) ${studentQuery}`, - ); - referenceRawPlan = await queryRunner.query( - `EXPLAIN (FORMAT JSON) ${referenceQuery}`, - ); - } finally { - await queryRunner.release(); - } + const studentRawPlan = await runQuery( + `EXPLAIN (FORMAT JSON) ${studentQuery}`, + ); + const referenceRawPlan = await runQuery( + `EXPLAIN (FORMAT JSON) ${referenceQuery}`, + ); if (!studentRawPlan || !referenceRawPlan) { executionPlan.planRetrieval = { diff --git a/services/sql-assessment-service/src/grading/grading-controller.ts b/services/sql-assessment-service/src/grading/grading-controller.ts index beac2e7..43b8a10 100644 --- a/services/sql-assessment-service/src/grading/grading-controller.ts +++ b/services/sql-assessment-service/src/grading/grading-controller.ts @@ -5,11 +5,16 @@ import { AST, Parser } from 'node-sql-parser'; import { connectToDatabase, generateDatabaseKey, + generatePGliteKey, + makePGliteRowQueryFn, + makeRowQueryFn, + RowQueryFn, } from '../shared/utils/database-utils'; import { isDatabaseRegistered, validateConnectionInfo, } from '../shared/utils/validation'; +import { pgliteInstances } from '../database/internal-memory'; import { SQLQueryGradingService } from './query-grading-service'; import { ComparisonResult, @@ -35,9 +40,15 @@ import { DatabaseService } from '../database/database-service'; // --------------------------------------------------------------------------- interface ValidatedConnection { - connectionInfo: PostgresConnectionOptions; databaseKey: string; - dataSource: DataSource; + /** Backend-agnostic query executor (TypeORM Postgres or in-process PGlite). */ + runQuery: RowQueryFn; + /** + * Releases any backend resources acquired for this request. For Postgres + * this destroys the DataSource; for the shared PGlite instance it is a + * no-op (the instance is long-lived and must never be closed here). + */ + cleanup: () => Promise; lang: SupportedLanguage; } @@ -298,20 +309,76 @@ export class GradingController { // ---- PGlite branch -------------------------------------------------- if ((body.connectionInfo as any)?.type === 'pglite') { - res - .status(400) - .json({ message: t('GRADING_PGLITE_NOT_SUPPORTED', lang) }); - return null; + return this.resolvePGliteConnection(body.connectionInfo as any, lang, res); } // --------------------------------------------------------------------- - const validationError = validateConnectionInfo(body.connectionInfo, lang); + return this.resolvePostgresConnection(body.connectionInfo, lang, res); + } + + // ========================================================================= + // Backend resolution helpers + // ========================================================================= + + /** + * Resolves a validated {@link ValidatedConnection} for an in-process PGlite + * database. The PGlite instance is expected to already be registered + * (auto-analyze runs before this helper). Writes a 400 response and + * returns null on failure. + * + * The returned `cleanup` is intentionally a no-op: the PGlite instance is + * shared and long-lived, so it must never be closed here. + */ + private resolvePGliteConnection( + connectionInfo: any, + lang: SupportedLanguage, + res: Response, + ): ValidatedConnection | null { + const databaseId = connectionInfo?.databaseId; + if (!databaseId || typeof databaseId !== 'string') { + res.status(400).json({ message: t('INVALID_CONNECTION_INFO', lang) }); + return null; + } + + const databaseKey = generatePGliteKey(databaseId); + if (!isDatabaseRegistered(databaseKey)) { + res.status(400).json({ message: t('DATABASE_NOT_REGISTERED', lang) }); + return null; + } + + const db = pgliteInstances.get(databaseId); + if (!db) { + res.status(400).json({ message: t('DATABASE_NOT_REGISTERED', lang) }); + return null; + } + + return { + databaseKey, + runQuery: makePGliteRowQueryFn(db), + cleanup: async () => { + /* shared PGlite instance — never closed here */ + }, + lang, + }; + } + + /** + * Resolves a validated {@link ValidatedConnection} backed by a TypeORM + * DataSource for a PostgreSQL connection. Writes a 400 response and + * returns null on failure. + */ + private async resolvePostgresConnection( + connectionInfo: PostgresConnectionOptions, + lang: SupportedLanguage, + res: Response, + ): Promise { + const validationError = validateConnectionInfo(connectionInfo, lang); if (validationError) { res.status(400).json({ message: validationError }); return null; } - const { host, port, schema } = body.connectionInfo; + const { host, port, schema } = connectionInfo; const databaseKey = generateDatabaseKey(host!, port!, schema!); if (!isDatabaseRegistered(databaseKey)) { @@ -322,7 +389,7 @@ export class GradingController { let dataSource: DataSource; let isConnected: boolean; try { - dataSource = new DataSource(body.connectionInfo); + dataSource = new DataSource(connectionInfo); isConnected = await connectToDatabase(dataSource); } catch { res.status(400).json({ message: t('UNABLE_TO_CONNECT', lang) }); @@ -335,9 +402,11 @@ export class GradingController { } return { - connectionInfo: body.connectionInfo, databaseKey, - dataSource, + runQuery: makeRowQueryFn(dataSource), + cleanup: async () => { + await dataSource.destroy(); + }, lang, }; } @@ -414,77 +483,71 @@ export class GradingController { } // --------------------------------------------------------------------- - // ---- PGlite branch -------------------------------------------------- - if ((connectionInfo as any)?.type === 'pglite') { - return res - .status(400) - .json({ message: t('GRADING_PGLITE_NOT_SUPPORTED', lang) }); - } - // --------------------------------------------------------------------- - - const validationError = validateConnectionInfo(connectionInfo, lang); - if (validationError) { - return res.status(400).json({ message: validationError }); - } + // Resolve a backend-agnostic executor for either PGlite or PostgreSQL. + let databaseKey: string; + let runQuery: RowQueryFn; + let cleanup: () => Promise; + let schema: string; - const databaseKey = generateDatabaseKey( - connectionInfo.host!, - connectionInfo.port!, - connectionInfo.schema!, - ); - if (!isDatabaseRegistered(databaseKey)) { - return res - .status(400) - .json({ message: t('DATABASE_NOT_REGISTERED', lang) }); + if ((connectionInfo as any)?.type === 'pglite') { + const validated = this.resolvePGliteConnection( + connectionInfo as any, + lang, + res, + ); + if (!validated) return res; + databaseKey = validated.databaseKey; + runQuery = validated.runQuery; + cleanup = validated.cleanup; + schema = 'public'; + } else { + const validated = await this.resolvePostgresConnection( + connectionInfo, + lang, + res, + ); + if (!validated) return res; + databaseKey = validated.databaseKey; + runQuery = validated.runQuery; + cleanup = validated.cleanup; + schema = connectionInfo.schema!; } - let dataSource: DataSource; - let isConnected: boolean; + const gradingRequest = gradingRequestOptions.gradingRequest; try { - dataSource = new DataSource(connectionInfo); - isConnected = await connectToDatabase(dataSource); - } catch { - return res.status(400).json({ message: t('UNABLE_TO_CONNECT', lang) }); - } + const resolvedReferenceQuery = this.resolveReferenceQuery( + gradingRequest.studentQuery, + gradingRequest.referenceQuery, + gradingRequest.referenceQueries, + ); - const gradingRequest = gradingRequestOptions.gradingRequest; - if (isConnected) { - try { - const resolvedReferenceQuery = this.resolveReferenceQuery( - gradingRequest.studentQuery, - gradingRequest.referenceQuery, - gradingRequest.referenceQueries, - ); - - const comparisonResult = await this.queryGradingService.gradeQuery( - resolvedReferenceQuery, - gradingRequest.studentQuery, - dataSource, - databaseKey, - lang, - ); + const comparisonResult = await this.queryGradingService.gradeQuery( + resolvedReferenceQuery, + gradingRequest.studentQuery, + runQuery, + databaseKey, + lang, + ); - await this.appendTaskDescription( - comparisonResult, - gradingRequest.studentQuery, - connectionInfo.schema!, - databaseKey, - lang, - gradingRequestOptions.generationStrategy, - gradingRequestOptions.gptOption, - ); + await this.appendTaskDescription( + comparisonResult, + gradingRequest.studentQuery, + schema, + databaseKey, + lang, + gradingRequestOptions.generationStrategy, + gradingRequestOptions.gptOption, + ); - await dataSource.destroy(); - return res.status(200).json({ comparisonResult }); - } catch (error) { - console.log(error); - await dataSource.destroy(); - return res.status(500).json({ - message: t('GRADING_FAILED_WITH_ERROR', lang, String(error)), - }); - } + await cleanup(); + return res.status(200).json({ comparisonResult }); + } catch (error) { + console.log(error); + await cleanup(); + return res.status(500).json({ + message: t('GRADING_FAILED_WITH_ERROR', lang, String(error)), + }); } - return res.status(500).json({ message: t('GRADING_FAILED', lang) }); } // ========================================================================= @@ -602,7 +665,7 @@ export class GradingController { const validated = await this.validateAndConnect(req, res); if (!validated) return res; - const { dataSource, lang } = validated; + const { runQuery, cleanup, lang } = validated; const body = req.body as IRequestComparisonOptions; const { studentQuery } = body; @@ -616,16 +679,16 @@ export class GradingController { const [match, rsFeedback] = await this.resultSetComparator.compare( referenceQuery, studentQuery, - dataSource, + runQuery, ); - await dataSource.destroy(); + await cleanup(); const feedback = rsFeedback.length > 0 ? { verdict: { message: rsFeedback[0] } } : undefined; return res.status(200).json({ match, feedback }); } catch (error) { - await dataSource.destroy(); + await cleanup(); return res .status(500) .json({ message: t('GRADING_FAILED_WITH_ERROR', lang, String(error)) }); @@ -647,7 +710,7 @@ export class GradingController { const validated = await this.validateAndConnect(req, res); if (!validated) return res; - const { dataSource, lang } = validated; + const { cleanup, lang } = validated; const body = req.body as IRequestComparisonOptions; const { studentQuery } = body; @@ -657,7 +720,7 @@ export class GradingController { body.referenceQueries, ); - await dataSource.destroy(); // No DB call needed for AST comparison + await cleanup(); // No DB call needed for AST comparison const parser = new Parser(); let studentAST: any; @@ -707,7 +770,7 @@ export class GradingController { const validated = await this.validateAndConnect(req, res); if (!validated) return res; - const { dataSource, lang } = validated; + const { runQuery, cleanup, lang } = validated; const body = req.body as IRequestComparisonOptions; const { studentQuery } = body; @@ -726,12 +789,12 @@ export class GradingController { referenceAST = parser.astify(referenceQuery, { database: 'postgresql' }); } catch (error) { console.error(error); - await dataSource.destroy(); + await cleanup(); return res.status(400).json({ message: t('GRADING_READ_ERROR', lang) }); } if (Array.isArray(studentAST) || Array.isArray(referenceAST)) { - await dataSource.destroy(); + await cleanup(); return res.status(400).json({ message: t('GRADING_READ_ERROR', lang) }); } @@ -749,18 +812,18 @@ export class GradingController { referenceAST as AST, studentAliasMap, referenceAliasMap, - dataSource, + runQuery, studentQuery, referenceQuery, ); - await dataSource.destroy(); + await cleanup(); return res.status(200).json({ plansMatch: result.plansMatch, feedback: result.executionPlan, penaltyPoints: result.penaltyPoints, }); } catch (error) { - await dataSource.destroy(); + await cleanup(); return res .status(500) .json({ message: t('GRADING_FAILED_WITH_ERROR', lang, String(error)) }); diff --git a/services/sql-assessment-service/src/grading/query-grading-service.ts b/services/sql-assessment-service/src/grading/query-grading-service.ts index 7c7548c..b9f2b8e 100644 --- a/services/sql-assessment-service/src/grading/query-grading-service.ts +++ b/services/sql-assessment-service/src/grading/query-grading-service.ts @@ -1,5 +1,5 @@ -import { DataSource } from 'typeorm'; import { AST, Parser } from 'node-sql-parser'; +import { RowQueryFn } from '../shared/utils/database-utils'; import { ComparisonResult } from '../shared/interfaces/index'; import { AssembledFeedback } from '../shared/interfaces/feedback'; import { ResultSetComparator } from './result-set-comparator'; @@ -35,7 +35,7 @@ export class SQLQueryGradingService { public async gradeQuery( referenceQuery: string, studentQuery: string, - dataSource: DataSource, + runQuery: RowQueryFn, databaseKey: string, lang: SupportedLanguage = 'en', ): Promise { @@ -47,7 +47,7 @@ export class SQLQueryGradingService { const [executable, execFeedback] = await this.resultSetComparator.isExecutable( studentQuery, - dataSource, + runQuery, lang, ); @@ -74,7 +74,7 @@ export class SQLQueryGradingService { const [resultSetMatch, rsFeedback] = await this.resultSetComparator.compare( referenceQuery, studentQuery, - dataSource, + runQuery, lang, ); @@ -205,7 +205,7 @@ export class SQLQueryGradingService { referenceAST, astResult.studentAliasMap, astResult.referenceAliasMap, - dataSource, + runQuery, studentQuery, referenceQuery, lang, diff --git a/services/sql-assessment-service/src/grading/result-set-comparator.ts b/services/sql-assessment-service/src/grading/result-set-comparator.ts index 204db48..9b98258 100644 --- a/services/sql-assessment-service/src/grading/result-set-comparator.ts +++ b/services/sql-assessment-service/src/grading/result-set-comparator.ts @@ -1,14 +1,50 @@ -import { DataSource, QueryFailedError } from 'typeorm'; +import { QueryFailedError } from 'typeorm'; +import { RowQueryFn } from '../shared/utils/database-utils'; import { t, SupportedLanguage } from '../shared/i18n'; +/** + * Extracts a human-readable message from a database error. + * + * TypeORM wraps driver errors in {@link QueryFailedError} (with a + * `driverError.message`), whereas PGlite throws a plain {@link Error}. This + * helper normalises both so grading feedback is populated regardless of the + * execution backend. + */ +function extractDbErrorMessage(error: unknown): string { + const driverMessage = (error as QueryFailedError)?.driverError?.message; + if (typeof driverMessage === 'string' && driverMessage.length > 0) { + return driverMessage; + } + if (error instanceof Error && error.message) { + return error.message; + } + return String(error); +} + /** * Handles dynamic execution and row-level comparison of two query result sets. + * + * Execution is delegated to a {@link RowQueryFn} so the comparator is agnostic + * to the underlying backend (TypeORM Postgres or in-process PGlite). */ export class ResultSetComparator { + /** + * Executes the reference and student queries and compares their result sets. + * + * NOTE ON SESSION SCOPE: the reference and student queries are executed via + * two independent {@link RowQueryFn} invocations. For the Postgres backend + * each invocation acquires and releases its own QueryRunner (a separate + * connection/session); the PGlite backend shares one in-process instance. + * Neither query may therefore rely on session-local state established by the + * other — e.g. temporary tables, `SET` / `SET LOCAL` parameters (search_path, + * timeouts), session variables, prepared statements, or transaction scope. + * This is safe for the read-only SELECTs used in grading; revisit the + * executor design before introducing any cross-query session dependency. + */ async compare( referenceQuery: string, studentQuery: string, - dataSource: DataSource, + runQuery: RowQueryFn, lang: SupportedLanguage = 'en', ): Promise<[boolean, string[]]> { let referenceResultSet: unknown[]; @@ -17,14 +53,12 @@ export class ResultSetComparator { const feedback: string[] = []; try { - const queryRunner = dataSource.createQueryRunner(); - referenceResultSet = await queryRunner.query(referenceQuery); - studentResultSet = await queryRunner.query(studentQuery); + referenceResultSet = await runQuery(referenceQuery); + studentResultSet = await runQuery(studentQuery); comparisonResult = this.areResultsEqual( referenceResultSet, studentResultSet, ); - queryRunner.release(); } catch (error) { feedback.push(t('FEEDBACK_QUERY_COMPARISON_ERROR', lang, String(error))); return [false, feedback]; @@ -35,17 +69,15 @@ export class ResultSetComparator { async isExecutable( query: string, - dataSource: DataSource, + runQuery: RowQueryFn, lang: SupportedLanguage = 'en', ): Promise<[boolean, string[]]> { const feedback: string[] = []; try { - const queryRunner = dataSource.createQueryRunner(); - await queryRunner.query(query); - queryRunner.release(); + await runQuery(query); } catch (error) { feedback.push(t('FEEDBACK_QUERY_EXECUTION_ERROR', lang)); - feedback.push((error as QueryFailedError)?.driverError?.message); + feedback.push(extractDbErrorMessage(error)); return [false, feedback]; } return [true, feedback]; diff --git a/services/sql-assessment-service/src/shared/i18n/messages.ts b/services/sql-assessment-service/src/shared/i18n/messages.ts index 20c1b5f..8c084cd 100644 --- a/services/sql-assessment-service/src/shared/i18n/messages.ts +++ b/services/sql-assessment-service/src/shared/i18n/messages.ts @@ -37,7 +37,6 @@ export type MessageKey = | 'GRADING_CONNECTION_READ_ERROR' | 'GRADING_FAILED' | 'GRADING_FAILED_WITH_ERROR' - | 'GRADING_PGLITE_NOT_SUPPORTED' // ── grading feedback — result set ───────────────────────────────────────── | 'FEEDBACK_RESULT_SET_MATCH' | 'FEEDBACK_RESULT_SET_MISMATCH' @@ -151,8 +150,6 @@ export const MESSAGES: MessageCatalogue = { GRADING_CONNECTION_READ_ERROR: 'Error reading connection information.', GRADING_FAILED: 'Unable to grade query.', GRADING_FAILED_WITH_ERROR: 'Unable to grade query. Error: {{value}}', - GRADING_PGLITE_NOT_SUPPORTED: - 'Grading endpoints require a PostgreSQL connection; PGlite is not supported here.', // grading feedback — result set FEEDBACK_RESULT_SET_MATCH: 'Same result set of both queries.', FEEDBACK_RESULT_SET_MISMATCH: 'Result sets differ.', @@ -295,8 +292,6 @@ export const MESSAGES: MessageCatalogue = { GRADING_FAILED: 'Die Abfrage konnte nicht bewertet werden.', GRADING_FAILED_WITH_ERROR: 'Die Abfrage konnte nicht bewertet werden. Fehler: {{value}}', - GRADING_PGLITE_NOT_SUPPORTED: - 'Bewertungsendpunkte erfordern eine PostgreSQL-Verbindung; PGlite wird hier nicht unterstützt.', // grading feedback — result set FEEDBACK_RESULT_SET_MATCH: 'Beide Abfragen liefern dasselbe Ergebnis.', FEEDBACK_RESULT_SET_MISMATCH: 'Die Ergebnismengen stimmen nicht überein.', diff --git a/services/sql-assessment-service/src/shared/utils/database-utils.ts b/services/sql-assessment-service/src/shared/utils/database-utils.ts index 063d673..df50896 100644 --- a/services/sql-assessment-service/src/shared/utils/database-utils.ts +++ b/services/sql-assessment-service/src/shared/utils/database-utils.ts @@ -1,4 +1,5 @@ import { DataSource, QueryRunner } from 'typeorm'; +import type { PGlite } from '@electric-sql/pglite'; import { IAliasMap, IParsedTable } from '../interfaces/domain'; /** @@ -24,6 +25,22 @@ export function makeRowQueryFn(dataSource: DataSource): RowQueryFn { }; } +/** + * Builds a {@link RowQueryFn} backed by a live in-process {@link PGlite} + * instance. Unwraps the `{ rows }` result envelope so the returned array has + * the same shape as the TypeORM {@link makeRowQueryFn} path. + * + * IMPORTANT: PGlite instances are shared and long-lived (stored in + * `pgliteInstances`). This function never closes the instance — callers must + * not destroy it after use. + */ +export function makePGliteRowQueryFn(db: PGlite): RowQueryFn { + return async (sql: string) => { + const result = await db.query(sql); + return result.rows as any[]; + }; +} + export function generateDatabaseKey( host: string, port: number, diff --git a/services/sql-assessment-service/test/grading/pglite-grading-controller.test.ts b/services/sql-assessment-service/test/grading/pglite-grading-controller.test.ts index a70c594..a61179b 100644 --- a/services/sql-assessment-service/test/grading/pglite-grading-controller.test.ts +++ b/services/sql-assessment-service/test/grading/pglite-grading-controller.test.ts @@ -1,15 +1,16 @@ /** - * Regression tests for the validation-ordering bug in GradingController. + * Integration tests for GradingController running against an in-process PGlite + * database. * - * Before the fix, `validateConnectionInfo` was called BEFORE the auto-analyze - * block in both `gradeQuery` and `validateAndConnect`. Any PGlite connection - * was therefore rejected with INVALID_CONNECTION_INFO before auto-analyze - * could run. + * Previously the controller rejected any PGlite connection with + * GRADING_PGLITE_NOT_SUPPORTED (400). Grading now routes PGlite connections + * through the shared `pgliteInstances` registry via a backend-agnostic + * `RowQueryFn`, so all four grading endpoints work end-to-end without an + * external PostgreSQL server. * - * After the fix the controller: - * 1. Runs auto-analyze first (so PGlite can be registered if sqlContent is provided) - * 2. Branches on `type === 'pglite'` and returns 400 with GRADING_PGLITE_NOT_SUPPORTED - * 3. Calls `validateConnectionInfo` only in the Postgres branch + * These tests exercise the real grading pipeline (executability, result-set, + * AST and EXPLAIN-based execution-plan comparison) against a real PGlite + * instance seeded from DDL. */ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import type { Request, Response } from 'express'; @@ -42,7 +43,10 @@ CREATE TABLE products ( name TEXT NOT NULL, price NUMERIC(10,2) ); -INSERT INTO products (name, price) VALUES ('Widget', 9.99); +INSERT INTO products (name, price) VALUES + ('Widget', 9.99), + ('Gadget', 19.99), + ('Gizmo', 29.99); `; const DB_ID = 'pglite-grading-test-db'; @@ -58,8 +62,11 @@ const PGLITE_CONN_NO_SQL = { databaseId: DB_ID, }; -const REF_QUERY = 'SELECT name FROM products'; -const STUDENT_QUERY = 'SELECT name FROM products'; +const REF_QUERY = 'SELECT name FROM products WHERE price > 15 ORDER BY name'; +const CORRECT_STUDENT_QUERY = + 'SELECT name FROM products WHERE price > 15 ORDER BY name'; +const WRONG_STUDENT_QUERY = 'SELECT name FROM products ORDER BY name'; +const NON_EXECUTABLE_QUERY = 'SELECT name FROM no_such_table'; // --------------------------------------------------------------------------- // Express mock helpers @@ -75,21 +82,45 @@ function mockRes() { return { res: { status, json } as unknown as Response, status, json }; } +/** Reads the JSON payload of the first `res.json(...)` call. */ +function firstJson(json: ReturnType['json']): any { + return json.mock.calls[0]?.[0]; +} + // --------------------------------------------------------------------------- // Tests // --------------------------------------------------------------------------- -describe('GradingController — PGlite validation ordering', () => { +describe('GradingController — PGlite backend', () => { let controller: GradingController; beforeEach(() => { const joinComparator = new JoinComparator(); + const resultSetComparator = new ResultSetComparator(); + const astComparator = new ASTComparator(joinComparator); + const executionPlanComparator = new ExecutionPlanComparator( + new ExecutionPlanParser(), + joinComparator, + ); + const queryGradingService = new SQLQueryGradingService( + resultSetComparator, + astComparator, + executionPlanComparator, + new GradeCalculator(), + new FeedbackAssembler(), + ); + controller = new GradingController( - {} as SQLQueryGradingService, - {} as TaskDescriptionGenerationService, - new ResultSetComparator(), - new ASTComparator(joinComparator), - new ExecutionPlanComparator(new ExecutionPlanParser(), joinComparator), + queryGradingService, + // Task-description generation is a no-op stub (no OPENAI key in tests): + // gradeQuery only calls it for non-equivalent queries, and the stub + // returns undefined so no description is appended. + { + generateTaskFromQuery: vi.fn().mockResolvedValue(undefined), + } as unknown as TaskDescriptionGenerationService, + resultSetComparator, + astComparator, + executionPlanComparator, new QueryProximityService(), new DatabaseService(new DatabaseAnalyzer()), ); @@ -108,128 +139,195 @@ describe('GradingController — PGlite validation ordering', () => { // ── POST /api/grading/grade ────────────────────────────────────────────── describe('gradeQuery', () => { - it('returns 400 GRADING_PGLITE_NOT_SUPPORTED, not INVALID_CONNECTION_INFO', async () => { + it('grades an identical student query as fully equivalent (grade 7)', async () => { const { res, status, json } = mockRes(); await controller.gradeQuery( mockReq({ connectionInfo: PGLITE_CONN_WITH_SQL, gradingRequest: { referenceQuery: REF_QUERY, - studentQuery: STUDENT_QUERY, + studentQuery: CORRECT_STUDENT_QUERY, }, }), res, ); - expect(status).toHaveBeenCalledWith(400); - const message: string = json.mock.calls[0]?.[0]?.message ?? ''; - expect(message).not.toMatch(/invalid.*connection/i); - expect(message).toMatch(/pglite/i); + + expect(status).toHaveBeenCalledWith(200); + const body = firstJson(json); + expect(body.comparisonResult.grade).toBe(7); + expect(body.comparisonResult.equivalent).toBe(true); + }); + + it('penalises a student query with a missing WHERE clause', async () => { + const { res, status, json } = mockRes(); + await controller.gradeQuery( + mockReq({ + connectionInfo: PGLITE_CONN_WITH_SQL, + gradingRequest: { + referenceQuery: REF_QUERY, + studentQuery: WRONG_STUDENT_QUERY, + }, + }), + res, + ); + + expect(status).toHaveBeenCalledWith(200); + const body = firstJson(json); + expect(body.comparisonResult.equivalent).toBe(false); + expect(body.comparisonResult.grade).toBeLessThan(7); }); - it('runs auto-analyze before rejecting (PGlite DB is registered after the call)', async () => { + it('returns grade 0 with executability feedback for a non-executable query', async () => { + const { res, status, json } = mockRes(); + await controller.gradeQuery( + mockReq({ + connectionInfo: PGLITE_CONN_WITH_SQL, + gradingRequest: { + referenceQuery: REF_QUERY, + studentQuery: NON_EXECUTABLE_QUERY, + }, + }), + res, + ); + + expect(status).toHaveBeenCalledWith(200); + const body = firstJson(json); + expect(body.comparisonResult.grade).toBe(0); + // executability feedback must be populated (regression: PGlite errors + // are plain Error objects, not TypeORM QueryFailedError) + expect( + body.comparisonResult.feedbackDetails.general.executability, + ).toBeDefined(); + }); + + it('does not close the shared PGlite instance after grading', async () => { const { res } = mockRes(); await controller.gradeQuery( mockReq({ connectionInfo: PGLITE_CONN_WITH_SQL, gradingRequest: { referenceQuery: REF_QUERY, - studentQuery: STUDENT_QUERY, + studentQuery: CORRECT_STUDENT_QUERY, }, }), res, ); - // auto-analyze ran → PGlite instance was registered - expect(pgliteInstances.has(DB_ID)).toBe(true); + + // Instance must still be registered and usable after the request. + const db = pgliteInstances.get(DB_ID); + expect(db).toBeDefined(); + const result = await db!.query('SELECT COUNT(*)::int AS c FROM products'); + expect((result.rows[0] as any).c).toBe(3); }); - it('returns 400 GRADING_PGLITE_NOT_SUPPORTED even without sqlContent', async () => { + it('returns 400 DATABASE_NOT_REGISTERED when no sqlContent and DB is unknown', async () => { const { res, status, json } = mockRes(); await controller.gradeQuery( mockReq({ connectionInfo: PGLITE_CONN_NO_SQL, gradingRequest: { referenceQuery: REF_QUERY, - studentQuery: STUDENT_QUERY, + studentQuery: CORRECT_STUDENT_QUERY, }, }), res, ); + expect(status).toHaveBeenCalledWith(400); - const message: string = json.mock.calls[0]?.[0]?.message ?? ''; - expect(message).toMatch(/pglite/i); + const message: string = firstJson(json)?.message ?? ''; + // No longer a "PGlite not supported" rejection. + expect(message).not.toMatch(/pglite.*not supported/i); }); }); - // ── POST /api/grading/compare/result-set (via validateAndConnect) ───────── + // ── POST /api/grading/compare/result-set ───────────────────────────────── describe('compareResultSet', () => { - it('returns 400 GRADING_PGLITE_NOT_SUPPORTED, not INVALID_CONNECTION_INFO', async () => { + it('reports match=true for equivalent result sets', async () => { const { res, status, json } = mockRes(); await controller.compareResultSet( mockReq({ connectionInfo: PGLITE_CONN_WITH_SQL, referenceQuery: REF_QUERY, - studentQuery: STUDENT_QUERY, + studentQuery: CORRECT_STUDENT_QUERY, }), res, ); - expect(status).toHaveBeenCalledWith(400); - const message: string = json.mock.calls[0]?.[0]?.message ?? ''; - expect(message).not.toMatch(/invalid.*connection/i); - expect(message).toMatch(/pglite/i); + + expect(status).toHaveBeenCalledWith(200); + expect(firstJson(json).match).toBe(true); }); - it('runs auto-analyze before rejecting (PGlite DB is registered after the call)', async () => { - const { res } = mockRes(); + it('reports match=false for differing result sets', async () => { + const { res, status, json } = mockRes(); await controller.compareResultSet( mockReq({ connectionInfo: PGLITE_CONN_WITH_SQL, referenceQuery: REF_QUERY, - studentQuery: STUDENT_QUERY, + studentQuery: WRONG_STUDENT_QUERY, }), res, ); - expect(pgliteInstances.has(DB_ID)).toBe(true); + + expect(status).toHaveBeenCalledWith(200); + expect(firstJson(json).match).toBe(false); }); }); - // ── POST /api/grading/compare/ast (via validateAndConnect) ─────────────── + // ── POST /api/grading/compare/ast ──────────────────────────────────────── describe('compareAST', () => { - it('returns 400 GRADING_PGLITE_NOT_SUPPORTED, not INVALID_CONNECTION_INFO', async () => { + it('compares ASTs without needing a live query', async () => { const { res, status, json } = mockRes(); await controller.compareAST( mockReq({ connectionInfo: PGLITE_CONN_WITH_SQL, referenceQuery: REF_QUERY, - studentQuery: STUDENT_QUERY, + studentQuery: CORRECT_STUDENT_QUERY, }), res, ); - expect(status).toHaveBeenCalledWith(400); - const message: string = json.mock.calls[0]?.[0]?.message ?? ''; - expect(message).not.toMatch(/invalid.*connection/i); - expect(message).toMatch(/pglite/i); + + expect(status).toHaveBeenCalledWith(200); + expect(firstJson(json).columnsMatch).toBe(true); }); }); - // ── POST /api/grading/compare/execution-plan (via validateAndConnect) ──── + // ── POST /api/grading/compare/execution-plan ───────────────────────────── describe('compareExecutionPlan', () => { - it('returns 400 GRADING_PGLITE_NOT_SUPPORTED, not INVALID_CONNECTION_INFO', async () => { + it('runs EXPLAIN on PGlite and reports plansMatch=true for identical plans', async () => { const { res, status, json } = mockRes(); await controller.compareExecutionPlan( mockReq({ connectionInfo: PGLITE_CONN_WITH_SQL, referenceQuery: REF_QUERY, - studentQuery: STUDENT_QUERY, + studentQuery: CORRECT_STUDENT_QUERY, }), res, ); - expect(status).toHaveBeenCalledWith(400); - const message: string = json.mock.calls[0]?.[0]?.message ?? ''; - expect(message).not.toMatch(/invalid.*connection/i); - expect(message).toMatch(/pglite/i); + + expect(status).toHaveBeenCalledWith(200); + const body = firstJson(json); + expect(body.plansMatch).toBe(true); + expect(body.penaltyPoints).toBe(0); + }); + + it('detects a differing WHERE clause via EXPLAIN and adds penalty points', async () => { + const { res, status, json } = mockRes(); + await controller.compareExecutionPlan( + mockReq({ + connectionInfo: PGLITE_CONN_WITH_SQL, + referenceQuery: REF_QUERY, + studentQuery: WRONG_STUDENT_QUERY, + }), + res, + ); + + expect(status).toHaveBeenCalledWith(200); + const body = firstJson(json); + expect(body.plansMatch).toBe(false); + expect(body.penaltyPoints).toBeGreaterThan(0); }); }); }); diff --git a/services/sql-assessment-service/test/shared/pglite-row-query-fn.test.ts b/services/sql-assessment-service/test/shared/pglite-row-query-fn.test.ts new file mode 100644 index 0000000..3629fb0 --- /dev/null +++ b/services/sql-assessment-service/test/shared/pglite-row-query-fn.test.ts @@ -0,0 +1,151 @@ +/** + * Unit tests for the PGlite-backed {@link RowQueryFn} and the grading + * comparators when driven by it. + * + * These verify the two shape-alignment invariants that make the grading + * pipeline backend-agnostic: + * + * 1. `makePGliteRowQueryFn` unwraps PGlite's `{ rows }` envelope so callers + * receive a plain row array — identical to the TypeORM `makeRowQueryFn`. + * 2. PGlite's `EXPLAIN (FORMAT JSON)` rows are shaped so `rawPlan[0]` equals + * `{ 'QUERY PLAN': [...] }`, exactly what ExecutionPlanParser expects. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { PGlite } from '@electric-sql/pglite'; +import { makePGliteRowQueryFn } from '../../src/shared/utils/database-utils'; +import { ResultSetComparator } from '../../src/grading/result-set-comparator'; +import { ExecutionPlanComparator } from '../../src/grading/comparators/execution-plan-comparator'; +import { ExecutionPlanParser } from '../../src/grading/execution-plan-parser'; +import { JoinComparator } from '../../src/grading/join-comparator'; +import { Parser, AST } from 'node-sql-parser'; + +const DDL = ` +CREATE TABLE products ( + id SERIAL PRIMARY KEY, + name TEXT NOT NULL, + price NUMERIC(10,2) +); +INSERT INTO products (name, price) VALUES ('Widget', 9.99), ('Gadget', 19.99); +`; + +let db: PGlite; + +beforeAll(async () => { + db = new PGlite(); + await db.exec(DDL); +}); + +afterAll(async () => { + await db.close(); +}); + +describe('makePGliteRowQueryFn', () => { + it('returns a plain array of rows (unwrapping the { rows } envelope)', async () => { + const runQuery = makePGliteRowQueryFn(db); + const rows = await runQuery('SELECT name FROM products ORDER BY name'); + expect(Array.isArray(rows)).toBe(true); + expect(rows).toEqual([{ name: 'Gadget' }, { name: 'Widget' }]); + }); + + it('produces EXPLAIN rows whose [0] entry has a "QUERY PLAN" key', async () => { + const runQuery = makePGliteRowQueryFn(db); + const rawPlan = await runQuery( + 'EXPLAIN (FORMAT JSON) SELECT name FROM products', + ); + expect(Array.isArray(rawPlan)).toBe(true); + expect(rawPlan[0]).toHaveProperty('QUERY PLAN'); + expect(Array.isArray(rawPlan[0]['QUERY PLAN'])).toBe(true); + }); + + it('propagates database errors as thrown Errors', async () => { + const runQuery = makePGliteRowQueryFn(db); + await expect(runQuery('SELECT * FROM missing_table')).rejects.toThrow(); + }); + + it('does not close the shared instance', async () => { + const runQuery = makePGliteRowQueryFn(db); + await runQuery('SELECT 1'); + // Still usable afterwards. + const rows = await runQuery('SELECT 2 AS v'); + expect((rows[0] as any).v).toBe(2); + }); +}); + +describe('ResultSetComparator driven by PGlite RowQueryFn', () => { + const comparator = new ResultSetComparator(); + const runQuery = () => makePGliteRowQueryFn(db); + + it('reports equal result sets', async () => { + const [match, feedback] = await comparator.compare( + 'SELECT name FROM products ORDER BY name', + 'SELECT name FROM products ORDER BY name', + runQuery(), + ); + expect(match).toBe(true); + expect(feedback).toEqual([]); + }); + + it('reports unequal result sets', async () => { + const [match] = await comparator.compare( + 'SELECT name FROM products WHERE price > 15', + 'SELECT name FROM products', + runQuery(), + ); + expect(match).toBe(false); + }); + + it('isExecutable populates feedback from a plain Error message', async () => { + const [ok, feedback] = await comparator.isExecutable( + 'SELECT * FROM missing_table', + runQuery(), + ); + expect(ok).toBe(false); + // Second feedback entry is the extracted DB error message — must not be + // undefined (regression guard for PGlite plain Error objects). + expect(feedback[1]).toBeTruthy(); + expect(typeof feedback[1]).toBe('string'); + }); +}); + +describe('ExecutionPlanComparator driven by PGlite RowQueryFn', () => { + const comparator = new ExecutionPlanComparator( + new ExecutionPlanParser(), + new JoinComparator(), + ); + const parser = new Parser(); + + function astOf(sql: string): AST { + return parser.astify(sql, { database: 'postgresql' }) as AST; + } + + it('returns plansMatch=true for identical queries', async () => { + const q = 'SELECT name FROM products WHERE price > 15 ORDER BY name'; + const result = await comparator.compare( + astOf(q), + astOf(q), + {}, + {}, + makePGliteRowQueryFn(db), + q, + q, + ); + expect(result.plansMatch).toBe(true); + expect(result.penaltyPoints).toBe(0); + }); + + it('detects a missing WHERE clause and adds penalty points', async () => { + const ref = 'SELECT name FROM products WHERE price > 15 ORDER BY name'; + const student = 'SELECT name FROM products ORDER BY name'; + const result = await comparator.compare( + astOf(student), + astOf(ref), + {}, + {}, + makePGliteRowQueryFn(db), + student, + ref, + ); + expect(result.plansMatch).toBe(false); + expect(result.penaltyPoints).toBeGreaterThan(0); + }); +});