You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
There are multiple multi-character operators in SQL such as >=, <=, <>, and maybe others. Currently, each of them except for != is split into multiple segments, for example:
Besides the problem/inefficiency I listed above, another problem with that regexp is that it misses some single-character operators, for example & and |. See https://www.w3schools.com/sql/sql_operators.asp for a list.
For those two reasons, I wonder if instead of the current regexp, we should just look for all sequences of punctuation, excluding the quotation marks that start strings and identifiers:
/(?<special>[^\w\s"'`]+)/
Admittedly as I wrote in #149 it's sometimes hard to differentiate between subtraction and negative numbers, and the regexp above would exacerbate that problem in corner cases like AGE*-5. It would work fine with spaces though, i.e. AGE * -5.
Hey, sorry for not replying here. Simply put I haven't had that much time the last few weeks due to various reasons. But next week is vacation, maybe I'll have some time then 😄 No promises though
As @wkeese mentioned in the description a fix for this issue might (or more likely will) affect #149. I'm very keen on resolving the number detection issue, and I think that coming up with a good solution to that issue first would be the better approach here. Then we could look at how to adapt the matching logic introduced there to work with multi character operators.
To add my five cents to the conversation regarding different regexes, without having actually tested these myself I believe implementing the more generic regex suggested by @wkeese would generally be a better approach since it takes away a bit of the burden maintaining that character list. That being there might be a risk it's a bit too greedy, so the tests will have to tell if it will actually work.
I believe implementing the more generic regex suggested by @wkeese would generally be a better approach since it takes away a bit of the burden maintaining that character list. That being there might be a risk it's a bit too greedy, so the tests will have to tell if it will actually work.
You made a lot of good points.
ISTM the root issue, probably impractical to solve, is that + and - can be unary or binary operators. So regardless of whether we have an explicit list of operators, this can be tokenized two ways:
x<>-5
And so can this:
x-5
But those are edge cases due to the lack of spaces.
I do think it makes sense for number matching to take priority as that will do the right thing for common expressions like
I added #193, now we match all operators. Actually, it's using "special" as the bucket for anything that doesn't match the other regexps (number, identifier, string, etc.). But I added lots of tests and I've convinced myself that it isn't too greedy. In particular, that minus, plus, or dot before a digit is treated as part of the number.
The fix for this in #193 has been merged to the beta branch and will be released very soon. I'll close this a bit prematurely, but feel free to reopen if you feel it's not fixed.
Activity
scriptsbot commentedon Oct 20, 2023
wkeese commentedon Oct 20, 2023
parallels999 commentedon Jan 23, 2024
Hi, i have the same problem
Quick solution, add
&|^to regex for "special" segments/(?<special>!=|[=%*/\-+,;:<>&\|\^])/scriptcoded commentedon Feb 1, 2024
Hey, sorry for not replying here. Simply put I haven't had that much time the last few weeks due to various reasons. But next week is vacation, maybe I'll have some time then 😄 No promises though
PaolaRuby commentedon Mar 11, 2024
That works for me, but could you do a PR?
wkeese commentedon Mar 12, 2024
Wait, that doesn't address the main problem I listed in this issue. Why not just go with what I originally suggested?
scriptcoded commentedon Mar 12, 2024
As @wkeese mentioned in the description a fix for this issue might (or more likely will) affect #149. I'm very keen on resolving the number detection issue, and I think that coming up with a good solution to that issue first would be the better approach here. Then we could look at how to adapt the matching logic introduced there to work with multi character operators.
To add my five cents to the conversation regarding different regexes, without having actually tested these myself I believe implementing the more generic regex suggested by @wkeese would generally be a better approach since it takes away a bit of the burden maintaining that character list. That being there might be a risk it's a bit too greedy, so the tests will have to tell if it will actually work.
wkeese commentedon Mar 30, 2024
You made a lot of good points.
ISTM the root issue, probably impractical to solve, is that + and - can be unary or binary operators. So regardless of whether we have an explicit list of operators, this can be tokenized two ways:
x<>-5
And so can this:
x-5
But those are edge cases due to the lack of spaces.
I do think it makes sense for number matching to take priority as that will do the right thing for common expressions like
x > -5
wkeese commentedon Apr 2, 2024
PS: I added a PR #149 for the numbers. I points to the
betabranch since presumably the master branch is now just for bug fixes.fix: improve operator detection
fix: improve operator detection
fix: improve operator detection
wkeese commentedon Apr 13, 2024
I added #193, now we match all operators. Actually, it's using "special" as the bucket for anything that doesn't match the other regexps (number, identifier, string, etc.). But I added lots of tests and I've convinced myself that it isn't too greedy. In particular, that minus, plus, or dot before a digit is treated as part of the number.
fix: improve operator detection
fix: improve operator detection
fix: improve operator detection
scriptcoded commentedon Jul 2, 2024
The fix for this in #193 has been merged to the beta branch and will be released very soon. I'll close this a bit prematurely, but feel free to reopen if you feel it's not fixed.
fix: improve operator detection
chore(release): 5.0.0 [skip ci]
chore(release): 5.0.0 [skip ci]
chore(release): 6.0.0 [skip ci]
scriptsbot commentedon Jul 2, 2024
🎉 This issue has been resolved in version 6.0.0 🎉
The release is available on:
Your semantic-release bot 📦🚀