Skip to content
This repository was archived by the owner on May 17, 2024. It is now read-only.

Compile all AST elements always via dialects, never directly #713

Merged
merged 4 commits into from
Sep 28, 2023

Conversation

nolar
Copy link
Contributor

@nolar nolar commented Sep 25, 2023

This PR inverses the flow of AST-to-SQL compilation.

Previously, it was going from the root code -> database -> compiler -> AST nodes, which in turn sometimes (but not always) redirected back to the compiler -> dialect for some pieces of SQL.

As a result, the SQL logic was spread over the code base and was difficult to override per database.

Now, the root code goes -> database -> compiler -> dialect directly. The dialect knows how to render all SQL. Specifically, the base dialect provides the core SQL for all databases, with each and every AST node being possible to override. The AST nodes know nothing about SQL, they only express our intentions of what should be executed, not how.

A very simple chart:
image

@nolar nolar requested a review from dlawin September 25, 2023 15:49
Sergey Vasilyev added 4 commits September 28, 2023 16:37
The root authority on how to do the SQL syntax properly is the dialect — not the AST element itself. AST tree must only carry the intentions of what we want to execute, but not how it should/could be executed.

This makes the AST truly independent of dialects and databases, allowing us to:
- Focus on the main logic regardless of the SQL capabilities.
- Create custom database connectors without involving AST changes every time.

Before the change, adding a new database connector with unusual syntax would often require changing the AST elements to direct to `compiler.dialect.some_method()` — with only a subset of SQL being described in dialects. The other rather arbitrary part of SQL syntax was hard-coded in AST elements and could not be easily overridden without such changes.

After the change, all the SQL logic is concentrated in one hierarchy of dialects, mostly in one base class.
@nolar nolar force-pushed the simplify-ast-compilation branch from cc66fd4 to 795bb0e Compare September 28, 2023 14:38
@nolar nolar force-pushed the simplify-squash-sqeleton branch from 8f36a70 to 871c201 Compare September 28, 2023 14:39
Base automatically changed from simplify-squash-sqeleton to master September 28, 2023 15:36
@nolar nolar merged commit f3dc52b into master Sep 28, 2023
@nolar nolar deleted the simplify-ast-compilation branch September 28, 2023 15:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants