Merge lp:~davidagraf/zorba/paging into lp:zorba

[David Graf]

Positional pagination

[Matthias Brantner (matthias-brantner)]

- code conventions in collections_impl.cpp
- getIterator(xs_integer aSkip)
  - should be getIterator(xs_integer& aSkip) because xs_integer can be MAPM
  - could be one function with default parameter 0
- to_xs_long conversion in SimpleCollection can raise an error which should be caught somewhere in the iterator and a location needs to be added
- CollectionIter(SimpleCollection* collection, xs_integer aSkip); => CollectionIter(SimpleCollection* collection, xs_integer& aSkip);
- store::Iterator_t getIterator(xs_integer aSkip); => store::Iterator_t getIterator(xs_integer& aSkip);
- functions should be in the .xq modules and have documentation
- dml:collection(xs:QName("local:test2"), -1) produces a segfault
- dml:collection(xs:QName("local:test2"), x) produces a segfault if x is bigger than the size of the collection
- skip and count optimizations don't seem to play well together (maybe not needed)

review: Needs Fixing

[Till Westmann (tillw)]

review: Approve

[Matthias Brantner (matthias-brantner)]

The documentation of the two functions is a little weak. For example, it doesn't say what's happening to the skip parameter if it's negative or bigger than the size of the collection. Also, it would make sense to say that the function also makes sense for unordered collections because nodes are returned in a stable order.

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Attempt to merge into lp:zorba failed due to conflicts:

text conflict in ChangeLog

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for merge proposal.
Log at: http://zorbatest.lambda.nu:8080/remotequeue/paging-2012-06-29T13-48-12.365Z/log.html

[Zorba Build Bot (zorba-buildbot)]

Validation queue job paging-2012-06-29T13-48-12.365Z is finished. The final status was:

All tests succeeded!

=== modified file 'ChangeLog'
--- ChangeLog	2012-06-28 22:07:25 +0000
+++ ChangeLog	2012-06-29 13:52:27 +0000
@@ -10,6 +10,7 @@
     - fn:parse-xml-fragment#1
   * Added support for transient maps to the http://www.zorba-xquery.com/modules/store/data-structures/unordered-map module.
   * Added support for fragments to fn:path
+  * Positional pagination support for collections
 
 Optimizations:
   * Small optimization of comparison operations.

