From fa7db2c965224235e2c8f3c48b15f16e2a3230c8 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Thu, 11 Dec 2025 09:45:53 +0000 Subject: [PATCH] WIP fixing parser --- internal-packages/tsql/src/index.ts | 3 - .../tsql/src/query/parser.test.ts | 4 +- internal-packages/tsql/src/query/parser.ts | 125 ++++++++------ internal-packages/tsql/src/query/query.ts | 162 +++++++++--------- 4 files changed, 159 insertions(+), 135 deletions(-) diff --git a/internal-packages/tsql/src/index.ts b/internal-packages/tsql/src/index.ts index 84a17bc91..6580918fb 100644 --- a/internal-packages/tsql/src/index.ts +++ b/internal-packages/tsql/src/index.ts @@ -1,4 +1 @@ import { Logger } from "@trigger.dev/core/logger"; - -export { TQuery } from "./query/query.js"; -export type { TQueryOptions } from "./query/query.js"; diff --git a/internal-packages/tsql/src/query/parser.test.ts b/internal-packages/tsql/src/query/parser.test.ts index 5cd50b44d..514c524ae 100644 --- a/internal-packages/tsql/src/query/parser.test.ts +++ b/internal-packages/tsql/src/query/parser.test.ts @@ -13,6 +13,7 @@ import type { ArithmeticOperation, Alias, JoinExpr, + HogQLXTag, } from "./ast.js"; import { ArithmeticOperationOp, CompareOperationOp } from "./ast.js"; import { SyntaxError } from "./errors.js"; @@ -32,10 +33,11 @@ function parseAndConvert(input: string) { describe("TSQLParseTreeConverter", () => { describe("SELECT statements", () => { - it("should convert a simple SELECT statement", () => { + it.only("should convert a simple SELECT statement", () => { const ast = parseAndConvert("SELECT * FROM users"); expect(ast).toBeDefined(); + console.log(ast); expect("select" in ast).toBe(true); const selectQuery = ast as SelectQuery; expect(selectQuery.select).toBeDefined(); diff --git a/internal-packages/tsql/src/query/parser.ts b/internal-packages/tsql/src/query/parser.ts index 4ead761c4..c15b1e239 100644 --- a/internal-packages/tsql/src/query/parser.ts +++ b/internal-packages/tsql/src/query/parser.ts @@ -110,7 +110,14 @@ export class TSQLParseTreeConverter implements TSQLParserVisitor { const end = stop !== undefined ? stop + 1 : undefined; try { const node = this.visitChildren(ctx); - if (node && typeof node === "object" && "start" in node && this.start !== undefined) { + // Only set position if node is a valid object and we have position info + if ( + node && + typeof node === "object" && + node !== null && + "start" in node && + this.start !== undefined + ) { node.start = start; node.end = end; } @@ -132,44 +139,14 @@ export class TSQLParseTreeConverter implements TSQLParserVisitor { /** * Visit a parse tree node, dispatching to the appropriate visitor method. - * Uses type guards to safely handle ParseTree, ParserRuleContext, TerminalNode, and ErrorNode. + * Uses the accept method for proper double dispatch, which handles ErrorNode vs TerminalNode correctly. */ private visitParseTree(node: ParseTree): any { - // ErrorNode extends TerminalNode, so check ErrorNode first - if (this.isErrorNode(node)) { - return this.visitErrorNode(node); - } - if (this.isTerminalNode(node)) { - return this.visitTerminal(node); - } - if (this.isParserRuleContext(node)) { - return this.visit(node); - } - // Fallback: use accept method for double dispatch + // Use accept method for double dispatch - this will call the correct visit method + // based on the actual runtime type of the node return node.accept(this); } - /** - * Type guard to check if a ParseTree is an ErrorNode. - */ - private isErrorNode(node: ParseTree): node is ErrorNode { - return "symbol" in node && node.symbol !== undefined; - } - - /** - * Type guard to check if a ParseTree is a TerminalNode. - */ - private isTerminalNode(node: ParseTree): node is TerminalNode { - return "symbol" in node && !("ruleIndex" in node); - } - - /** - * Type guard to check if a ParseTree is a ParserRuleContext. - */ - private isParserRuleContext(node: ParseTree): node is ParserRuleContext { - return "ruleIndex" in node && "start" in node; - } - visitChildren(ctx: ParserRuleContext): any { if (!ctx.children || ctx.children.length === 0) { return null; @@ -788,14 +765,16 @@ export class TSQLParseTreeConverter implements TSQLParserVisitor { } else { throw new NotImplementedError(`Unsupported ColumnExprPrecedence1: ${ctx.getText()}`); } - const left = this.visit(ctx.left); - const right = this.visit(ctx.right); + // Use columnExpr() method to get left and right operands + const left = this.visit(ctx.columnExpr(0)); + const right = this.visit(ctx.columnExpr(1)); return { left, right, op }; } visitColumnExprPrecedence2(ctx: any): ArithmeticOperation | Call { - const left = this.visit(ctx.left); - const right = this.visit(ctx.right); + // Use columnExpr() method to get left and right operands + const left = this.visit(ctx.columnExpr(0)); + const right = this.visit(ctx.columnExpr(1)); if (ctx.PLUS()) { return { left, right, op: ArithmeticOperationOp.Add }; @@ -822,8 +801,9 @@ export class TSQLParseTreeConverter implements TSQLParserVisitor { } visitColumnExprPrecedence3(ctx: any): CompareOperation { - const left = this.visit(ctx.left); - const right = this.visit(ctx.right); + // Use columnExpr() method to get left and right operands + const left = this.visit(ctx.columnExpr(0)); + const right = this.visit(ctx.columnExpr(1)); let op: CompareOperationOp; if (ctx.EQ_SINGLE() || ctx.EQ_DOUBLE()) { @@ -1140,12 +1120,15 @@ export class TSQLParseTreeConverter implements TSQLParserVisitor { visitTableIdentifier(ctx: any): string[] { const nested = ctx.nestedIdentifier() ? this.visit(ctx.nestedIdentifier()) : []; + // Ensure nested is always an array + const nestedArray = Array.isArray(nested) ? nested : nested ? [nested] : []; if (ctx.databaseIdentifier()) { - return [this.visit(ctx.databaseIdentifier()), ...nested]; + const dbId = this.visit(ctx.databaseIdentifier()); + return [dbId, ...nestedArray]; } - return nested; + return nestedArray; } visitTableArgList(ctx: any): Expr[] { @@ -1157,6 +1140,12 @@ export class TSQLParseTreeConverter implements TSQLParserVisitor { } visitNumberLiteral(ctx: any): Constant { + // NumberLiteralContext is a ParserRuleContext, use getText() to get the text + if (!ctx || typeof ctx.getText !== "function") { + throw new SyntaxError( + "Invalid number literal context - expected ParserRuleContext with getText()" + ); + } const text = ctx.getText().toLowerCase(); if ( text.includes(".") || @@ -1175,9 +1164,14 @@ export class TSQLParseTreeConverter implements TSQLParserVisitor { return { value: null }; } if (ctx.STRING_LITERAL()) { - const text = parseStringLiteralCtx(ctx); + // STRING_LITERAL() returns a TerminalNode, which has getText() + const stringLiteral = ctx.STRING_LITERAL(); + const text = parseStringLiteralCtx(stringLiteral); return { value: text }; } + if (ctx.numberLiteral()) { + return this.visitNumberLiteral(ctx.numberLiteral()); + } return this.visitChildren(ctx); } @@ -1193,14 +1187,45 @@ export class TSQLParseTreeConverter implements TSQLParserVisitor { } visitIdentifier(ctx: any): string { - let text = ctx.getText(); - if ( - text.length >= 2 && - ((text.startsWith("`") && text.endsWith("`")) || (text.startsWith('"') && text.endsWith('"'))) - ) { - text = parseStringLiteralText(text); + // IdentifierContext is a ParserRuleContext that has IDENTIFIER() method returning TerminalNode + // If ctx has IDENTIFIER() method, extract the terminal node + if (ctx.IDENTIFIER && typeof ctx.IDENTIFIER === "function") { + const terminalNode = ctx.IDENTIFIER(); + if (terminalNode) { + // TerminalNode has getText() at runtime but types don't expose it + // Use symbol.text as fallback + let text = (terminalNode as any).getText?.() || terminalNode.symbol?.text || ""; + if ( + text.length >= 2 && + ((text.startsWith("`") && text.endsWith("`")) || + (text.startsWith('"') && text.endsWith('"'))) + ) { + text = parseStringLiteralText(text); + } + return text; + } } - return text; + // Fallback: if it's a ParserRuleContext, use getText() + if (typeof ctx.getText === "function") { + let text = ctx.getText(); + if ( + text.length >= 2 && + ((text.startsWith("`") && text.endsWith("`")) || + (text.startsWith('"') && text.endsWith('"'))) + ) { + text = parseStringLiteralText(text); + } + return text; + } + // If it's already a string + if (typeof ctx === "string") { + return ctx; + } + // Try to get text from symbol if it's a TerminalNode + if (ctx.symbol && ctx.symbol.text) { + return ctx.symbol.text; + } + throw new SyntaxError("Invalid identifier context"); } visitColumnExprNullish(ctx: any): Call { diff --git a/internal-packages/tsql/src/query/query.ts b/internal-packages/tsql/src/query/query.ts index 94411c90f..020ea3a2f 100644 --- a/internal-packages/tsql/src/query/query.ts +++ b/internal-packages/tsql/src/query/query.ts @@ -1,94 +1,94 @@ -import { CharStreams, CommonTokenStream } from "antlr4ts"; -import { TSQLLexer } from "../grammar/TSQLLexer.js"; -import { TSQLParser } from "../grammar/TSQLParser.js"; -import { ClickHouseQueryVisitor } from "./ClickHouseQueryVisitor.js"; -import type { ClickHouse } from "@internal/clickhouse"; -import { ClickhouseQueryBuilder } from "@internal/clickhouse/client/queryBuilder.js"; -import { z } from "zod"; +// import { CharStreams, CommonTokenStream } from "antlr4ts"; +// import { TSQLLexer } from "../grammar/TSQLLexer.js"; +// import { TSQLParser } from "../grammar/TSQLParser.js"; +// import { ClickHouseQueryVisitor } from "./parser.js"; +// import type { ClickHouse } from "@internal/clickhouse"; +// import { ClickhouseQueryBuilder } from "@internal/clickhouse/client/queryBuilder.js"; +// import { z } from "zod"; -export interface TQueryOptions { - organizationId: string; - projectId: string; - environmentId: string; -} +// export interface TQueryOptions { +// organizationId: string; +// projectId: string; +// environmentId: string; +// } -export class TQuery { - private readonly organizationId: string; - private readonly projectId: string; - private readonly environmentId: string; - private readonly clickhouseReader: ClickHouse; +// export class TQuery { +// private readonly organizationId: string; +// private readonly projectId: string; +// private readonly environmentId: string; +// private readonly clickhouseReader: ClickHouse; - constructor(clickhouseReader: ClickHouse, options: TQueryOptions) { - this.clickhouseReader = clickhouseReader; - this.organizationId = options.organizationId; - this.projectId = options.projectId; - this.environmentId = options.environmentId; - } +// constructor(clickhouseReader: ClickHouse, options: TQueryOptions) { +// this.clickhouseReader = clickhouseReader; +// this.organizationId = options.organizationId; +// this.projectId = options.projectId; +// this.environmentId = options.environmentId; +// } - /** - * Execute a TSQL query and return the results - * @param input TSQL query string - * @param schema Zod schema for the output rows - * @returns Promise with query results - */ - async query>( - input: string, - schema: TOutput - ): Promise<[Error | null, z.output[] | null]> { - // Parse the TSQL input - const inputStream = CharStreams.fromString(input); - const lexer = new TSQLLexer(inputStream); - const tokenStream = new CommonTokenStream(lexer as any); - const parser = new TSQLParser(tokenStream); +// /** +// * Execute a TSQL query and return the results +// * @param input TSQL query string +// * @param schema Zod schema for the output rows +// * @returns Promise with query results +// */ +// async query>( +// input: string, +// schema: TOutput +// ): Promise<[Error | null, z.output[] | null]> { +// // Parse the TSQL input +// const inputStream = CharStreams.fromString(input); +// const lexer = new TSQLLexer(inputStream); +// const tokenStream = new CommonTokenStream(lexer as any); +// const parser = new TSQLParser(tokenStream); - // Parse as a SELECT statement - const tree = parser.select(); +// // Parse as a SELECT statement +// const tree = parser.select(); - // Convert AST to QueryConfig - const visitor = new ClickHouseQueryVisitor(); - const queryConfig = visitor.visit(tree); +// // Convert AST to QueryConfig +// const visitor = new ClickHouseQueryVisitor(); +// const queryConfig = visitor.visit(tree); - // Use ClickhouseQueryBuilder to build the query - const queryBuilder = new ClickhouseQueryBuilder( - "tsql-query", - queryConfig.baseQuery, - this.clickhouseReader.reader, - schema - ); +// // Use ClickhouseQueryBuilder to build the query +// const queryBuilder = new ClickhouseQueryBuilder( +// "tsql-query", +// queryConfig.baseQuery, +// this.clickhouseReader.reader, +// schema +// ); - // Add existing WHERE clauses from the TSQL query - for (const whereClause of queryConfig.whereClauses) { - queryBuilder.where(whereClause.clause, whereClause.params); - } +// // Add existing WHERE clauses from the TSQL query +// for (const whereClause of queryConfig.whereClauses) { +// queryBuilder.where(whereClause.clause, whereClause.params); +// } - // Add scoping WHERE clauses - queryBuilder - .where("organization_id = {organizationId: String}", { - organizationId: this.organizationId, - }) - .where("project_id = {projectId: String}", { - projectId: this.projectId, - }) - .where("environment_id = {environmentId: String}", { - environmentId: this.environmentId, - }); +// // Add scoping WHERE clauses +// queryBuilder +// .where("organization_id = {organizationId: String}", { +// organizationId: this.organizationId, +// }) +// .where("project_id = {projectId: String}", { +// projectId: this.projectId, +// }) +// .where("environment_id = {environmentId: String}", { +// environmentId: this.environmentId, +// }); - // Add GROUP BY if present - if (queryConfig.groupBy) { - queryBuilder.groupBy(queryConfig.groupBy); - } +// // Add GROUP BY if present +// if (queryConfig.groupBy) { +// queryBuilder.groupBy(queryConfig.groupBy); +// } - // Add ORDER BY if present - if (queryConfig.orderBy) { - queryBuilder.orderBy(queryConfig.orderBy); - } +// // Add ORDER BY if present +// if (queryConfig.orderBy) { +// queryBuilder.orderBy(queryConfig.orderBy); +// } - // Add LIMIT if present - if (queryConfig.limit !== undefined) { - queryBuilder.limit(queryConfig.limit); - } +// // Add LIMIT if present +// if (queryConfig.limit !== undefined) { +// queryBuilder.limit(queryConfig.limit); +// } - // Execute the query - return await queryBuilder.execute(); - } -} +// // Execute the query +// return await queryBuilder.execute(); +// } +// }