stevedlawrence commented on code in PR #1137:
URL: https://github.com/apache/daffodil/pull/1137#discussion_r1465313107
##########
daffodil-core/src/main/scala/org/apache/daffodil/core/dpath/Expression.scala:
##########
@@ -2329,6 +2342,11 @@ abstract class FunctionCallBase(
protected def checkArgArrayOrOptional(): Unit = {
lazy val isArrayOrOptional = expressions.head match {
case pe: PathExpression => pe.isPathToOneWholeArrayOrOptional
+ case ie: IfExpression => {
+ val tp = ie.thenPart
+ val ep = ie.elsePart
+ tp.isPathToOneWholeArrayOrOptional ||
ep.isPathToOneWholeArrayOrOptional
Review Comment:
Agreed, I would expect an `&&` here too.
Another thing maybe worth considering is that this doesn't allow nested ifs,
since it only checks one level deep. So if the `then` or `else` branches
contain another if-statement, even if those resolve to a path to a whole
array/optional, it will fail.
Though, I'm not sure if that matters, and in fact, I'm wondering if we
should just require that `fn:count()` accepts exactly a path--no conditionals,
no other functions, etc. So we don't allow `fn:count(if(cond) then /path1 else
/path2)` and that fails with an SDE like "fn:count argument must be a path". If
you want conditional counts then you do it like this: `if (cond) then
fn:count(/path1) else fn:count(/path2)`. It's a bit more verbose, but is more
clear and avoids potential edge cases.
--
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]
