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

[FR] Add white space checking for KQL parse #3789

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

eric-forte-elastic
Copy link
Contributor

@eric-forte-elastic eric-forte-elastic commented Jun 14, 2024

Issues

#2700

Summary

This addresses an issue where lark parses KQL queries without whitespace around certain tokens, where KQL does not.

E.g. "Get-NetComputerSiteName" or "Get-NetLocalGroup" vs "Get-NetComputerSiteName" or"Get-NetLocalGroup". Both of which parse via lark/ANTLR, but the second fails in Kibana.

Some notes about alternative implementations:

  1. We don't want to update the grammar because we need to ignore white space
  2. We don't want to use regex pre-processing because it will catch things like "category" as invalid unavoidably

This approach adds a post-processing step to the lark parsing to tell us where the and and or tokens are in the original string, then compare to see if those tokens locations have the appropriate spacing.

Note since this PR updates the KQL lib please make sure to update the KQL lib version appropriately.

Contributor checklist

@brokensound77
Copy link
Contributor

This should probably be handled in the grammar instead

@eric-forte-elastic
Copy link
Contributor Author

eric-forte-elastic commented Jun 20, 2024

This should probably be handled in the grammar instead

The current grammar requires white space to be ignored. I think the way you are suggesting would require a refactor of both the grammar and the parsing to handle this. This would not only be a refactor/overhaul but in effect a full replacement as most of not all of the code would need to be updated compared to the relatively minor change I am suggesting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Missing Spaces Between Logic Operators Does Not Raise Error
3 participants