Add GremlinLang class for JavaScript GLV#3304
Add GremlinLang class for JavaScript GLV#3304kirill-stepanishin wants to merge 9 commits intoapache:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3304 +/- ##
============================================
- Coverage 77.87% 75.87% -2.00%
+ Complexity 13578 13272 -306
============================================
Files 1015 1020 +5
Lines 59308 59852 +544
Branches 6835 7034 +199
============================================
- Hits 46184 45412 -772
- Misses 10817 11816 +999
- Partials 2307 2624 +317 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| describe('P and TextP predicates', function () { | ||
| it('should handle P.gt predicate', function () { | ||
| const gremlinLang = new GremlinLang(); | ||
| assert.strictEqual(gremlinLang.addStep('has', ['age', P.gt(5)]).getGremlin(), "g.has('age', gt(5))"); |
There was a problem hiding this comment.
Nit: Some of these test cases don't represent valid gremlin, in this case it should likely be g.V().has('age', gt(5)) as has() is not a start step. I don't expect GremlinLang to do this sort of validation as this is already covered by GraphTraversal and the grammar, however it is preferred to use valid examples here.
|
|
||
| it('should handle P.within with array', function () { | ||
| const gremlinLang = new GremlinLang(); | ||
| assert.strictEqual(gremlinLang.addStep('V', [P.within([1, 2, 3])]).getGremlin(), "g.V(within([1, 2, 3]))"); |
There was a problem hiding this comment.
Similar to above, this gremlin isn't quite right, as V() doesn't take predicates to match ids. Something like this would be preferred.
| assert.strictEqual(gremlinLang.addStep('V', [P.within([1, 2, 3])]).getGremlin(), "g.V(within([1, 2, 3]))"); | |
| assert.strictEqual(gremlinLang.addStep('V').addStep('has', [T.id, P.within([1, 2, 3])]).getGremlin(), "g.V().has(T.id,P.within([1,2,3]))"); |
|
|
||
| it('should handle chained P predicates', function () { | ||
| const gremlinLang = new GremlinLang(); | ||
| assert.strictEqual(gremlinLang.addStep('has', ['age', P.gt(5).and(P.lt(10))]).getGremlin(), "g.has('age', and(gt(5), lt(10)))"); |
There was a problem hiding this comment.
This looks like P.and is getting mixed up with the and() step here. The expected GremlinLang should be
| assert.strictEqual(gremlinLang.addStep('has', ['age', P.gt(5).and(P.lt(10))]).getGremlin(), "g.has('age', and(gt(5), lt(10)))"); | |
| assert.strictEqual(gremlinLang.addStep('V').addStep('has', ['age', P.gt(5).and(P.lt(10))]).getGremlin(), "g.V().has("age",gt(5).and(lt(10)))"); |
| private _argAsString(arg: any): string { | ||
| if (arg === null || arg === undefined) return 'null'; | ||
| if (typeof arg === 'boolean') return arg ? 'true' : 'false'; | ||
| if (typeof arg === 'number') return String(arg); |
There was a problem hiding this comment.
We should also handle utils.Long here to cover integers which fit into Gremlin/Java's long, but are too big for javascripts number.
We will also need to further consider what to do with the small number types like byte, short, and float. There isn't necessarily a nice answer as the numeric types just don't map well into JS. I think we can leave that out of scope for this PR, but we will have to face it before we can get all of the feature tests running again.
|
|
||
| it('should handle array arguments', function () { | ||
| const gremlinLang = new GremlinLang(); | ||
| assert.strictEqual(gremlinLang.addStep('V', [[1, 2, 3]]).getGremlin(), 'g.V([1, 2, 3])'); |
There was a problem hiding this comment.
Can you add an extra case to cover "special numbers" Infinity, -Infinity and NaN?
| if (entries.length === 0) return '[:]'; | ||
| return '[' + entries.map(([k, v]) => `${this._argAsString(k)}:${this._argAsString(v)}`).join(', ') + ']'; | ||
| } | ||
| return String(arg); |
There was a problem hiding this comment.
There are a few missing types which will need cases here (and tests):
- Date
- Set
- Map
UUID is also missing however I think this can be ignored for now as I think it needs to be completely redone in JS for TP4.
3 participants
Description:
As part of the TP4 migration, the server no longer accepts Bytecode and only processes Gremlin queries in script form. This PR introduces the GremlinLang class, which builds Gremlin script strings incrementally.
This covers the core class and unit tests only. It does not yet integrate with GraphTraversal, GraphTraversalSource, or the driver layer.
GremlinLang supports:
Follow-up PR will wire GremlinLang into GraphTraversal/GraphTraversalSource, update the driver to submit string queries, add nested traversal support, and delete Bytecode/Translator.