hue icon indicating copy to clipboard operation
hue copied to clipboard

[frontend] Advance the 'AND' rule in the definition of 'between' syntax for Hive and Presto

Open laochake opened this issue 2 years ago • 3 comments

SQL parser: Move the ' AND' rule in the lexical definitions of Hive and Presto ahead of the 'AND' rule to avoid auto-completion errors in cases like "where x between 'a' and 'b' a". The keyword 'AND' with state 'between' is treated as a normal 'AND'. Other SQL syntax such as Flink, SQL, etc. are correct.

What changes were proposed in this pull request?

Optimize the completion of syntax containing 'where x between 'a' and 'b''.

  • (Please fill in changes proposed in this fix) Move the order of ' 'AND' {this.popState(); return 'BETWEEN_AND';}' in advance to before 'AND' in the hive and presto sql.jisonlex files, and regenerate the parser.

How was this patch tested?

  • (Please explain how this patch was tested. Ex: unit tests, manual tests) npm run test -- hiveAutocompleteParser.test.js

  • (If this patch involves UI changes, please attach a screen-shot; otherwise, remove this)

Please review Hue Contributing Guide before opening a pull request.

laochake avatar Nov 28 '23 03:11 laochake

@laochake Thanks for contributing! @JohanAhlen can you have a look?

bjornalm avatar Nov 29 '23 11:11 bjornalm

gentle ping. @JohanAhlen

laochake avatar Dec 15 '23 02:12 laochake

This PR is stale because it has been open 45 days with no activity and is not labeled "Prevent stale". Remove "stale" label or comment or this will be closed in 10 days.

github-actions[bot] avatar Feb 10 '24 01:02 github-actions[bot]

Hi @laochake and sorry for our slow response here. I think this change makes sense. I see that you tested by running hiveAutocompleteParser.test.js but it doesn't contain all the test cases. I tried running hiveAutocompleteParser.Select.test.js with you change and it also passed. Could you perhaps update that test file with your specific issue (i.e. write a test that fails without your fix) and we will make sure to get this merged asap? Much appreciated and thanks for your patience.

bjornalm avatar Feb 21 '24 09:02 bjornalm