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

RD-14898: SqlCompiler crashes when running 'hover' against a query that has two statements #516

Conversation

bgaidioz
Copy link
Collaborator

What happens is specific to LSP and hover.

  • LSP calls do not fail when the ANTL4 parser has detected failures because we like to perform completion and the like even with syntax errors.
  • Hover has a specificity that if hovering an argument like :iata, we need to perform some code semantic analysis to compute the type of the argument. This involves passing the query to postgres to let it do its own inference and checks (sometimes the type is totally inferred by postgres).
  • When there are syntax errors, we'll get errors and postgres won't be able to infer, fine. But having two statements isn't always an error (they can be comments) so we just go and try, but after cutting the code at the first statement.

The bug we hit is that the parsed tree contains references to arguments like :iata when they're located in the second statement, and when we try to construct the postgres query (the one that contains ? in place of those), we get references to arguments that are in portions of code we don't have (since it was cut).

I could work a first fix where the processing is careful with arguments and ignores those that are outside the code, but that is a little dangerous, there were several places to fix. I thought the parser could just skip those when they wouldn't belong to the first statement.

@bgaidioz bgaidioz merged commit 4251a72 into main Sep 20, 2024
7 checks passed
@bgaidioz bgaidioz deleted the RD-14898-execution-crash-report-com-rawlabs-compiler-compiler-service-exception branch September 20, 2024 07:50
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