Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

helper:assert-throws-error, xdmp:apply, varargs #503

Closed
kefo opened this issue Aug 13, 2015 · 5 comments
Closed

helper:assert-throws-error, xdmp:apply, varargs #503

kefo opened this issue Aug 13, 2015 · 5 comments

Comments

@kefo
Copy link

kefo commented Aug 13, 2015

I have a function I want to test.  It takes 6 parameters.  If I call helper:assert-throws-error, I get a “Too few args” error message.  The error is actually occurring at line 295.

I think this might be a bug because xdmp:apply seems to require the $params be itemized.  Instead of this

    let $params := (‘1’, ‘2’, ‘3’)
    return xdmp:apply( $func, $params )

You seem to have to do this:

    let $params := (‘1’, ‘2’, ‘3’)
    return xdmp:apply($func, $params[1], $params[2], $params[3])

(Side note: Some googling turned up this 2011 post discussing the ‘varargs’ aspect of xdmp:apply.)

I’ve actually implemented a fix locally and it seems to work. Of course, it has all ugliness mentioned in the above post. Happy to submit a pull request if, in fact, this is an issue.

If it isn’t an issue, please advise how one uses helper:assert-throws-error with multiple vars.

@grtjn
Copy link
Contributor

grtjn commented Aug 17, 2015

I think you found the best solution for the moment, though I wonder if you need to distinguish between all numbers of args like in the 2011 post. That link also proposes to use map:map to pass through arguments, which solves passing through sequences as argument. Or perhaps json:array would be practical here. Definitely a bug..

@grtjn grtjn modified the milestone: 1.7.3 Sep 17, 2015
grtjn added a commit to grtjn/roxy that referenced this issue Sep 21, 2015
@grtjn grtjn self-assigned this Sep 22, 2015
dmcassel added a commit that referenced this issue Oct 30, 2015
Fixed #503: leveraged json:array to fix helper function
jmeekhof pushed a commit to jmeekhof/roxy that referenced this issue Nov 14, 2015
grtjn added a commit to grtjn/roxy that referenced this issue Jan 28, 2016
dmcassel added a commit to dmcassel/roxy that referenced this issue Mar 4, 2016
Looks like a previous PR missed a return after adding a let. Maybe that
was just a rebasing problem or something, but it led to invalid syntax.
paxtonhare added a commit that referenced this issue Mar 4, 2016
@grtjn
Copy link
Contributor

grtjn commented Mar 11, 2016

Fixed in dev..

@grtjn grtjn closed this as completed Mar 11, 2016
@ghost
Copy link

ghost commented Jun 1, 2016

This is not actually working from what I can tell. There are 2 problems that i see, one is that the way the cascade of function calls are built, it will not work with <6 parameters. What ends up happening is that all sample functions get called with 6 parameters, with the balance being the empty sequence.

Once that was corrected, I noticed a different problem/limitation in json:to-array. There is some sequence collapse processing happening somewhere in the call stack that shouldn't be. This problem can be seen with this: json:to-array(json:to-array(1)) which should yield [ [ 1 ] ] but actually yields [ 1 ]. Luckily there is a way to workaround this by doing json:to-array((json:to-array(1), json:to-array('xtra')), 1)

See attached patch for a fix:
refactored-json-array-patch.txt

I can make a pull-request if that is helpful, but didn't seem necessary for such a small diff.

@ghost
Copy link

ghost commented Jun 1, 2016

Hmm, not sure my patch is correct... I think sending an empty sequence to an XQuery function as an argument in this case the function will receive an empty JSON array instead... I need to double check that when I get a chance.

UPDATE: Nevermind, I think it is OK after all. The json:array-values() does seem to work correctly to convert an empty json array into an empty sequence. The only trouble would come in if you tried to test a function with arrity > 6 and also sent some arguments as empty-sequence and it hit the fallback statement.

@grtjn
Copy link
Contributor

grtjn commented Jun 3, 2016

:)

Feel free to reopen again, or file a new ticket if you think those edge cases can be a real issue..

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants