Skip to content
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

Different error conditions for XPath expressions #796

Closed
xml-project opened this Issue Apr 6, 2019 · 11 comments

Comments

Projects
None yet
4 participants
@xml-project
Copy link
Contributor

xml-project commented Apr 6, 2019

Currently we have:

XD0008: It is a dynamic error if a sequence of items appears where an item to be used as the context item is expected.

and

XD0005: It is a dynamic error if more than one document appears on the connection for this input port.

To my reading this means, that

<p:variable name="a" select="54">
  <doc /> <doc />
</p:variable>

is not an error (XD0008) because no context item is expected, but

<p:if test="true()">
  <p:with-input><doc /> <doc /></p:with-input>
</p:if>

will raise XD0005 even though the context item is not referred.

I fail to see the reason for this difference and think we should remove it.

Did I miss anything?

@ndw

This comment has been minimized.

Copy link
Contributor

ndw commented Apr 6, 2019

Those seem like near enough the same error that we don't need two error codes.

I have a marginal preference for the error not being dependent on whether or not the context item is actually referenced,.

In other words, I think both of the examples you give above should raise the error.

If we say it's ok to have a sequence as long as the context item isn't referenced, I think that introduces a usability problem: I have a pipeline that's been working fine for ages, I make a one character change to some expression and now it fails; I think that's confusing. It also introduces an implementation challenge since it means the processor has to analyze the expression and determine whether or not the context item is referenced which, in the face of conditional expressions, could be quite difficult to determine.

@xml-project

This comment has been minimized.

Copy link
Contributor Author

xml-project commented Apr 6, 2019

👍

@xml-project

This comment has been minimized.

Copy link
Contributor Author

xml-project commented Apr 8, 2019

@gimsieke @eriksiegel
Any opinions and/or preferences on this?

@gimsieke

This comment has been minimized.

Copy link
Contributor

gimsieke commented Apr 8, 2019

I concur with what Norm said

@eriksiegel

This comment has been minimized.

Copy link
Contributor

eriksiegel commented Apr 8, 2019

I concur with what Norm said and Gerrit concurred

@xml-project xml-project self-assigned this Apr 8, 2019

@xml-project

This comment has been minimized.

Copy link
Contributor Author

xml-project commented Apr 8, 2019

Conclusion: Remove XD0008, use everywhere XD0005 instead.

@xml-project

This comment has been minimized.

Copy link
Contributor Author

xml-project commented Apr 8, 2019

Actually I do not like our solution. Consider this example from the test-suite (ab-declare-step-011.xml)

<p:declare-step version="3.0" xmlns:p="http://www.w3.org/ns/xproc"
	xmlns:x="http://dummy">
	<p:output port="result" />
	
	<p:declare-step type="Q{http://dummy}step" name="step">
		<p:input port="source" sequence="true" />
		<p:output port="result" sequence="true"/>
		<p:option name="counter" as="xs:integer" required="true" />            
		<p:choose>
			<p:when test="$counter = 0"> <!-- [1] -->
				<p:identity />
			</p:when>
			<p:otherwise>
				<x:step counter = "{$counter -1}" name="recursion">
                                        <!-- [2] -->
					<p:with-input>
						<p:inline><element /></p:inline>
						<p:pipe step="step" port="source" />
					</p:with-input>
				</x:step>
				
			</p:otherwise>
		</p:choose>
	</p:declare-step>
	
	<x:step counter="3">
		<p:with-input><p:empty/></p:with-input>
	</x:step>
	
	<p:wrap-sequence wrapper="result" />
</p:declare-step>

This pipeline raises errors in two positions [1] and [2] because in both cases a sequence appears as context item. So one has to add an explicit <p:empty /> in [1] and either add <p:sink /> before [2] or rewrite AVT to p:option.

I think this is a usability problem.

Still sure you want the change?

@xml-project xml-project removed their assignment Apr 8, 2019

@xml-project

This comment has been minimized.

Copy link
Contributor Author

xml-project commented Apr 9, 2019

May be we should try another approach: We could say, that XProc provides a context item for an XPath expression if and only if there is exactly one document provided. If there is no document or more than one document the context item is undefined.

In all cases where the XPath expression does not depend on the context item, this solution is fine and you get the result you would have expected. (Without adding extra p:sink or p:empty to your pipelines)

In the case where the XPath expression depends on the context item and you provide one XProc document every is fine too.

If the XPath expression depends on the context item and you provide none, you get an error as before. The only new thing is, that if XPath depends on the context item and you provide a sequence, you get an error from the underlying XPath implementation (err:XPDY0002) because the context item is absent.

To make this more transparent to pipeline authors we might say that an XProc processor is required to catch this error and produce an XDxxxx saying "This Xpath expression needs a context item, but you provided either none or a sequence".

Will that work out?

@xml-project

This comment has been minimized.

Copy link
Contributor Author

xml-project commented Apr 10, 2019

Any comments on this proposal?

@gimsieke

This comment has been minimized.

Copy link
Contributor

gimsieke commented Apr 10, 2019

I’m not against it. I might not spot every hidden trap though.

@ndw

This comment has been minimized.

Copy link
Contributor

ndw commented Apr 18, 2019

Consensus: remove the errors for a sequence; the context item is simply undefined if a sequence appears.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.