mbeckerle commented on code in PR #1137:
URL: https://github.com/apache/daffodil/pull/1137#discussion_r1462278718
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dpath/Expression.scala:
##########
@@ -744,21 +744,41 @@ abstract class PathExpression() extends Expression {
* about an array such as its current count.
*/
lazy val isPathToOneWholeArray: Boolean = {
- if (steps == Nil) false // root is never an array
+ isNotRootOrSelf &&
+ steps.last.isArray &&
+ priorStepsHaveIndexingPredicates
+ }
+
+ /**
+ * Path to a single array element without a [N] predicate qualifying it
+ * or to an optional element.
+ *
+ * That is, the kind of path expression used to access information
+ * about an array or optional such as its current count.
+ */
+ lazy val isPathToOneWholeArrayOrOptional: Boolean = {
+ isNotRootOrSelf &&
+ (steps.last.isArray || steps.last.isOptional) &&
+ priorStepsHaveIndexingPredicates
+ }
+
+ private lazy val isNotRootOrSelf: Boolean = {
if (steps == Nil) false // root is never an array
else if (isSelf) false // self path is never an array
- else
- steps.last.isArray &&
- !steps.last.pred.isDefined && // last cannot have a [N] pred
- steps.dropRight(1).forall {
- // for all the prior steps
- // if they mention an array element, there
- // must be a [N] predicate.
- step =>
- if (step.isInstanceOf[Up]) true
- else if (step.isArray) step.pred.isDefined
- else true
- }
+ else true
+ }
+
+ private lazy val priorStepsHaveIndexingPredicates: Boolean = {
+ steps.last.pred.isEmpty && // last cannot have a [N] pred
+ steps.dropRight(1).forall {
Review Comment:
-1 on this change to use "init" terminology.
I have never heard of "init" function before.
In English, 'initial' means first, not 'all but last'. So I claim 'init' is
an extremely really poor choice of name for this concept and not a mistake I
would choose to repeat in our terminology.
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dpath/Expression.scala:
##########
@@ -744,21 +744,41 @@ abstract class PathExpression() extends Expression {
* about an array such as its current count.
*/
lazy val isPathToOneWholeArray: Boolean = {
- if (steps == Nil) false // root is never an array
+ isNotRootOrSelf &&
+ steps.last.isArray &&
+ priorStepsHaveIndexingPredicates
+ }
+
+ /**
+ * Path to a single array element without a [N] predicate qualifying it
+ * or to an optional element.
+ *
+ * That is, the kind of path expression used to access information
+ * about an array or optional such as its current count.
+ */
+ lazy val isPathToOneWholeArrayOrOptional: Boolean = {
+ isNotRootOrSelf &&
+ (steps.last.isArray || steps.last.isOptional) &&
+ priorStepsHaveIndexingPredicates
+ }
+
+ private lazy val isNotRootOrSelf: Boolean = {
if (steps == Nil) false // root is never an array
else if (isSelf) false // self path is never an array
- else
- steps.last.isArray &&
- !steps.last.pred.isDefined && // last cannot have a [N] pred
- steps.dropRight(1).forall {
- // for all the prior steps
- // if they mention an array element, there
- // must be a [N] predicate.
- step =>
- if (step.isInstanceOf[Up]) true
- else if (step.isArray) step.pred.isDefined
- else true
- }
+ else true
+ }
+
+ private lazy val priorStepsHaveIndexingPredicates: Boolean = {
+ steps.last.pred.isEmpty && // last cannot have a [N] pred
Review Comment:
Well, we don't implement "query style" expressions,....
Technically a path like foo/barArray/baz/quuxArray/value returns every value
for every index of quuxArray for every index of barArray. I.e., it is like an
implied array. That's what it means if treated like an XPath anyway.
But the point of the DFDL expression language was *not* to enable querying
nor to be a clone of XPath, but to express formats, and I know of no need for
these sorts of queries in expressing data formats, so that's why we left it
out. There is some confusion of whether the DFDL spec even allows this at all.
I can't think of a context where it makes sense in DFDL.
I'm ok with requiring [N] for fixed N for every array path step (except the
last), for any path at all. Keeping in mind that ".." never moves you up to a
whole array, but to an instance within an array. Hence "..[N]" is never needed
(nor allowed).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