=== modified file 'modules/com/zorba-xquery/www/modules/store/dynamic/collections/dml.xq'
--- modules/com/zorba-xquery/www/modules/store/dynamic/collections/dml.xq	2012-06-28 04:14:03 +0000
+++ modules/com/zorba-xquery/www/modules/store/dynamic/collections/dml.xq	2012-06-29 13:52:27 +0000
@@ -615,6 +615,23 @@
 
 
 (:~
+ : The collection function returns the sequence of nodes and/or json items
+ : that belong to the collection identified by the given name.
+ :
+ : @param $name The name of the collection.
+ : @param $skip The number of collection items to skip.
+ :
+ : @return The sequence contained in the given collection.
+ :
+ : @error zerr:ZDDY0003 If available collections does not provide a mapping
+ :        for the expanded QName $name.
+ :
+ :)
+declare function
+dml:collection($name as xs:QName, $skip as xs:integer) as item()* external;
+
+
+(:~
  : The collection-name function returns the name of the collection the given
  : item (node or json item) belongs to.
  :

=== modified file 'modules/com/zorba-xquery/www/modules/store/static/collections/dml.xq'
--- modules/com/zorba-xquery/www/modules/store/static/collections/dml.xq	2012-06-28 04:14:03 +0000
+++ modules/com/zorba-xquery/www/modules/store/static/collections/dml.xq	2012-06-29 13:52:27 +0000
@@ -867,6 +867,23 @@
 
 
 (:~
+ : The collection function returns the sequence of nodes and/or json items
+ : that belong to the collection identified by the given name.
+ :
+ : @param $name The name of the collection.
+ : @param $skip The number of collection items to skip.
+ :
+ : @return The sequence contained in the given collection.
+ :
+ : @error zerr:ZDDY0001 if the collection identified by $name is not declared.
+ : @error zerr:ZDDY0003 if the collection identified by $name is not available.
+ :
+ :)
+declare function 
+cdml:collection($name as xs:QName, $skip as xs:integer) as item()*  external;
+
+
+(:~
  : The collection-name function returns the name of the collection the given
  : item (node or json item) belongs to.
  :

=== modified file 'src/functions/func_sequences_impl.cpp'
--- src/functions/func_sequences_impl.cpp	2012-06-28 04:14:03 +0000
+++ src/functions/func_sequences_impl.cpp	2012-06-29 13:52:27 +0000
@@ -543,20 +543,16 @@
     ZorbaCollectionIterator& collection =
     static_cast<ZorbaCollectionIterator&>(*argv[0]);
 
-    if (collection.isDynamic())
-    {
-      return new CountCollectionIterator(sctx,
-                                         loc,
-                                         collection.getChildren(),
-                                         CountCollectionIterator::ZORBADYNAMIC);
-    }
-    else
-    {
-      return new CountCollectionIterator(sctx,
-                                         loc,
-                                         collection.getChildren(),
-                                         CountCollectionIterator::ZORBASTATIC);
-    }
+    return new CountCollectionIterator(
+                 sctx,
+                 loc,
+                 collection.getChildren(),
+                 (
+                   collection.isDynamic()
+                     ? CountCollectionIterator::ZORBADYNAMIC
+                     : CountCollectionIterator::ZORBASTATIC
+                 )
+               );
   }
   else if (typeid(FnCollectionIterator) == counted_type)
   {
@@ -600,10 +596,9 @@
     return new ProbeIndexRangeGeneralIterator(
         sctx, loc, lIter.getChildren(), true);
   }
-  else
-  {
-    return new FnCountIterator(sctx, loc, argv);
-  }
+  
+  // fallback
+  return new FnCountIterator(sctx, loc, argv);
 }
 
 

