-
Notifications
You must be signed in to change notification settings - Fork 19
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
w3c/sparql-12#101 SEP-0003 for re-introducing min/max #116
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great to me; mainly some comments about the semantics.
In the SPARQL 1.1. working group, a form of bounded property path where supported in the late [drafts](https://www.w3.org/TR/2012/WD-sparql11-query-20120105/#propertypaths). The form being a path element _elt_, followed by a curly bracket enclosed min/max pattern. | ||
|
||
||Syntax Form||Matches|| | ||
| _elt_{n,m} | A path of between n and m occurrences of elt. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume m
is inclusive here?
An option would be to use the [n,m]
notation in the description in this table (also for the next forms).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The july 2012 spec, did not say but it is something that should be written down. For consistency with other functions, I think it should be inclusive.
| _elt_{n,m} | A path of between n and m occurrences of elt. | | ||
| _elt_{n} | A path of exactly n occurrences of elt. | | ||
| _elt_{n,} | A path of n or more occurrences of elt. | | ||
| _elt_{,n} | A path of between 0 and n occurrences of elt. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just wondering if would be valuable to have this mean "A path of between 1 and n occurrences of elt" instead?
I've noticed that an implementation of the zero-case can be tricky to optimize, while users may not really need it this often(?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to agree that the zero-length case is often not what is wanted (especially in cases where both subject and object are variables), and requiring the full {0,m}
form to get those semantics would probably be desirable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imho, it would be a bit strange to have {,n}
mean {1,n}
and not {0,n}
because 1 is not the lower bound of the possible values. What about not allowing {,n}
? {1,n}
and {0,n}
are just one key press away from {,n}
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Tpt on reflection, I think being explicit is better than being implicit so disallowing the form {,n}
seems sensible. Adding {n,*}
to mean at least n hops seems sensible to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! My personal opinion would be to keep {n,}
instead of {n,*}
in order to avoid adding an extra symbol and use the same syntax as a lot of regex languages.
Fix typo Co-authored-by: Ruben Taelman <rubensworks@users.noreply.github.com>
Co-authored-by: Ruben Taelman <rubensworks@users.noreply.github.com>
|
||
|Syntax Form|Matches| | ||
|-----------|-----------| | ||
| _elt_{n,m} | A path of between n and m occurrences of elt. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To discuss? must m
and n
be a constant or are variables allowed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Supporting variables would require a major change to the evaluation semantics.
Could we have discussions about the proposal on issue #101? This keeps things together. We can merge draft SEPs and PRs to SEPs without losing important input. |
Shall we merge this as "working draft" and continue work using issue #101 for the design, with PR's for updates + editorial discussion? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO Ready to merge.
Re-Introduce the property path min-max syntax in a non counting form.