Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a way for Doltgres to inject expressions #309

Merged
merged 2 commits into from
Feb 7, 2024
Merged

Conversation

Hydrocharged
Copy link

@Hydrocharged Hydrocharged commented Feb 5, 2024

This primarily adds a new AST node that DoltgreSQL takes advantage of. All currently-existing nodes are specifically designed for MySQL transformations. In many cases, Doltgres will have different transformations that need to take place, and this provides a way for Doltgres to implement those transformations without having to modify other packages whenever a new transformation is added or edited.

Related PRs:

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

LGTM

go/vt/sqlparser/ast.go Outdated Show resolved Hide resolved
Copy link

@max-hoffman max-hoffman left a comment

Choose a reason for hiding this comment

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

I'm a bit worried about this skipping a phase of semantic resolution in ways that might be inconvenient if you end up needing those phases of semantic resolution. The way we do things like this in the engine right now is to make an interface with some dummy method that hooks into an extra lifecycle step. Postgres specific name for an expression seems unusual when it could be generic without loss of generality.

@Hydrocharged Hydrocharged merged commit c057d23 into main Feb 7, 2024
4 checks passed
@Hydrocharged Hydrocharged deleted the daylon/super-type branch February 7, 2024 12:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants