feat(sql): Support exclusive and half-open intervals in BETWEEN expressions #4779
Conversation
|
|
| public static final int INCLUSIVE_BETWEEN = 1; | ||
| public static final int EXCLUSIVE_BETWEEN = 2; | ||
| public static final int RIGHT_OPEN_BETWEEN = 3; | ||
| public static final int LEFT_OPEN_BETWEEN = 4; |
There was a problem hiding this comment.
Prefix with BETWEEN i.e BETWEEN_INCLUSIVE, BETWEEN_EXCLUSIVE
| @@ -184,6 +189,8 @@ public class QueryModel implements Mutable, ExecutionModel, AliasTranslator, Sin | |||
| private QueryModel updateTableModel; | |||
| private TableToken updateTableToken; | |||
| private ExpressionNode whereClause; | |||
| // exclusive and inclusive with between | |||
| private int betweenType = INCLUSIVE_BETWEEN; | |||
There was a problem hiding this comment.
Default should be 0 or -1 since not every query has a BETWEEN statement in it
| @@ -433,6 +440,7 @@ public void clear() { | |||
| explicitTimestamp = false; | |||
| showKind = -1; | |||
| sampleByOffset = ZERO_OFFSET; | |||
| betweenType = INCLUSIVE_BETWEEN; | |||
There was a problem hiding this comment.
Default should be 0 or -1 since not every query has a BETWEEN statement in it
| tok = tok(lexer, "'inclusive' 'exclusive' 'left' 'open' 'right'"); | ||
|
|
||
| if(SqlKeywords.isInclusiveKeyword(tok)) { | ||
| // for INCLUSIVE | ||
| } else if(SqlKeywords.isExclusiveKeyword(tok)) { | ||
| // for EXCLUSIVE | ||
| } else if (SqlKeywords.isRightKeyword(tok)) { | ||
| if (SqlKeywords.isOpenKeyword(tok)) { | ||
| // for RIGHT OPEN | ||
| } | ||
| } else if (SqlKeywords.isLeftKeyword(tok)) { | ||
| if (SqlKeywords.isOpenKeyword(tok)) { | ||
| // for LEFT OPEN | ||
| } | ||
| } else { | ||
| throw SqlException.$(lexer.lastTokenPosition(), "''inclusive' 'exclusive' 'left' 'open' 'right' expected"); | ||
| } |
There was a problem hiding this comment.
Good start here.
Check the else clause - a keyword is not required at this point, it could just be BETWEEN x AND y which is would be equivalent to BETWEEN x AND y INCLUSIVE.
For RIGHT OPEN and LEFT OPEN, you'll need to progress the lexer with methods like tok or optTok.
| @@ -1195,6 +1208,14 @@ public static boolean isOnlyKeyword(CharSequence tok) { | |||
| && (tok.charAt(3) | 32) == 'y'; | |||
| } | |||
|
|
|||
| public static boolean isOpenKeyword(CharSequence tok) { | |||
| return tok.length() == 9 | |||
| public void setBetweenType(int betweenType) { | ||
| this.betweenType = betweenType; | ||
| } | ||
|
|
||
| @Override | ||
| public void toSink(@NotNull CharSink<?> sink) { |
There was a problem hiding this comment.
You should add to this to make sure the extra keywords (exclusive, right open etc.) are written out into the model when its printed.
If you step through the code with a debugger, you can quickly view the model using QueryModel.toString0()
There was a problem hiding this comment.
I didn't quite get this one.
There was a problem hiding this comment.
We run some tests by parsing the SQL into a QueryModel and then printing this model out for comparison.
You can update toSink() to make it print out your new additions. This will help you to write tests to make sure your new parsing logic works as expected.
See SqlParserTest which has tests asserting against the QueryModel.toSink() output.
There was a problem hiding this comment.
Hey, while testing I am getting this error repeatedly.
The type jdk.internal.math.FDBigInteger is not accessible
and other errors related to jdk.internal.
There was a problem hiding this comment.
Can you give more context? Like a stack trace or info about where it is being thrown?
There was a problem hiding this comment.
I used this and tried to write tests and update the parser but I can't test because of these errors. Should I build the project again, but I'll lose all the changes? How do I proceed? Do you have any suggestions
There was a problem hiding this comment.
I'm not sure wy this is being thrown. Perhaps its related to Java versions, Coretto 17 should work.
You could commit and push, and then delete the directory and re-clone it.
There was a problem hiding this comment.
Hey, I did reclone it, but it still shows the same errors, I think I need to clone it from the main repo, since there are only 60-70 lines of changes I can redo them after cloning the main repo. But I am not sure about this pr, do I need to create a new one and delete this one? Also, I am sorry for not doing anything in the past week as I was busy with my school exams. Thank you!
There was a problem hiding this comment.
Do whatever is most convenient, and no problem about the delays :)
There was a problem hiding this comment.
I will close this pr and create a new one!!!
No description provided.