change: allow blank nodes to be inserted in N3 Patches #711
Conversation
There was a problem hiding this comment.
Some comments and a question before I can proceed with a technical review:
In https://lists.w3.org/Archives/Public/public-solid/2025Jan/0028.html , I asked:
Has the Solid CG recently decided to continue working on (some or all) items that are also part of the LWS WG charter deliverables?
The position of the Solid CG was stated in https://lists.w3.org/Archives/Public/public-solid/2025Jan/0044.html :
The general understanding is that only the core Solid Protocol report is currently an input of the WG.
My understanding of this is that the Solid Protocol is not something the Solid CG will continue to work on. However, this PR, along with the decision made during the Solid CG meeting on 2025-02-05 (minutes under review), appears to conflict with that understanding as per the Solid CG charter's Out of Scope section:
Community Group’s Work Items that have transitioned to a deliverable of an active Working Group.
This is further complicated by an unnecessary/redundant "decision" - since the charter had already ruled it out of scope - made in the 2024-09-11 meeting, literally:
PROPOSAL: CG stops working on the Solid Protocol draft
- MdJ: +1
- eP: +1
- PAC: +1
RESOLUTION: CG stops working on the Solid Protocol draft
This sends mixed signals. I ask once again: Is the Solid CG working on reports like the Solid Protocol or not?
- If yes, the CG's "final" decision should also be reflected in the CG charter and communicated to LWS WG. I'm happy to review if that understanding is made clear, with prior consent to review and edit.
- If not, then this PR can remain open but unmerged (while still collecting data, additional implementation reports, reviews etc.)
Please also note that as per Solid CG Contribution Guidelines, I believe this PR qualifies as a Class 3 change. If you believe it falls under a different category, please indicate that in the PR comments. Assuming it is a Class 3 change, it would be best to follow the guidelines to processing PRs - in this case, allowing "within 10 days or 2 meetings" to provide sufficient time for review.
Good shout - I've updated the timeline in the original message.
My interpretation (including the 2024-09-11 resolution) is that the CG should focus energy on deliverables outside of LWS scope; and thus not be actively working on the CG spec - however, this does not mean that the CG spec is locked. This change would fix an active blocker in the solidcommunity.net migration of NSS -> CSS, and land before the LWS starts to actively use the input documents such as the CG spec - since LWS is still in use-case and requirements stages; so pragmatically I think it is a good idea for the change to be made now if there are no objections on technical grounds. |
|
I would propose to keep the issue focused on technical conversation and move all the process-related chat to mailing list thread https://lists.w3.org/Archives/Public/public-solid/2025Feb/0019.html |
|
Jesse, I find your interpretation of the charter's out of scope section and the resolution "CG stops working on the Solid Protocol draft" as meaning the spec isn't locked rather unusual. If the intent was simply to indicate that the spec isn't locked so to speak, there could've been many other, clearer ways to express that. See also this bit leading to that proposal/resolution:
But, who exactly shouldn't be actively working on it? https://github.com/solid/specification/commits/main/ED/protocol.html And if that's the case, isn't it strange that only three people in the entire CG voted "CG stops working on the Solid Protocol draft" on the spot, without consulting the actual primary contributors to the specification? I'm not asking you to justify it. If the intent wasn't to lock, what did the resolution even achieve given CG charter's out of scope? This seems part of a broader pattern, particularly with the reluctance to move things forward in the Solid CG before the LWS WG charter was approved. For instance, see the concerns as mentioned here: Pavlik, there's no reason why process-related comments can't be made here especially in this case when I'm raising concerns about mixed signals regarding the process for handling updates to certain specs. And when merge dates are being mentioned, it's better to err on the side of caution. In any case, I've proposed a simple approach for mutual understanding between the LWS WG and Solid CG here: https://lists.w3.org/Archives/Public/public-lws-wg/2025Feb/0000.html That'd be all I have to say on process here and happy to follow up once the mutual understanding is accepted/rejected. |
There was a problem hiding this comment.
Leaning on SPARQL Update and SPARQL Query for some guidance pertaining to INSERT and blank nodes, SPARQL Update doesn't prohibit INSERT's with blank nodes, but there are some expectations on how they are applied, e.g.:
https://www.w3.org/TR/2013/REC-sparql11-update-20130321/#insertData states:
Blank nodes in QuadDatas are assumed to be disjoint from the blank nodes in the Graph Store, i.e., will be inserted with "fresh" blank nodes.
Only blank nodes seem to be disallowed in DELETE, https://www.w3.org/TR/2013/REC-sparql11-query-20130321/#sparqlGrammar :
- Blank node syntax is not allowed in DELETE WHERE, the DeleteClause for DELETE, nor in DELETE DATA.
- Rules for limiting the use of blank node labels are given in section 19.6.
https://www.w3.org/TR/2013/REC-sparql11-query-20130321/#grammarBNodes :
The same blank node label can not be used in:
two INSERT DATA operations within a single SPARQL Update request
https://www.w3.org/TR/2013/REC-sparql11-update-20130321/#deleteInsert :
Blank nodes that appear in an INSERT clause operate similarly to blank nodes in the template of a CONSTRUCT query, i.e., they are re-instantiated for any solution of the WHERE clause;
So, as far as I can tell (and my hunch), removing ?insertions would at least better align with SPARQL Update.
As I see it, this is a class 3 change:
makes conforming data, processors, or other conforming agents become non-conforming according to the new version
makes non-conforming data, processors, or other agents become conforming
So, it is not a conformance issue for conforming client implementations that were not using blank nodes in INSERT. They can continue to work as they currently do but now have the option to include blank nodes in their INSERT queries.
As for previously non-conforming servers, i.e., the ones that were processing INSERT queries with blank nodes, with this change, they become conforming.
As for previously conforming servers, i.e., the ones that were not processing INSERT queries with blank nodes, with this change, they would become non-conforming. They have two general options: either implement to allow blank nodes in INSERT (successful response) or continue to run with reduced capability but make sure to reject payloads including INSERT and blank nodes with 405 or 415.
It is useful to be able to insert statements with blank nodes. Approving the PR based on the above. If there is information that's being overlooked as to why this change shouldn't be made, it'd be good to review that as well.
If not already, perhaps the N3 Patch tests can take the following into account from SPARQL Update's INSERT DATA ( https://www.w3.org/TR/2013/REC-sparql11-update-20130321/#insertData ).
Blank nodes in QuadDatas are assumed to be disjoint from the blank nodes in the Graph Store, i.e., will be inserted with "fresh" blank nodes.
|
During the SolidOS call, @bourgeoa mentioned that this might be seen as a bug fix rather than a spec change. Considering that this repo has 180 open issues, it's likely there are still edge-case bugs that will only surface during migrations and broader usage. I propose we allow some flexibility to address these issues on a case-by-case basis as we move forward. |
There was a problem hiding this comment.
Removing this statement introduces ambiguity and incompatibility, as blank node scoping is not defined in N3, and existing implementations are known to differ. Additionally, it creates a multiplicity problem.
The blank node restriction was deliberately added to the document after earlier experimentation was found to trigger such problems in existing implementations. Instead, patches should use variables, with the spec-mandated restriction that those should also occur in the conditions block, such that the spec-mandated procedure for dealing with multiplicities can be used.
That is already the case in RDFlib.js CommunitySolidServer/CommunitySolidServer#1998 (comment) |
@RubenVerborgh The proposal here still forbids to use blank nodes in the
Note that the current Solid spec also states: The triples resulting from ?insertions are to be added to the RDF dataset, with each blank node from ?insertions resulting in a newly created blank node. This suggests that blank nodes are allowed in
I can understand that variables allow to clearly identify a blank node when deleting. But when inserting, the blank node does not exist yet in the patched document and thus it can't be identified in the |
|
@lecoqlibre There are still issues with blank node interpretation in N3 inside and outside of formulas. In any case, the current PR is missing a statement about uniqueness of any blank nodes that would occur in the insertions graph. And even if such a statement were to be added, the lack of consistent blank node semantics in N3 would prevent verification or enforcement thereof, so it would be void. There are various cans of worms here; it's not an easy fix. N3Patch was always a stopgap; we knew this from its inception. |
|
@RubenVerborgh Can you explain what is different when using non spec PATCH with SPARQL update which is used in NSS and CSS ? what are the limitations that allow NSS made the choice to map I think that at this point it is this minimal PATCH that we would like to see implemented in all SOLID servers. |
|
@RubenVerborgh I hoped there was an easy fix :) So in the current Solid protocol we have no way to insert blank nodes with a PATCH request. We are then forced to do a PUT request. If a PATCH can't handle blank nodes it seems to loose its interest as blank nodes are a core feature of RDF. I think I already saw Solid apps that are only using PUT request even to patch documents. This workaround does the job but it's not satisfying and also leads to performance issues (transferring the whole document instead of just what needs to be changed).
If this issue was known from the beginning, did someone already work on trying to resolve the problem?
Would you have a plan? Could you provide more details about "issues with blank node interpretation in N3 inside and outside of formulas"? Is "the lack of consistent blank node semantics in N3" the only problem? FYI @bourgeoa, Tim. Berners-Lee indicated that the main problem of SPARQL-update was the impossibility to make a lock (see this comment. He also added that N3 provides more features like to have more metadata about who made the change, etc. |
@lecoqlibre I finally found the discussion. From what I understand of @timbl comment is that with the SPARQL specification and using a SPARQL endpoint there is no lock if you follow the spec. It seems that the actual point raised by @RubenVerborgh is with N3.js and marginally with the spec. @timbl fully agreed with the need of bnodes.
|
No, it is with the (lack of) a Notation3 specification/standard implemented by all parties.
Existing Notation3 implementations have diverging interpretations of how to handle blank nodes in Notation3 documents, when those blank nodes occur in different scopes/formulas (as would be the case with this proposal). Even worse, any constraint the Solid spec would impose on such blank nodes cannot be implemented consistently because of those diverging interpretations. We'd essentially ask to operate on a subset of N3 that we cannot properly define without changing N3 and updating existing implementations. The original phrasing sidestepped that problem by sticking to a commonly agreed subset of Notation3 supported by all known parsers, at the cost of limiting functionality (superficially/cosmetically, in the sense that Workarounds are possible; they always are. A proper fix is not within this group's mandate, as it pertains to Notation3. |
Do you have a concrete example of what you mean by uniqueness. |
_:b1 a solid:InsertDeletePatch;
solid:where { _:b1 ex:familyName "Garcia". };
solid:inserts { _:b1 ex:givenName "Alex". }.The issue is that different parsers will disagree on whether the 3 syntactical occurrences of Time constraints might prevent me from further engaging in this thread, so I will halt my participation here. |
This is interesting. From the spec I understand that this should not be a valid n3-patch even after the proposed modification I suppose that you find this as subject to interpretation Even if they relate to 2 different documents. And there can be a provision to say that the |
|
@RubenVerborgh I had in mind that we should allow this one:
@prefix solid: <http://www.w3.org/ns/solid/terms#>.
@prefix ex: <http://www.example.org/terms#>.
_:insert a solid:InsertDeletePatch;
solid:inserts { ex:product ex:hasPrice [ ex:value "5.5"; ex:currency "euro". ] . }. |
|
@lecoqlibre We totally should—but we can't (easily). A different syntactical representation of the above Notation3 document is: _:p a solid:InsertDeletePatch;
solid:inserts { ex:product ex:hasPrice _:v. _:v ex:value "5.5"; ex:currency "euro". }.And, according to some (!) parsers, another one is: _:p a solid:InsertDeletePatch;
solid:inserts { ex:product ex:hasPrice _:p. _:p ex:value "5.5"; ex:currency "euro". }.And sure, we can start restricting the syntax of N3Patch documents. We can do those things. But none of them are a simple fix. Better to build a proper patch format to replace N3Patch. But again, not a simple fix. |
|
Dear all, I am very late in reading the discussion but I am pleased to read that you discuss to use N3 for patches here. I was co-chairing the N3 community group and we released a first draft for the semantics where especially the meaning of blank nodes was important (all documents can be found here: https://w3c.github.io/N3/spec/). I would also be open to give more feedback to the patches as they are once I better understand (I am a little bit late in that conversation :) ). |
Hi @doerthe I think the short answer is - yes, there would be interest in giving N3 WG status. I would endorse / support such a WG; and encourage my institution to vote in favour of a charter. I am interested is in seeing a stable WG specification so there is no ambiguity on how blank nodes are interpreted. As far as I understand, there is no need from the Solid side for much further evolution of the specs - just a need for stability. |
|
In our 5 March 2025 CG call we discussed that we'll merge this PR and then once that's done, extract N3-PATCH into a separate document. I think @jeswr said that in a few weeks from now (once he has a bit more time to dedicate to this) he will add a note to this PR about the planned extraction of N3-PATCH into a separate doc, and then merge it, right? |
|
FYI Some potential issues with the notes:
Not correct; I wrote the NSS implementation and created the spec text to reflect what I implemented. The rdflib–NSS connection relied on (a non-standard interpretation of) SPARQL UPDATE.
So the if condition fails there. Pivot (or any other implementation) can't implement “it”, in the sense that “it” is a client–server contract. Pivot could create a custom Notation3 implementation, but clients wouldn't necessarily follow it. That's the real problem, not the implementation. The issue is that the proposed spec text is not implementable; not that it's not implemented. Pivot could implement SPARQL UPDATE though.
Good idea, but just a reminder that N3Patch was always a stopgap. It was never designed for the long term. |
Co-authored-by: Michiel de Jong <[email protected]>
There was a problem hiding this comment.
(Supersedes my previous review and any of my comments.)
This review is on behalf of an implementation (the Community Solid Server) of this specification.
While I still disagree with the proposed change, I am removing any objections (in as far as they have any weight on the process).
CSS will, reluctantly, implement the updated specification, as it concerns a superset of existing behavior, and thus does not break clients that follow the existing specifications.
However, I would like to add the following comments to the record:
-
As the original implementer of N3Patch, and author of the descriptive text, I can assure you that the lack of the described functionality was a deliberate design decision and not an oversight. At the moment of design and implementation, we were aware of this limitation, and the fact that some cases would require the insertion of blank nodes. N3Patch was always intended as a short-term stopgap for a design problem back then, namely that NSS relied on SPARQL UPDATE semantics that contradicted to SPARQL UPDATE 1.1 specification.
-
N3Patch was a simplification that avoided a couple of complexities, which allowed us to keep the specification text short and simple. However, re-introducing those complexities, is unlikely to be possible while keeping the simple text.
-
The previous point is exemplified by the fact that certain behaviors are now undefined (and not necessarily described as such in the text).
-
Whether or not the change is limited to a small number of lines of code is, in this review, not part of the argument.
-
The main argument is that:
- This is not a bugfix in the sense that the original behavior was intentional. Arguments that rely on it being a previously unknown or unintentional bug, do not match the timeline.
- It not being a bugfix makes it a change, which, in our opinion, puts this work outside of the agreed-upon delineation of CG and WG.
- For reasons explained above, the change is more substantial than the amount of text indicates, and leads to undefined behavior.
- N3Patch shouldn't be the way forward in any case; it was a stopgap. A sustainable solution requires a more detailed requirements analysis, which was not performed back in the day. However, adding such a solution to the spec falls outside the CG scope (but it could be interesting for the CG to investigate and advise the WG.)
I still think better ways exist, notably supporting blank node IRIs, which the server could transform to blank nodes when needed.
Separately, we should also critically investigate existing libraries and tools for relying on behavior outside of the specification. The CSS has always been very literal in its interpretation, precisely to help surface such bugs. In that sense, it's a pity that this non-standard usage of the Solid Protocol only started to be discussed when the time pressure of solidcommunity.net was already there, as it forces us to find fast solutions rather than sustainable ones. The fact that this feature first appeared as a CSS pull request, rather than a discussion about the spec (which CSS implements), is also something we should strive to avoid.
|
Thank you for the background details, @RubenVerborgh. It might be worth putting this comment (or one like it, or links from there to it here) in appropriate places on the CSS repo and/or more places (e.g., CSS project wiki, or an issue for which a solution will never be implemented, etc.) on this repo — something a bit less volatile and other–problem–specific than a comment on a PR. History is important, and we have all been in places where delivering something functional "enough" on a specific date was deemed more important than delivering an indisputably better and more functional thing on an indeterminate future date. Knowing the basis of the decisions that were made then, especially that some other decision was only ruled out because of time–to–implement and/or that a chosen implementation was intended to be a stopgap, even though it was implemented 10 (or whatever) years ago now.... |
Co-authored-by: Sarven Capadisli <[email protected]>
Yes, indeed. Words like "producer", "emitter", "provider", "consumer", "sender", "receiver" (and so many more!) are often better even where there's a clear difference between "client" and "server". |
Co-authored-by: Ted Thibodeau Jr <[email protected]>
There was a problem hiding this comment.
As it stands, this PR has been reasonably reviewed, and feedback has been integrated.
Current state:
- Updating requirement
#server-patch-n3-blank-nodesis a correction class 3 - Adding requirement
#server-patch-n3-blank-nodes-syntaxis a correction class 4
I'm not aware of any objections, particularly from Solid CG participants) or implementers. That said, since we've covered quite a bit of (back)ground in this PR, I think it would be worthwhile to signal a "last call" for review where necessary, e.g., Solid CG weekly calls, chats, and the mailing list. It would also be helpful to get additional feedback from client/server implementers on whether they commit to implementing this change or other concrete technical concerns, i.e., new information that's not already covered in this PR.
Please leave a comment in this PR linking to minutes or elsewhere on the decision, and then I can follow up with updating #changelog and other information before merging the PR into ED.
Once again, if and when there is official transfer of this work/findings to another W3C CG or W3C Group, incubation in the Solid CG can continue and reflected in the ED.
I will request a snapshot of this PR's URL at https://web.archive.org/ (snapshot) and https://archive.is/ (snapshot) , FWIW.
As an implementer of patch document in dokieli (source code), the changes in this PR does not pose a change to the current implementation given that dokieli:
- does not use bnodes in the insertions formula;
- while its
patchResourceGraphcan potentially generate a patch document including the conditions formula, currently no feature uses the conditions formula. Hence, the changes related to bnode do not affect anything related to cases involving the conditions formula. - dokieli currently only uses bnode with a UUID based identifier for the patch resource subject (but will soon change to use a URI with a fragment instead as there is no need for a bnode).
(Related implementation experience was shared in #652 (comment) )
There was a problem hiding this comment.
Sorry for the confusion about whether this PR is paused or not, and also sorry for changing my stance on this PR twice. I initially said we should leave this to LWS WG, then Jesse convinced me, and then it sparked some deep conversation which made us take a step back and reflect on our community's dynamics.
On 24 March I had conversations with both @jeswr (as OP and also as ODI representative) and the CG co-chairs and we decided to take a break from this work. I had communicated this in the Solid OS chatroom where it came up, and I'm linking to that here now: https://matrix.to/#/!wAfwwonbRnRLYejFOR:gitter.im/$tNYNDeAilg5vmOhMwheiCgGoq-Gfn6aoMmMftzPHaC4?via=gitter.im&via=matrix.org&via=chat.semantic.works
So speaking from the CG side, this PR is paused now, until the 21 May 9 April CG call
|
@michielbdejong , can you please link to the CG decision - along with technical information - that justifies your blocking of this PR?
Citation needed. Please also share your reflections.
That's not CG consensus. As far as I can tell, Jesse has continued engaging with the PR to improve the text, so they are definitely not "taking a break". When you use "we", please recognise the difference between that "we" and the CG as a whole. That's why I'm asking for a CG decision that actually justifies things. And, no, the chairs doesn't just decide to take a break on things, and if anything, they try to ensure the process and consensus is accurately reflected and documented. As I've said in my last review, if the CG needs more time to review this PR (to gather new information), it can do so. That however does not justify your non-technical blocking. Once again, please refer to https://github.com/w3c-cg/solid/blob/main/CONTRIBUTING.md#processing-pull-requests on the expected processing time, and that's of course being generous and mindful at this point about the fact that the last change to the text was made today. So, that's "Within 10 days or 2 meetings" if anyone would like to request that. It can be of course be earlier if there is sufficient feedback gathered at the discretion of the group. So, perhaps reach out to implementers and get them put things on record in this PR. As the editor, I believe the current text adequately expresses what's intended. And, TimBL, as co-editor, has also expressed interest in merging. So, we're just being mindful about the review period at this point. |
|
Thanks for voicing your concerns about CG process Sarven, you definitely have a point. The reason for the pause was that the community we call "Solid" as a whole (not just the CG) is now failing to discuss a tiny topic like this in a peaceful way. Not all the conversations around it took place in CG calls, and not all of it was minuted, but we'll dedicate some time to it in today's call, and we'll also discuss the bigger picture next week. |
|
This issue was discussed in the todays CG call, key points disscussed:
|
There was a problem hiding this comment.
Thanks for your patience, I'm glad we got a chance to talk about what happened here. Let's move forward as discussed yesterday
|
Note: I made a process mistake. Only editors are supposed to merge this - I will not merge PRs in this repo in the future. I have checked with @csarven who has indicated that in this instance the PR can be left merged - and will make the changelog updates (which should have been made in this PR) in a separate commit. |
|
Thanks @jeswr . Updated with related in 9c0467e . Just to be clear, changelog and other related info didn't necessarily need to be part of this PR. That is not a major issue. In a nutshell, the editors ensure group's decision is best represented (which is why I called for a final decision to be noted). That said, since this PR introduced two substantive changes (changelog), it is generally best for someone else (i.e., the editors) to merge the PR besides the proposer/authors. It is only in some cases (such as the editorial changes and the TR/ document) that the editors can merge at their own discretion (even if they've authored the PR) but still with some expectations on what's reasonable to PR and when it'd be reasonable to merge: Solid Technical Reports Contributing Guide, Processing pull requests. |
In todays CG and SolidOS calls we discussed this change, to allow blank node insertions in N3 patches.
In both cases, all present members saw no reason why blank node insertions should be forbidden. @bourgeoa indicated that it was likely a mistake, rather than an intentional decision to forbid blank node insertions in the spec.
If you have any objections to this change, please raise them in this PR. If no objections are raised, this PR will be merged 10 days from now.
Context:
?insertions? #704cc @csarven @bourgeoa @CxRes @timbl @lecoqlibre