DM-55216: Add Hyrise-based SQL parser backend#1030
Open
malensek wants to merge 9 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a compile-time switchable SQL parsing backend using a Hyrise-based parser/adapter to produce Qserv IR for SELECT statements, while retaining the existing ANTLR4-based path.
Changes:
- Introduces
ccontrol::HyriseAdapterand routesParseRunner::makeSelectStmt()through it whenQSERV_USE_HYRISE_SQL_PARSERis enabled. - Refactors parsing entrypoints so
UserQueryFactoryandQuerySessionuseParseRunner::makeSelectStmt(), and updates tests to normalize backend output differences. - Adds a parser corpus stress test and adds/updates CMake + submodule wiring for the new dependency.
Reviewed changes
Copilot reviewed 35 out of 37 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tests/QueryAnaHelper.h | Removes ParseRunner helper declaration no longer used by tests. |
| src/tests/QueryAnaHelper.cc | Removes ParseRunner helper implementation. |
| src/tests/ParserExpected.h | Adds macro helper to normalize expected strings per backend. |
| src/qproc/testQueryAnaOrderBy.cc | Updates expected ORDER BY serialization differences via PARSER_EXPECTED. |
| src/qproc/testQueryAnaGeneral.cc | Normalizes numeric/operator formatting differences; updates parser invocation in tests. |
| src/qproc/testQueryAnaAggregation.cc | Normalizes numeric literal formatting differences in expected strings. |
| src/qproc/QuerySession.cc | Parses via ParseRunner::makeSelectStmt() instead of constructing a ParseRunner. |
| src/ccontrol/UserQueryType.h | Adds helper APIs to identify CALL QSERV_RESULT_DELETE(...) and parse SET GLOBAL .... |
| src/ccontrol/UserQueryType.cc | Implements regex-based CALL/SET extraction helpers. |
| src/ccontrol/UserQueryFactory.cc | Consolidates SELECT parsing; adds Hyrise-specific handling for CALL/SET. |
| src/ccontrol/testUserQueryType.cc | Adds unit tests for new UserQueryType helpers. |
| src/ccontrol/testParserCorpus.cc | Adds parser corpus stress/perf regression test for IR construction. |
| src/ccontrol/testdata/parser-corpus/README.md | Documents parser corpus usage. |
| src/ccontrol/testdata/parser-corpus/q01_small.sql | Adds corpus SQL input. |
| src/ccontrol/testdata/parser-corpus/q02_medium.sql | Adds corpus SQL input. |
| src/ccontrol/testdata/parser-corpus/q03_original.sql | Adds corpus SQL input. |
| src/ccontrol/testdata/parser-corpus/q03_stripped.sql | Adds corpus SQL input. |
| src/ccontrol/testdata/parser-corpus/q07_object_table_predicate_stress.sql | Adds corpus SQL input. |
| src/ccontrol/testdata/parser-corpus/q08_object_source_join_wide.sql | Adds corpus SQL input. |
| src/ccontrol/testdata/parser-corpus/q09_q3_expression_dense.sql | Adds corpus SQL input. |
| src/ccontrol/testdata/parser-corpus/q10_having_limit.sql | Adds corpus SQL input. |
| src/ccontrol/testdata/parser-corpus/q11_join_using_time_window.sql | Adds corpus SQL input. |
| src/ccontrol/testdata/parser-corpus/q12_spatial_operator_mix.sql | Adds corpus SQL input. |
| src/ccontrol/testCControl.cc | Updates expected parse errors per backend; adds string-literal regression tests. |
| src/ccontrol/testAntlr4GeneratedIR.cc | Expands ANTLR IR test coverage + unsupported-feature tests. |
| src/ccontrol/ParseRunner.cc | Switches makeSelectStmt() to Hyrise adapter when enabled. |
| src/ccontrol/HyriseAdapter.h | Declares Hyrise adapter entrypoint for building Qserv IR. |
| src/ccontrol/HyriseAdapter.cc | Implements Hyrise parse-tree validation + conversion to Qserv IR. |
| src/ccontrol/CMakeLists.txt | Adds adapter source, links Hyrise parser, adds corpus test, conditionally selects IR test suite. |
| doc/CMakeLists.txt | Separates linkcheck doctree output directory. |
| CMakeLists.txt | Adds build option and includes Hyrise parser subdirectory. |
| .gitmodules | Adds extern/hyrise-sql-parser submodule definition. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
malensek
changed the title
malensek
force-pushed
the
tickets/DM-55216
branch
from
July 9, 2026 21:33
1688713 to
3f16cd2
Compare
fritzm
self-requested a review
July 15, 2026 18:08
fritzm
reviewed
Jul 15, 2026
qserv-compat branch from lsst/hyrise-sql-parser Updates the documentation link check output directory to avoid a conflict / race during parallel builds.
Translates the Hyrise parse tree to Qserv IR. HyriseDiagnostics includes helpers for error reporting. Adds a compile option QSERV_USE_HYRISE_SQL_PARSER for builds that use the new parser backend.
This commit enables the Hyrise parser and adapter for SELECT queries only, based on the QSERV_USE_HYRISE_SQL_PARSER option. Previously there were a few different entrypoints to the ANTLR parser, but these have been consolidated to and switched between parser backends with compile time checks.
For queries that do not involve SELECT, the UserQuery classes used a mix of regexes (most other queries) and the legacy ANTLR parser (SET and CALL). This change makes non-SELECT query handling consistent (regexes only) so these classes no longer depend on ANTLR when QSERV_USE_HYRISE_SQL_PARSER is enabled.
There are some non-semantic differences in the output produced by the Hyrise and ANTLR adapters, so this change adds a macro for normalizing test cases between the two backends and updates the test cases so that either parser can be tested.
Similarly to the other test suites, there are some non-semantic differences in how the Hyrise adapter renders Qserv IR. Since the differences were far more frequent than in other test modules, a separate Hyrise-specific test suite was developed. This commit toggles the IR test between Hyrise and ANTLR backends, adds more tests for edge cases that were found during development, and backports the new Hyrise tests to the ANTLR suite.
This executes many large queries to stress test the parser backend and adapter layer. It includes some pathological inputs that can cause the parser backend to consume significant processing time. An added benefit of this test is catching performance regressions; if a change makes a significant impact on SQL parsing performance, it will be more obvious due to this test.
Instead of throwing an error for SETs we cannot handle, this reverts to the previous behavior where all SETs were accepted (essentially a no-op if they did not refer to either of the debug/config variables). Removed test case noise and updated to reflect the new isSetGlobal() method.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds a new, optional SQL parser backend based on the Hyrise SQL parser (https://github.com/hyrise/sql-parser). The ANTLR4-based parser remains available as an option depending on whether
QSERV_USE_HYRISE_SQL_PARSERis enabled. Based on our testing and benchmarking (also see DM-53046) this change should improve query processing speed.A modified version of the Hyrise SQL parser that supports features we need in qserv (3-column identifiers,
HAVINGwithoutGROUP BY,MOD, backtick-quoted identifiers, a few more centered around compatibility with mySQL) was added as a submodule underextern, branch qserv-compat, here: https://github.com/lsst/hyrise-sql-parser/ . Ideally the general modifications should be possible to submit as PRs for merging upstream. At some point we may want to evaluate what mySQL-derived features we want to explicitly drop or support in qserv, as that may improve compatibility with the unmodified library.Primary functionality was implemented in
src/ccontrol/HyriseAdapter.cc, which serves two purposes:There were previously two entrypoints to the parser, through
ParseRunner::ParseRunner()andParseRunner::makeSelectStmt, but those have been largely consolidated so thatUserQueryFactorydrives the parsing logic and selects the appropriate parser backend at compile time.Finally, the test suite has been updated:
src/tests/ParserExpected.hhelper to simplify normalizing a few non-semantic differences between the two backends.The next milestone is adding integration tests that exercise the slow query paths that we observed previously to confirm performance has improved, and work to refactor / optimize the parse tree traversals for performance.