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

Better SQL query splitter #3791

Merged
merged 1 commit into from
Jan 24, 2023

Conversation

rodrigozhou
Copy link
Collaborator

What changed?
Implemented a better SQL query splitter. It splits the statements taking into consideration BEGIN/END blocks.

Why?
The current algorithm simply splits by semicolon (;), which would lead to wrong query statements when there are more complex statements that contains BEGIN/END blocks (eg: when creating a trigger).

How did you test it?
Wrote unit tests.

Potential risks
No issues, it parses the existing SQL files correctly.

Is hotfix candidate?
No.

@rodrigozhou rodrigozhou requested a review from a team as a code owner January 10, 2023 01:46
@MichaelSnowden
Copy link
Contributor

Could we use a library for this? The code here seems pretty complicated.

@rodrigozhou
Copy link
Collaborator Author

@MichaelSnowden I did a quick search before, and I didn't find anything for our use case.

@rodrigozhou rodrigozhou force-pushed the better-query-splitter branch 2 times, most recently from be4ca7d to ad885d7 Compare January 24, 2023 01:24
tools/common/schema/util.go Outdated Show resolved Hide resolved
common/persistence/query_util.go Outdated Show resolved Hide resolved
common/persistence/query_util.go Outdated Show resolved Hide resolved
common/persistence/query_util.go Show resolved Hide resolved
common/persistence/query_util.go Outdated Show resolved Hide resolved
common/persistence/query_util_test.go Show resolved Hide resolved
@rodrigozhou rodrigozhou merged commit 3abd50d into temporalio:master Jan 24, 2023
@rodrigozhou rodrigozhou deleted the better-query-splitter branch January 24, 2023 20:01
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.

4 participants