Skip to content

Expose op:sample-by function in Java Client API #1315

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

Closed
llinggit opened this issue Aug 18, 2021 · 5 comments
Closed

Expose op:sample-by function in Java Client API #1315

llinggit opened this issue Aug 18, 2021 · 5 comments

Comments

@llinggit
Copy link
Contributor

As task in bugtrack, https://bugtrack.marklogic.com/55089
A sample query as below

xquery version "1.0-ml";
import module namespace op="http://marklogic.com/optic"
at "/MarkLogic/optic.xqy";
op:from-view("opticUnitTest", "musician")
=> op:sample-by(map:map()=>map:with("limit", 2))
=> op:result()

So we can address your issue, please include the following:

Version of MarkLogic Java Client API

See Readme.txt

Version of MarkLogic Server

See admin gui on port 8001 or run xdmp:version() in Query Console - port 8000)

Java version

Run java -version

OS and version

For MAC, run sw_vers.
For Windows, run systeminfo | findstr /B /C:"OS Name" /C:"OS Version"
For Linux, run cat /etc/os-release and uname -r

Input: Some code to illustrate the problem, preferably in a state that can be independently reproduced on our end

Actual output: What did you observe? What errors did you see? Can you attach the logs? (Java logs, MarkLogic logs)

Expected output: What specifically did you expect to happen?

Alternatives: What else have you tried, actual/expected?

@ehennum
Copy link
Contributor

ehennum commented Sep 20, 2021

Besides writing a unit test using PlanBuilder.sampleByOptions(), please consider whether it makes sense to add

  • A PlanBuilderBase.AccessPlanBase.sampleBy() method overload that takes the limit as an int.
  • A PlanBuilderSubImpl.AccessPlanSubImpl,sampleBy() implementation that constructs the PlanSampleByOptionsImpl and calls the existing method

@llinggit
Copy link
Contributor Author

Thanks, Erik. Will do.

@llinggit llinggit added test and removed new labels Sep 20, 2021
@llinggit
Copy link
Contributor Author

llinggit commented Oct 5, 2021

PlanSampleByOptions options = p.sampleByOptions().withLimit(2);

The unit test throws an error, "Server Message: OPTIC-INVALARGS: fn.error(null, 'OPTIC-INVALARGS', 'limit must be a positive number: '+length); -- Invalid arguments: limit must be a positive number: 4"

I found that it errors out at line 2731 of optic-impl.sjs. The number we passed in is treated as a string, not a number. Investigating.

@llinggit
Copy link
Contributor Author

llinggit commented Oct 5, 2021

The limit in PlanSampleByOptions is indeed a xs:int, but later changed to String in ExportablePlan.

@llinggit
Copy link
Contributor Author

llinggit commented Oct 5, 2021

Proposed changes:

Index: marklogic-client-api/src/main/java/com/marklogic/client/impl/PlanBuilderSubImpl.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/marklogic-client-api/src/main/java/com/marklogic/client/impl/PlanBuilderSubImpl.java b/marklogic-client-api/src/main/java/com/marklogic/client/impl/PlanBuilderSubImpl.java
--- a/marklogic-client-api/src/main/java/com/marklogic/client/impl/PlanBuilderSubImpl.java	(revision b495d4f5a70e8e953f6ba03a82ba74387729aef4)
+++ b/marklogic-client-api/src/main/java/com/marklogic/client/impl/PlanBuilderSubImpl.java	(date 1633408801496)
@@ -560,13 +560,13 @@
 
     return mapdef;
   }
-  static Map<String,String> makeMap(PlanSampleByOptions options) {
+  static Map<String,XsIntVal> makeMap(PlanSampleByOptions options) {
     if (options == null) {
       return null;
     }
 
     XsIntVal limit = options.getLimit();
-    return (limit == null) ? null : makeMap("limit", limit.toString());
+    return (limit == null) ? null : makeMap("limit", limit);
   }
   static Map<String,String> makeMap(PlanSparqlOptions options) {
     if (options == null) {
@@ -599,7 +599,15 @@
     return map;
   }
 
-  static BaseTypeImpl.BaseMapImpl asArg(Map<String,String> arg) {
+  static Map<String, XsIntVal> makeMap(String key, XsIntVal value) {
+    Map<String, XsIntVal> map = new HashMap<String, XsIntVal>();
+    if (key != null) {
+      map.put(key, value);
+    }
+    return map;
+  }
+
+  static BaseTypeImpl.BaseMapImpl asArg(Map<String, ?> arg) {
     if (arg == null) {
       return null;
     }

llinggit pushed a commit to llinggit/java-client-api that referenced this issue Oct 5, 2021
…so modify makeMap to work with XsIntVal for fromSparql options.
llinggit pushed a commit that referenced this issue Oct 5, 2021
@llinggit llinggit added ship and removed test labels Oct 5, 2021
@llinggit llinggit closed this as completed Nov 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants