change Math operator to support unary operations, move outside string… #2971
Conversation
🦋 Changeset detectedLatest commit: 52cfd1e The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks for the UI updates. The UI has now been torn down - reopening this PR will republish it. |
Performance Report
🟥 - Performance worsened (dbHits) Show Full Table
Old Schema Generation: 29.024s |
| @@ -71,7 +71,7 @@ describe("CypherBuilder With", () => { | |||
| }); | |||
|
|
|||
| test("With expression aliased", () => { | |||
| const expr = Cypher.plus(new Cypher.Param("The "), new Cypher.Param("Matrix")); | |||
| const expr = Cypher.append(new Cypher.Param("The "), new Cypher.Param("Matrix")); | |||
There was a problem hiding this comment.
Based on the comment above the export, do we want to consider exporting these functions under their namespace?
| const expr = Cypher.append(new Cypher.Param("The "), new Cypher.Param("Matrix")); | |
| const expr = Cypher.string.concat(new Cypher.Param("The "), new Cypher.Param("Matrix")); |
This would also apply to math operators:
Cypher.math.multiply(literal1, literal2);There was a problem hiding this comment.
I was thinking something the same, but I was not sure what should be exported under a different namespace and what to export from the root namespace!
There was a problem hiding this comment.
@angrykoala, any thoughts on the above?
There was a problem hiding this comment.
As discussed with @MacondoExpress
I'm quite heavily opinionated in keeping this as plain operators, we do not really have the semantic knowledge to know if a certain expression is a string or a number, meaning that we cannot really ensure that we are concatenating strings or adding numbers, which could lead to errors on the users
|
Is this still relevant @MacondoExpress ? |
|
No, I'll close it in favor of this PR: #3153 |
Add Math operators, separate string concat
Add other operators to the Math operators and separate the string.concat implementation from the math.plus.
This is a breaking change as the plus and minus are now treated as unary plus and unary minus and while the previous behavior is moved to
add/subtractrespectively.Complexity
Complexity: Low
Checklist
The following requirements should have been met (depending on the changes in the branch):