Paths keyword added to sparql algebra #1
Conversation
|
Local verification is always failing, even on fresh clones. @keiji-numata and I tried it, but locally tests are failing, we also opened a new issue regarding the same on the original repo. |
|
@psvkaushik I did a clean checkout of the original repo and didn't have any errors running tests. I'm using node v21.7.1 and ran these commands:
|
|
I enabled the CI action here, so I will close and reopen the PR to trigger it. |
lib/algebra.ts
Outdated
| start?: rdfjs.Variable | IriTerm; | ||
| via?: rdfjs.Variable | IriTerm; | ||
| end?: rdfjs.Variable | IriTerm; |
There was a problem hiding this comment.
start, end, and via should not be optional, according to the grammar.
Keep start and end as required type rdfjs.Variable, and add two more properties like:
startPattern?: IriTerm | Bgp
endPattern?: IriTerm | Bgp
I'm not sure if Bgp is the best match for <GRAPH PATTERN> from the grammar. Maybe check to see what the parser returns for WHERE clause in SELECT.
via should have type rdfjs.Variable | PropertyPathSymbol | Bgp (same uncertainty about Bgp).
There was a problem hiding this comment.
Or maybe the graph pattern is just Operation.
lib/algebra.ts
Outdated
| @@ -1,5 +1,5 @@ | |||
| import * as rdfjs from '@rdfjs/types'; | |||
| import { Wildcard } from 'sparqljs'; | |||
| import { IriTerm, Wildcard } from 'sparqljs-nrt'; | |||
There was a problem hiding this comment.
I think we need to figure out the workflow for depending on a fork rather than changing the import name everywhere. This will be inconvenient later.
package.json
Outdated
| @@ -23,7 +23,8 @@ | |||
| "rdf-isomorphic": "^1.3.0", | |||
| "rdf-string": "^1.6.0", | |||
| "rdf-terms": "^1.10.0", | |||
| "sparqljs": "^3.7.1" | |||
| "sparqlalgebrajs": "^4.3.7", | |||
There was a problem hiding this comment.
Changes to the variables made as you've suggested. Also removed the self-dependency
lib/algebra.ts
Outdated
| start: IriTerm ; | ||
| via: IriTerm; | ||
| end: IriTerm ; |
There was a problem hiding this comment.
Check my previous review, and also look through the PATHS docs again; start and end must be variables. Another property is required to hold the optional IRIs.
|
Closing this PR in favor of keeping a separate feature branch going. |
No description provided.