Expansion Algorithm, possible imprecision step 13.4.6.2

[timothee-haudebourg]

While expanding @included step 13.4.6.2 it is said

Set expanded value to the result of using this algorithm recursively passing active context, active property, value for element, base URL, and the frameExpansion and ordered flags, ensuring that the result is an array.

I believe the implementation is supposed to pass @included as active property instead of passing the current value. Otherwise, free floating literals inside the included block are automatically dropped, which makes the test expand/in07 to fail.

[gkellogg]

The syntax specification doesn't make it clear if the node containing the node with @included references the included nodes themselves, although in07 is consistent with their not being a relationship (which I think is the original intent).

Interestingly, my implementation does pass active property, but it doesn't end up creating such a relationship, but I can see how it might in another implementation.

Expansion could pass @included for active property, or null, which should have the same affect.

The Syntax document should be updated to clarify that there is no such relationship.

[pchampin]

To be fair, the syntax document states:

When flattened, this will move the employee and contractor elements from the included block into the outer array.

but granted, this is a bit technical, and should be explained more clearly and sooner in the section.

[iherman]

This issue was discussed in a meeting.

• RESOLVED: Clarify that in 13.4.6.2 for @included an implementation should pass null to avoid creating a reference, per api #479

View the transcript

Rob Sanderson: #479
Gregg Kellogg: It passes active property, while it shouldn’t. A node object contains an incl block, and has a rel to another block, the prop relationship should only hold to …. Passing null should be fine here.
Pierre-Antoine Champin: Does passing this active property have some impact on scoped contexts?
Gregg Kellogg: yes, by passing the active property, it is used in an unexpanded sense to find relationships in unscoped contexts, and also for informing the relationship. In a streaming impl, it could be used for creating that triple. I discovered that in my streaming impl. If you strictly follow the algorithm, it doesn’t create the issue, as can be seen by the implementations that pass the tests. It is however more correct to not pass an a
Ruben Taelman: ctive property.
Pierre-Antoine Champin: I was afraid that this would cause an unintended ripple effect.
Gregg Kellogg: Those things should be unrelated.
… I tried it in my implementation, putting null did not break anything.
… But it was required in my streaming implementation.
Proposed resolution: Clarify that in 13.4.6.2 for @included an implementation should pass null to avoid creating a reference, per api #179 (Rob Sanderson)
Gregg Kellogg: +1
Rob Sanderson: +1
Pierre-Antoine Champin: +1
Ivan Herman: +1
Ruben Taelman: +1
Benjamin Young: +1
Resolution #5: Clarify that in 13.4.6.2 for @included an implementation should pass null to avoid creating a reference, per api #479

[gkellogg]

@timothee-haudebourg This is fixed in #481. Please let us know if that satisfies your concern.

[timothee-haudebourg]

Yes, it is fine for me, thanks. Should I close the issue, or do I let you?

[gkellogg]

It will close automatically when the PR is merged, thanks!

[timothee-haudebourg]

Well I may have spoken too soon. Just to be sure, I tried to pass null as active property (I was passing @included until now) and failed to pass the tests expand/in07 and expand/in09. I'm not sure why yet, and I don't have time to look into it today, it will have to wait tomorrow...

[timothee-haudebourg]

Yes, so passing null as active property here is equivalent to passing on the current active property value, since there is no active property. It is not the same as passing @included. And according to step 4.1 (or step 19 for the expansion test in09) of the expansion algorithm

If active property is null or @graph, drop the free-floating scalar by returning null.

So the value is dropped before being detected step 13.4.6.3. No error is raised and the test fails.