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

Refactor SQL query converter #3950

Merged

Conversation

rodrigozhou
Copy link
Contributor

What changed?
Refactoring SQL query converter:

  • Better error messages.
  • Util function to extract values from a SQL tuple.
  • Rewrite convertInExpr in PostgreSQL query converter: change to non-recursive function.

Why?

  • Better errors messages.
  • Less code duplication.
  • Better performance with non-recursive function.

How did you test it?
Existing tests.

Potential risks
No risks.

Is hotfix candidate?
No.

@rodrigozhou rodrigozhou requested a review from a team as a code owner February 14, 2023 00:21
@rodrigozhou rodrigozhou force-pushed the refactor-sql-query-converter branch 2 times, most recently from 54b7675 to 0083201 Compare February 14, 2023 00:23
Copy link
Member

@alexshtin alexshtin left a comment

Choose a reason for hiding this comment

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

I would love to see tests for this code.

@rodrigozhou rodrigozhou force-pushed the refactor-sql-query-converter branch from 0083201 to 2b98ab4 Compare February 15, 2023 01:05
@rodrigozhou rodrigozhou merged commit 5d6d93c into temporalio:master Feb 15, 2023
@rodrigozhou rodrigozhou deleted the refactor-sql-query-converter branch February 15, 2023 01:40
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