-
Notifications
You must be signed in to change notification settings - Fork 22
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
allow use of id-maps in :values pattern #809
Conversation
I love this feature and struggled with this myself, super glad you tackled it. The one thing I think we should support slightly differently than you put it in is that the values variable should work like a pure substiution. e.g. this make tons of sense:
as you can think of it resolving to this:
But this doesn't make sense to me:
As you'd imagine it would resolve to this, which isn't how you'd query:
Instead, I think if you were trying to do that same query you'd want to define it like this and sub in the variable:
Likewise in the original example above, if you used @id for ex:friend you'd think this would logically work:
Is this possible without making it substantially more complex a problem? |
Unfortunately it wouldn't be possible to achieve this without the addition of some complex analysis of where the variable is used. And I think it may even be inconsistent, because there is nothing stopping you from using the same However, I think you can frame it another way where the semantic isn't "literal substitution" and more "expansion, then substitution". If we imagine the query/txn as a json-ld document (it's not, but we try to pretend it is as far as we can), then we can think of the id-maps as annotation for the
Which expands to:
Now, this isn't how we actually do expansion, but users don't need to know that. They can correctly mentally model the values of the |
Ok, either way this is helpful over what we had. We can think about addressing the next step if it becomes an issue for our users. I want to confirm one thing, even though these are different queries, I assume they will both behave as the same query?
|
Yes, and I've added a test to ensure that semantic persists. |
FQL is not JSON-LD. There are a lot of things that we do in FQL that don't agree with the JSON-LD spec. The whole notion of binding variables to particular values is foreign to JSON-LD, so this scenario would never arise and I'm not surprised that JSON-LD doesn't address it.
Do you have any examples of this? Every usage of iris in FQL that I can think of use id maps to represent the node. For example, in a where clause
represents the node with "@id" "ex:foo", just like
represents a node with the value of the "ex:bar" property being "ex:baz". Also
represent a node with the value of the "ex:bestFriend" property being the node with "@id" "ex:charlie". Binding an id map to a variable and then using that variable as the value of an id later is inconsistent in my mind. You are saying that a node's id is the node itself, not the iri that represents that node. |
In practice I think our users will mainly be depending on the heuristic that identifiers that need expansion need to be wrapped in an id-map. I know I tried to use id-maps at first and when they didn't work I was confused. And FWIW, we do pretend in our official documentation that FQL is JSON-LD. And that's not strictly wrong, we just utilized I still think this is useful, do you think we shouldn't allow id-maps in |
The semantic is not "wrap this in an id map if you want to expand it"; the semantic is "an id map represents a node, everything else is scalar data". We automatically expand any value of the "@id" attribute because we know that must unambiguously be an iri, just like we also expand "ex:foo" in FQL is a pattern matching system and we substitute the value bound to a variable directly in the places that variable appears. This patch breaks that, but only sometimes, in certain specific situations. That inconsistency will lead to much more confusion. I don't think we should allow id maps in values because of all of that inconsistency. @bplatz listed some of these inconsistencies involving weirdly recursive "@id" values:
I agree that this doesn't make sense. This is my point.
This is the most consistent way to express this query given the rest of the syntax of FQL, and it's is what we currently do without this patch. The only missing piece is that if we rely on automatic inference, the parser will think that "ex:brian" is a string, so we now need some way to tell the parser that "ex:brian" is supposed to be an iri and not a string. In every other scenario where automatic inference fails and the user has to specify how to interpret scalar data, we use an "@value" map to provide that extra information, so the most consistent thing to do here is to use an "@value" map as well. "@id" maps represent nodes; "@value" maps represent scalar data along with extra information for how to interpret that data. |
I also want to make clear that I'm not wedded to using value maps for this per se if folks find that construct confusing in this situation. I think value maps are fine, but I could get on board with a new syntax (depending on what the syntax is, of course). For example, maybe we could try to do something with the I just think it would be a bad idea to reuse an existing syntax that already means something else. |
I understand your point but don't fully agree that there is a distinction between an IRI and a 'node'. I think an IRI is always a node. It may not have properties assigned to it in the local db (yet), but you must assume it has properties in different db somewhere. The fact that we don't require specifying the IRI data type in a few circumstances is probably the main culprit that makes this confusing. Those circumstances are: The JSON-LD spec allows you to specify the datatype of an IRI like this:
I think this should be the base case of what we support. Here @id is a shortcut for xsd:anyURI (I presume, although I'm not sure that is ever made explicit in the spec). This is similar to how @type is a shortcut for rdf:type. I consider this a further shortcut for the above, which I am supportive of using but there is arguably some debate about that:
Then the question becomes if we can do anything to handle the 3 circumstances above such that the behavior of using I understand the complexity of this, so I'd recommend punting that decision as we have some larger issues that don't have work arounds currently which deserve our focus. I do think we should at minimum support the |
This has the potential to get real philosophical real fast, but I just want to assert that there is a concrete distinction between IRI and subject node. We need to keep that distinction straight in order to build a consistent system. It's the same as the distinction between "Benjamin Lamothe" and myself. I am not the sequence of characters "Benjamin Lamothe", but people use that character sequence as a symbol to refer to me in certain contexts. The JSON-LD Spec describes a node identifier as an IRI used to refer to a subject node. It isn't the node itself. The node itself is an entity that has a set of characteristics, or properties. In order to talk about that specific entity in the context of an RDF graph, and to differentiate it from other entities in the graph, we give it a specific name. That name is an IRI. We indicate that a specific IRI is the identifier of a subject by using the "@id" key of the map we use to describe that subject. I have a height, weight, age, and favorite food, but my name does not. My name is a thing that people use to refer to me. The goal of this exercise is to allow users to include subject node identifiers in Everywhere in FQL, we use For example, when a subject node is an object object of an RDF triple, we don't use the raw iri string, we use a map as in This proposal here is to change that semantic, and instead use a subject node map to serve as a node identifier, but only in a certain specific situation. That would already introduce an internal inconsistency, but it would also have weird consequences that would require that we introduce cascading inconsistencies to resolve. You mentioned this about the JSON-LD spec:
This is not the case. Note the paragraph describing the value of the "@type" key within an "@value" object:
The spec excludes the "@id" keyword from that list. The spec does allow you to define You went on to say
This is what we support today, without this patch. The only caveat is that it uses
As I've mentioned, introducing this as a shortcut for I thought about this a lot when I developed this syntax for specifying IRIs in values. The first option I considered was using What we have now using an "@value" map with "xsd:anyURI" as the data type is the most clear and consistent alternative that I could think of but, like I said, I'm not wedded to it and am open to the possibility that there is a better syntax I haven't considered. I just don't think introducing a new, contradicting meaning to a syntax we already use is the right call. |
Do you have an example in mind of this? I was able to get this working with ~2 code lines changed and I don't think it affects anything downstream - we'd just have to document the syntax. We've long since departed from the path of True JSON-LD Semantics, so I don't believe that anybody is looking at the spec and complaining about inconsistencies between value node and node identifier representations in our syntax. It's also not clear to me what practical consequences would result from this inconsistency. The downsides of using the value-map syntax is you need to have the I do feel that id-maps are a lot easier to explain in this vein:
I don't see any case in our syntax where this heuristic would cause a problem, plus it's easier to type and easier to remember |
8df445c
to
1bf3abd
Compare
The json-ld standard does not actually support iri expansion in a value map. Also, iris are denoted with id maps in every other bit of FQL syntax. This commit allows both json-ld-compliant iri declaration and makes our syntax more consistent.
Also, fixed a problem in the test data where id refs were being inserted as strings.
This section is meant for testing values clauses.
1bf3abd
to
07bc37d
Compare
Rebased on main. Per the discussion in #818, we've decided to use |
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.
🧥
(if-let [dt-iri (get-expanded-datatype attrs context)] | ||
(if (= const/iri-anyURI dt-iri) | ||
(if (or (= const/iri-anyURI dt-iri) |
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.
We should eventually remove this, but I'm ok with keeping it in for now in case someone is using it.
Awesome thanks @dpetran |
The json-ld standard does not actually support iri expansion in a value map. Also, iris are denoted with id maps in every other bit of FQL syntax. This commit allows both json-ld-compliant iri declaration and makes our syntax more consistent.
Here's an example of our current value-map syntax not expanding iris
I believe we should deprecate the
{"@value" <iri> "@type" "xsd:anyURI"}
syntax for iri values and not document this usage for public use.