=== modified file 'src/functions/pregenerated/func_collections.cpp'
--- src/functions/pregenerated/func_collections.cpp	2012-06-28 04:14:03 +0000
+++ src/functions/pregenerated/func_collections.cpp	2012-06-29 13:52:27 +0000
@@ -342,6 +342,19 @@
 
       {
     DECL_WITH_KIND(sctx, static_collections_dml_collection,
+        (createQName("http://www.zorba-xquery.com/modules/store/static/collections/dml","","collection"), 
+        GENV_TYPESYSTEM.QNAME_TYPE_ONE, 
+        GENV_TYPESYSTEM.INTEGER_TYPE_ONE, 
+        GENV_TYPESYSTEM.ANY_NODE_TYPE_STAR),
+        FunctionConsts::STATIC_COLLECTIONS_DML_COLLECTION_2);
+
+  }
+
+
+
+
+      {
+    DECL_WITH_KIND(sctx, static_collections_dml_collection,
         (createQName("http://www.zorba-xquery.com/modules/store/dynamic/collections/dml","","collection"), 
         GENV_TYPESYSTEM.QNAME_TYPE_ONE, 
         GENV_TYPESYSTEM.ANY_NODE_TYPE_STAR),
@@ -353,6 +366,19 @@
 
 
       {
+    DECL_WITH_KIND(sctx, static_collections_dml_collection,
+        (createQName("http://www.zorba-xquery.com/modules/store/dynamic/collections/dml","","collection"), 
+        GENV_TYPESYSTEM.QNAME_TYPE_ONE, 
+        GENV_TYPESYSTEM.INTEGER_TYPE_ONE, 
+        GENV_TYPESYSTEM.ANY_NODE_TYPE_STAR),
+        FunctionConsts::DYNAMIC_COLLECTIONS_DML_COLLECTION_2);
+
+  }
+
+
+
+
+      {
     DECL_WITH_KIND(sctx, static_collections_dml_collection_name,
         (createQName("http://www.zorba-xquery.com/modules/store/static/collections/dml","","collection-name"), 
         GENV_TYPESYSTEM.ANY_NODE_TYPE_ONE, 

=== modified file 'src/functions/pregenerated/function_enum.h'
--- src/functions/pregenerated/function_enum.h	2012-06-28 21:54:08 +0000
+++ src/functions/pregenerated/function_enum.h	2012-06-29 13:52:27 +0000
@@ -54,7 +54,9 @@
   FN_COLLECTION_0,
   FN_COLLECTION_1,
   STATIC_COLLECTIONS_DML_COLLECTION_1,
+  STATIC_COLLECTIONS_DML_COLLECTION_2,
   DYNAMIC_COLLECTIONS_DML_COLLECTION_1,
+  DYNAMIC_COLLECTIONS_DML_COLLECTION_2,
   STATIC_COLLECTIONS_DML_COLLECTION_NAME_1,
   DYNAMIC_COLLECTIONS_DML_COLLECTION_NAME_1,
   STATIC_COLLECTIONS_DML_INDEX_OF_1,

=== modified file 'src/runtime/collections/collections_impl.cpp'
--- src/runtime/collections/collections_impl.cpp	2012-06-28 04:14:03 +0000
+++ src/runtime/collections/collections_impl.cpp	2012-06-29 13:52:27 +0000
@@ -230,6 +230,7 @@
 {
   store::Collection_t coll;
   store::Item_t name;
+  xs_integer lCount;
 
   PlanIteratorState* state;
   DEFAULT_STACK_INIT(PlanIteratorState, state, planState);
@@ -245,7 +246,20 @@
     coll = getW3CCollection(planState);
   }
 
-  STACK_PUSH(GENV_ITEMFACTORY->createInteger(result, coll->size()), state);
+  lCount = coll->size();
+  if (theChildren.size() > 1) 
+  {
+    // skip parameter passed
+    store::Item_t lSkipItem;
+    consumeNext(lSkipItem, theChildren[1].getp(), planState);
+    xs_integer lSkip = lSkipItem->getIntegerValue(); 
+    // negative is transformed into 0
+    lCount -= ( lSkip <= xs_integer::zero() ? xs_integer::zero() : lSkip );
+    // negative is transformed into 0
+    lCount = ( lCount < xs_integer::zero() ? xs_integer::zero() : lCount );
+  }
+
+  STACK_PUSH(GENV_ITEMFACTORY->createInteger(result, lCount), state);
 
   STACK_END(state);
 }
@@ -360,6 +374,7 @@
 {
   store::Item_t name;
   store::Collection_t collection;
+  xs_integer lSkip;
 
   ZorbaCollectionIteratorState* state;
   DEFAULT_STACK_INIT(ZorbaCollectionIteratorState, state, planState);
@@ -368,10 +383,39 @@
 
   (void)getCollection(theSctx, name, loc, theIsDynamic, collection);
 
-  // return the nodes of the collection
-  state->theIterator = collection->getIterator();
+  if (theChildren.size() > 1)
+  {
+    // skip parameter passed
+    store::Item_t lSkipItem;
+    consumeNext(lSkipItem, theChildren[1].getp(), planState);
+    lSkip = lSkipItem->getIntegerValue(); 
+    // negative is transformed into 0
+    state->theIterator = ( lSkip > xs_integer::zero() 
+                             ? collection->getIterator(lSkip)
+                             : collection->getIterator()
+                         );
+  }
+  else
+  {
+    state->theIterator = collection->getIterator();
+  }
+
   ZORBA_ASSERT(state->theIterator != NULL);
-  state->theIterator->open();
+
+  try
+  {
+    state->theIterator->open();
+  }
+  catch ( std::range_error const& )
+  {
+    throw XQUERY_EXCEPTION(
+      zerr::ZXQD0004_INVALID_PARAMETER,
+      ERROR_PARAMS(ZED(ZXQD0004_NOT_WITHIN_RANGE),
+                  lSkip),
+      ERROR_LOC( loc )
+    );
+  }
+
   state->theIteratorOpened = true;
 
   while (state->theIterator->next(result))
@@ -384,7 +428,6 @@
   STACK_END(state);
 }
 
-
 /*******************************************************************************
   declare function index-of($name as xs:QName,
                             $target as node()) as xs:integer

=== modified file 'src/runtime/spec/collections/collections.xml'
--- src/runtime/spec/collections/collections.xml	2012-06-28 04:14:03 +0000
+++ src/runtime/spec/collections/collections.xml	2012-06-29 13:52:27 +0000
@@ -249,8 +249,20 @@
         <zorba:output>structured-item()*</zorba:output>
       </zorba:signature>
 
-      <zorba:signature localname="collection" prefix="dynamic-collections-dml">
-        <zorba:param>xs:QName</zorba:param>
+      <zorba:signature localname="collection" prefix="static-collections-dml">
+        <zorba:param>xs:QName</zorba:param>
+        <zorba:param>xs:integer</zorba:param><!-- nodes to skip -->
+        <zorba:output>structured-item()*</zorba:output>
+      </zorba:signature>
+
+      <zorba:signature localname="collection" prefix="dynamic-collections-dml">
+        <zorba:param>xs:QName</zorba:param>
+        <zorba:output>structured-item()*</zorba:output>
+      </zorba:signature>
+
+      <zorba:signature localname="collection" prefix="dynamic-collections-dml">
+        <zorba:param>xs:QName</zorba:param>
+        <zorba:param>xs:integer</zorba:param><!-- nodes to skip -->
         <zorba:output>structured-item()*</zorba:output>
       </zorba:signature>
 

=== modified file 'src/store/api/collection.h'
--- src/store/api/collection.h	2012-06-28 04:14:03 +0000
+++ src/store/api/collection.h	2012-06-29 13:52:27 +0000
@@ -53,8 +53,12 @@
    *
    * It is allowed to have several concurrent iterators on the same Collection,
    * but each iterator should be used by a single thread only.
+   *
+   * @param  aSkip The number of collection entries to skip.
+   * @return Iterator
    */
-  virtual Iterator_t getIterator() = 0;
+  virtual Iterator_t getIterator(
+      const xs_integer& aSkip = xs_integer::zero()) = 0;
 
   /**
    * Get the node at the given position in the collection.

=== modified file 'src/store/naive/collection.h'
--- src/store/naive/collection.h	2012-06-28 04:14:03 +0000
+++ src/store/naive/collection.h	2012-06-29 13:52:27 +0000
@@ -54,7 +54,7 @@
 
   zorba::xs_integer size() const  = 0;
 
-  zorba::store::Iterator_t getIterator() = 0;
+  zorba::store::Iterator_t getIterator(const xs_integer& aSkip) = 0;
 
   virtual zorba::store::Item_t nodeAt(xs_integer position) = 0;
 

=== modified file 'src/store/naive/simple_collection.cpp'
--- src/store/naive/simple_collection.cpp	2012-06-28 04:14:03 +0000
+++ src/store/naive/simple_collection.cpp	2012-06-29 13:52:27 +0000
@@ -81,9 +81,9 @@
   Note: it is allowed to have several concurrent iterators on the same collection
   but each iterator should be used by a single thread only.
 ********************************************************************************/
-store::Iterator_t SimpleCollection::getIterator()
+store::Iterator_t SimpleCollection::getIterator(const xs_integer& aSkip)
 {
-  return new CollectionIter(this);
+  return new CollectionIter(this, aSkip);
 }
 
 
@@ -607,10 +607,13 @@
 /*******************************************************************************
 
 ********************************************************************************/
-SimpleCollection::CollectionIter::CollectionIter(SimpleCollection* collection)
+SimpleCollection::CollectionIter::CollectionIter(
+    SimpleCollection* collection,
+    const xs_integer& aSkip)
   :
   theCollection(collection),
-  theHaveLock(false)
+  theHaveLock(false),
+  theSkip(aSkip)
 {
 }
 
@@ -634,6 +637,7 @@
   theHaveLock = true;
 
   theIterator = theCollection->theXmlTrees.begin();
+  theIterator += to_xs_long(theSkip);
   theEnd = theCollection->theXmlTrees.end();
 }
 
@@ -668,6 +672,7 @@
 void SimpleCollection::CollectionIter::reset()
 {
   theIterator = theCollection->theXmlTrees.begin();
+  theIterator += to_xs_long(theSkip);
   theEnd = theCollection->theXmlTrees.end();
 }
 

=== modified file 'src/store/naive/simple_collection.h'
--- src/store/naive/simple_collection.h	2012-06-28 04:14:03 +0000
+++ src/store/naive/simple_collection.h	2012-06-29 13:52:27 +0000
@@ -54,9 +54,10 @@
     checked_vector<store::Item_t>::iterator theIterator;
     checked_vector<store::Item_t>::iterator theEnd;
     bool                                    theHaveLock;
+    xs_integer                              theSkip;
 
   public:
-    CollectionIter(SimpleCollection* collection);
+    CollectionIter(SimpleCollection* collection, const xs_integer& aSkip);
 
     ~CollectionIter();
 
@@ -109,7 +110,7 @@
 
   TreeId createTreeId();
 
-  store::Iterator_t getIterator();
+  store::Iterator_t getIterator(const xs_integer& aSkip);
 
   void addNode(store::Item* node, xs_integer position = xs_integer(-1));
 

=== added file 'test/rbkt/ExpQueryResults/zorba/collections/paging_1.xml.res'
--- test/rbkt/ExpQueryResults/zorba/collections/paging_1.xml.res	1970-01-01 00:00:00 +0000
+++ test/rbkt/ExpQueryResults/zorba/collections/paging_1.xml.res	2012-06-29 13:52:27 +0000
@@ -0,0 +1,1 @@
+<d/><e/><a/><b/><c/><d/><e/>

=== added file 'test/rbkt/ExpQueryResults/zorba/collections/paging_2.xml.res'
--- test/rbkt/ExpQueryResults/zorba/collections/paging_2.xml.res	1970-01-01 00:00:00 +0000
+++ test/rbkt/ExpQueryResults/zorba/collections/paging_2.xml.res	2012-06-29 13:52:27 +0000
@@ -0,0 +1,1 @@
+2 5 0

=== added file 'test/rbkt/ExpQueryResults/zorba/collections/paging_3.xml.res'
--- test/rbkt/ExpQueryResults/zorba/collections/paging_3.xml.res	1970-01-01 00:00:00 +0000
+++ test/rbkt/ExpQueryResults/zorba/collections/paging_3.xml.res	2012-06-29 13:52:27 +0000
@@ -0,0 +1,1 @@
+<d/><e/><a/><b/><c/><d/><e/>

=== added file 'test/rbkt/ExpQueryResults/zorba/collections/paging_4.xml.res'
--- test/rbkt/ExpQueryResults/zorba/collections/paging_4.xml.res	1970-01-01 00:00:00 +0000
+++ test/rbkt/ExpQueryResults/zorba/collections/paging_4.xml.res	2012-06-29 13:52:27 +0000
@@ -0,0 +1,1 @@
+2 5 0

=== added file 'test/rbkt/Queries/zorba/collections/paging_1.xq'
--- test/rbkt/Queries/zorba/collections/paging_1.xq	1970-01-01 00:00:00 +0000
+++ test/rbkt/Queries/zorba/collections/paging_1.xq	2012-06-29 13:52:27 +0000
@@ -0,0 +1,20 @@
+import module namespace ddl = "http://www.zorba-xquery.com/modules/store/static/collections/ddl";
+import module namespace dml = "http://www.zorba-xquery.com/modules/store/static/collections/dml";
+import module namespace ns = "http://example.org/datamodule/" at "collections.xqdata";
+
+declare namespace ann = "http://www.zorba-xquery.com/annotations";
+
+declare %ann:sequential function local:test()
+{
+  ddl:create(xs:QName("ns:test2"));
+  dml:insert-nodes(xs:QName("ns:test2"), <a/>);
+  dml:insert-nodes(xs:QName("ns:test2"), <b/>);
+  dml:insert-nodes(xs:QName("ns:test2"), (<c/>, <d/>, <e/>));
+  (
+    dml:collection(xs:QName("ns:test2"), 3),
+    dml:collection(xs:QName("ns:test2"), -1)
+  )
+};
+
+local:test()
+

=== added file 'test/rbkt/Queries/zorba/collections/paging_2.xq'
--- test/rbkt/Queries/zorba/collections/paging_2.xq	1970-01-01 00:00:00 +0000
+++ test/rbkt/Queries/zorba/collections/paging_2.xq	2012-06-29 13:52:27 +0000
@@ -0,0 +1,21 @@
+import module namespace ddl = "http://www.zorba-xquery.com/modules/store/static/collections/ddl";
+import module namespace dml = "http://www.zorba-xquery.com/modules/store/static/collections/dml";
+import module namespace ns = "http://example.org/datamodule/" at "collections.xqdata";
+
+declare namespace ann = "http://www.zorba-xquery.com/annotations";
+
+declare %ann:sequential function local:test()
+{
+  ddl:create(xs:QName("ns:test2"));
+  dml:insert-nodes(xs:QName("ns:test2"), <a/>);
+  dml:insert-nodes(xs:QName("ns:test2"), <b/>);
+  dml:insert-nodes(xs:QName("ns:test2"), (<c/>, <d/>, <e/>));
+  (
+    fn:count(dml:collection(xs:QName("ns:test2"), 3)),
+    fn:count(dml:collection(xs:QName("ns:test2"), -1)),
+    fn:count(dml:collection(xs:QName("ns:test2"), 100))
+  )
+};
+
+local:test()
+

=== added file 'test/rbkt/Queries/zorba/collections/paging_3.xq'
--- test/rbkt/Queries/zorba/collections/paging_3.xq	1970-01-01 00:00:00 +0000
+++ test/rbkt/Queries/zorba/collections/paging_3.xq	2012-06-29 13:52:27 +0000
@@ -0,0 +1,20 @@
+import module namespace ddl = "http://www.zorba-xquery.com/modules/store/dynamic/collections/ddl";
+import module namespace dml = "http://www.zorba-xquery.com/modules/store/dynamic/collections/dml";
+
+declare namespace ann = "http://www.zorba-xquery.com/annotations";
+declare namespace ns = "http://www.zorba-xquery.com/test";
+
+declare %ann:sequential function local:test()
+{
+  ddl:create(xs:QName("ns:test2"));
+  dml:insert-nodes-last(xs:QName("ns:test2"), <a/>);
+  dml:insert-nodes-last(xs:QName("ns:test2"), <b/>);
+  dml:insert-nodes-last(xs:QName("ns:test2"), (<c/>, <d/>, <e/>));
+  (
+    dml:collection(xs:QName("ns:test2"), 3),
+    dml:collection(xs:QName("ns:test2"), -1)
+  )
+};
+
+local:test()
+

=== added file 'test/rbkt/Queries/zorba/collections/paging_4.xq'
--- test/rbkt/Queries/zorba/collections/paging_4.xq	1970-01-01 00:00:00 +0000
+++ test/rbkt/Queries/zorba/collections/paging_4.xq	2012-06-29 13:52:27 +0000
@@ -0,0 +1,21 @@
+import module namespace ddl = "http://www.zorba-xquery.com/modules/store/dynamic/collections/ddl";
+import module namespace dml = "http://www.zorba-xquery.com/modules/store/dynamic/collections/dml";
+
+declare namespace ann = "http://www.zorba-xquery.com/annotations";
+declare namespace ns = "http://www.zorba-xquery.com/test";
+
+declare %ann:sequential function local:test()
+{
+  ddl:create(xs:QName("ns:test2"));
+  dml:insert-nodes-last(xs:QName("ns:test2"), <a/>);
+  dml:insert-nodes-last(xs:QName("ns:test2"), <b/>);
+  dml:insert-nodes-last(xs:QName("ns:test2"), (<c/>, <d/>, <e/>));
+  (
+    fn:count(dml:collection(xs:QName("ns:test2"), 3)),
+    fn:count(dml:collection(xs:QName("ns:test2"), -1)),
+    fn:count(dml:collection(xs:QName("ns:test2"), 100))
+  )
+};
+
+local:test()
+