Skip to content

Fix: Backticks being removed causing mysql queries to fail#91

Merged
hardikgu23 merged 3 commits intomainfrom
fixBacktickIssue
Nov 14, 2024
Merged

Fix: Backticks being removed causing mysql queries to fail#91
hardikgu23 merged 3 commits intomainfrom
fixBacktickIssue

Conversation

@hardikgu23
Copy link
Copy Markdown
Collaborator

@hardikgu23 hardikgu23 commented Nov 14, 2024

We were removing ` from all sql queries before executing them causing some mysql queries to fail when table name consists of multiple words.
See examples here: https://dashboards.corp.google.com/embed/cloud_databases_nl2sql_dashboards_dash_specs_run_details?p=run_id:ee1e52bb-4a5e-4acf-bf2a-61c32c3b7f07

Skipping this step when prompt generator is NOOP

Copy link
Copy Markdown
Collaborator

@mahyareb mahyareb left a comment

Choose a reason for hiding this comment

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

Good find!
We actually shouldn’t modify / sanitize the sql at all. Could you just add an if statement here that skips all the sanitization if the prompt generator is the NOOP one. (What we use for SQLGen)

Copy link
Copy Markdown
Collaborator

@mahyareb mahyareb left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks Hardik!

@hardikgu23 hardikgu23 merged commit f1acba6 into main Nov 14, 2024
@hardikgu23 hardikgu23 deleted the fixBacktickIssue branch November 14, 2024 17:56
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