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

Step doesn't work if main function is written in xquery in DHF 5.5.0 [BUG] #5792

Open
KrystianPlackowski opened this issue Jun 16, 2021 · 7 comments
Labels

Comments

@KrystianPlackowski
Copy link

KrystianPlackowski commented Jun 16, 2021

The issue

If main plugin of a step is written in xquery in the default way, that was working in DHF 5.4.0 (main function that takes $content and $options arguments), the step will always trigger the following error in DHF 5.5.0.:

[1.0-ml] XDMP-TOOMANYARGS: (err:XPST0017) Too many args, expected 2 but got 3

Investigation

The problem is with this line: https://github.com/marklogic/marklogic-data-hub/blob/master/marklogic-data-hub/src/main/resources/ml-modules/root/data-hub/5/flow/flowRunner.sjs#L225

This line evaluates main function of a step, which by default (if you run hubCreateStepDefinition command) is declared like that:

function main(content, options) { ... }

so it should take 2 arguments. However the line 225 evaluates it with 3 arguments. This is not a visible problem in sjs, but if you declare main function of the step in xquery, then it will return an error "Too many args, expected 2 but got 3".

Here is an example of that behavior:

sjs:

'use strict';
function main(content, options) { return 5 };
let content = [1,2];
let options = [3,4];
let context = [5,6];
main(content, options, context)

(no error thrown)

xquery:

xquery version "1.0-ml";

declare function local:fun($content, $options) { 5 };

let $content := (1,2)
let $options := (3,4)
let $context := (5,6)
return local:fun($content, $options, $context)

(throws [1.0-ml] XDMP-TOOMANYARGS: (err:XPST0017) local:fun($content, $options, $context) -- Too many args, expected 2 but got 3)

Technical details

Please provide the following version information:

  • Operating System = win10
  • MarkLogic = 9.0-13.1
  • Data Hub = 5.5.0
@KrystianPlackowski KrystianPlackowski changed the title Function is evaluated with extra number of arguments in DHF 5.5.0 [BUG] Step doesn't work if main function is written in xquery in DHF 5.5.0 [BUG] Jun 16, 2021
@rjrudin
Copy link
Contributor

rjrudin commented Jun 16, 2021

Thanks @KrystianPlackowski - are you creating an XQuery custom step via hubCreateStepDefinition? Doing that results in the main function being in an SJS library, which then calls a main function in an XQuery library. That's the intended way in DHF 5 for XQuery to be used for a custom step - i.e. DHF 5 is not intended to call an XQuery library function directly.

@KrystianPlackowski
Copy link
Author

KrystianPlackowski commented Jun 16, 2021

When DHF 5.4.0 was in place I had run hubCreateStepDefinition, then manually rewrote automatically created sjs main function to xquery format (while keeping the logic). Our project is fully written in xquery, so that wouldn't make sense for us to have few modules written in sjs. If you say that it's not indented to have xquery module in this place, then that's pretty sad.

@rjrudin
Copy link
Contributor

rjrudin commented Jun 16, 2021

Got it, thanks - we know that the XQuery developer experience is not well-documented for DHF 5, and so what you're doing makes sense. I understand the frustration too - I've happily used XQuery since 2009, and while I certainly enjoy using SJS within ML, the XQuery developer experience should be a lot better in DHF 5.

We'll do a little analysis of what it means for DHF 5 to invoke an XQuery library directly. If that is something we can support and just need a tweak here, this may be a 5.5.x fix; otherwise, it may be 5.6.x.

In the immediate short term - as a workaround, can you add a 3rd argument to the function? The new stepExecutionContext that's being passed in as a 3rd argument is not yet something we want to expose as part of the public API, which is why it's not yet included in the SJS module that's created when generating a new step definition. But I am thinking you can declare a 3rd argument and just ignore it for now until a release can address this.

@KrystianPlackowski
Copy link
Author

KrystianPlackowski commented Jun 16, 2021

The new stepExecutionContext that's being passed in as a 3rd argument is not yet something we want to expose as part of the public API

I would like to notice that if someone manually adds a third argument to default main function created by hubCreateStepDefinition task, he will get access to that argument.

We'll do a little analysis of what it means for DHF 5 to invoke an XQuery library directly. If that is something we can support and just need a tweak here, this may be a 5.5.x fix; otherwise, it may be 5.6.x.

if you agree on supporting that, then I think it would be a good idea to add automatic creation of main plugin in xquery format if an appropriate flag is passed to hubCreateStepDefinition task.
If not then please completely forbid calling xquery main modules from stepDefinition files by returning some understable error. Because right now calling xquery seems to be an undefined behavior I think.

Sure I can add a 3rd argument. I didn't know if calling functions with extra arguments is a normal procedure while writing sjs code. It looked like a mistake, so I wanted to report it

@rjrudin
Copy link
Contributor

rjrudin commented Jun 16, 2021

I just reproduced this issue, but I'm also curious - once I added a 3rd argument, I then get the following error:

"{\"stack\":\"XDMP-NOTANODE: (err:XPTY0019) $content/value

My step definition is now pointing to the scaffolded XQuery module, which starts with this:

declare function custom:main(
  $content as item()?,
  $options as map:map, $something-else)
{
  let $doc := if (xdmp:node-kind($content/value) eq "text") then xdmp:unquote($content/value) else $content/value

This is not going through the scaffolded SJS module, which does the following in order to construct a "content node":

let modifiedContent = datahubxqymodule.main(new NodeBuilder().addNode(content).toNode(), options);

So I'm wondering - what are you doing in your XQuery module in order for it to be invoked correctly?

@KrystianPlackowski
Copy link
Author

KrystianPlackowski commented Jun 17, 2021

There are 2 kinds of jsons in ML. One is object node, second has a map structure. Seems like DHF passes a map kind json to main plugin referred by step definition, so instead of $content/value, you need to use map:get($content, "value").

@fgeorges
Copy link

fgeorges commented Jun 1, 2023

Hi @rjrudin

We'll do a little analysis of what it means for DHF 5 to invoke an XQuery library directly. If that is something
we can support and just need a tweak here, this may be a 5.5.x fix; otherwise, it may be 5.6.x.

Just wondering whether any progress has been done on this front?

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

No branches or pull requests

4 participants