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

Add an interpreter base for function creation #1158

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

Hydrocharged
Copy link
Collaborator

@Hydrocharged Hydrocharged commented Jan 30, 2025

This adds the base of an interpreter, upon which CREATE FUNCTION and CREATE PROCEDURE will convert their contents to. Only the interpreter portion is partially implemented, along with a temporary example function and accompanying test to ensure that it works.

The next major step is to add parsing support for all of the statements that we'll need, as they do not yet exist (hence why the temporary example function's operations were manually created, although the conversion should result in the same output). The last major step will be to convert from the PL/pgSQL code to the operations that our interpreter expects.

The original intention was to implement as much of this in GMS as possible, however for now, the bulk of it will exist in Doltgres as I couldn't quite reconcile the proper way to implement it there without diving more fully into MySQL's CREATE FUNCTION and CREATE PROCEDURE support (rather than simply relying on memory). This is something that I'll revisit in a later PR, as I feel it'll be easier to do once I have a relatively complete implementation done in Doltgres.

Related PR:

@Hydrocharged Hydrocharged requested a review from zachmu January 30, 2025 12:42
@Hydrocharged Hydrocharged force-pushed the daylon/create-function branch from 3ee3aa2 to 205464c Compare January 30, 2025 12:52
Copy link
Contributor

github-actions bot commented Jan 30, 2025

Main PR
covering_index_scan_postgres 351.49/s 360.50/s +2.5%
index_join_postgres 153.95/s 154.08/s 0.0%
index_join_scan_postgres 183.54/s 184.39/s +0.4%
index_scan_postgres 12.70/s 12.63/s -0.6%
oltp_point_select 2792.98/s 2796.80/s +0.1%
oltp_read_only 1878.47/s 1886.05/s +0.4%
select_random_points 112.34/s 113.69/s +1.2%
select_random_ranges 127.71/s 126.48/s -1.0%
table_scan_postgres 11.88/s 11.98/s +0.8%
types_table_scan_postgres 5.67/s 5.65/s -0.4%

Copy link
Contributor

github-actions bot commented Jan 30, 2025

Main PR
Total 42090 42090
Successful 15537 15537
Failures 26553 26553
Partial Successes1 5226 5226
Main PR
Successful 36.9138% 36.9138%
Failures 63.0862% 63.0862%

Footnotes

  1. These are tests that we're marking as Successful, however they do not match the expected output in some way. This is due to small differences, such as different wording on the error messages, or the column names being incorrect while the data itself is correct.

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.

Overall seems pretty reasonable, a good start.

Main comment is that skipping backwards iteratively seems like an unnecessary complication, it should be possible to keep track of statements and jump to them directly.

server/functions/framework/interpreter_operation.go Outdated Show resolved Hide resolved
server/functions/framework/interpreter_logic.go Outdated Show resolved Hide resolved
server/functions/framework/interpreter_operation.go Outdated Show resolved Hide resolved
server/functions/framework/interpreter_operation.go Outdated Show resolved Hide resolved
server/functions/framework/interpreter_stack.go Outdated Show resolved Hide resolved
@Hydrocharged Hydrocharged force-pushed the daylon/create-function branch from 205464c to 6cd631f Compare February 4, 2025 11:15
@Hydrocharged Hydrocharged merged commit 9c3900c into main Feb 4, 2025
14 checks passed
@Hydrocharged Hydrocharged deleted the daylon/create-function branch February 4, 2025 11:42
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.

2 participants