-
Notifications
You must be signed in to change notification settings - Fork 351
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
Add QueryExpression.LENGTH and QueryExpression.CONCAT operators #2306
base: dev
Are you sure you want to change the base?
Conversation
Note: this shows three commits, but the first two are a forward and backward commit because I did a previous update on dev rather than creating a new branch as I should. The six files that are changed are indeed the six files that belong to this PR. |
@danielabutano I need someone that understands the query testing system to fix the Travis test errors. I'm completely confused about where the error occurs and why. I've tried a couple times to fix it but I'm stabbing in the dark. Thanks. :) Not a high-priority PR in any case. |
@sammyjava at the moment is the checkstyle which is failing, the tests have not been executed yet (the tests are executed after the checkstyle check). Looking at the commit you did, I noticd a lot of spaces not required or identation issues |
@danielabutano Working on the checkstyle, but I know that I'll not be able to fix these subtle tests on the new query expressions. At least not without a massive amount of study. :) But I'll get the checkstyle cleaned up so that's all that's left. |
@danielabutano I fixed checkstyle, but it'll still error on the tests which mystify me. But it's ready for someone who understands those tests to fix. |
Details
This adds the LENGTH and CONCAT operators to QueryExpression, which can be useful. They were for me. :)
Testing
This update has been in production on the LIS mines for quite a while.
Checklist
Before your pull request can be approved, be sure to check all boxes: