fix: move sql operators to include file #3901
Conversation
|
Does the logical operators need to be moved too? |
fzipi
left a comment
There was a problem hiding this comment.
Very good question. What do you think is better?
regex-assembly/942520.ra
Outdated
| ##! sql operators | ||
|
|
||
| [|&<>*\/%=^+-]{1,3} |
There was a problem hiding this comment.
| ##! sql operators | |
| [|&<>*\/%=^+-]{1,3} | |
| ##!> include sql-operators | |
There was a problem hiding this comment.
Why do we have there the *, % and ^ chars? Are those valid part of the sql language operators 🤔 ?
There was a problem hiding this comment.
Those characters are causing tests to fail. Which is why I revert my initial commit.
There was a problem hiding this comment.
Also, if you read a little above, a url points to the same SQLITE operators page I mentioned earlier.
There was a problem hiding this comment.
Looks like those are valid operators for MySQL: https://dev.mysql.com/doc/refman/8.4/en/non-typed-operators.html
There was a problem hiding this comment.
Looks like we've to add *, ^, %, |, +, - to fix the tests for 942520. What do you say, should we add them alongwith other MySQL operators? Will adding single chars cause FPs?
There was a problem hiding this comment.
Also, the main question is should we include the logical operators too. I think we should, including logical operators for other engines too. But then, what about 942120.ra? Only the include statement will be left. 😄
There was a problem hiding this comment.
@fzipi I had to remove some tests to fix the regression tests error. Is it okay with you, as some of those operators are for mysql?
|
I'm not convinced that moving these operators to an include file makes much sense. It's a very small set of tokens that can be shared between only a few rules. In addition, the |
Co-authored-by: Felipe Zipitría <3012076+fzipi@users.noreply.github.com>
|
📊 Quantitative test results for language: |
|
Ping @fzipi |
|
Sure, let's not move then and keep it here. |
|
Sorry for wasting your time @Xhoenix 😞 |
No description provided.