Merge lp:~zorba-coders/zorba/skip-items into lp:zorba

[Nicolae Brinza]

Implemented the Skip-items facility

Implemented the Skip-items facility

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for the following merge proposals:
https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723

Progress dashboard at http://jenkins.lambda.nu/view/ValidationQueue

[Zorba Build Bot (zorba-buildbot)]

Validation queue result for https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723

Stage "TestZorbaUbuntu" failed.
3 tests failed (8344 total tests run).

Check test results at http://jenkins.lambda.nu/job/TestZorbaUbuntu/81/testReport/ to view the results.

[Nicolae Brinza (nbrinza)]

review: Approve

[Matthias Brantner (matthias-brantner)]

- Shouldn't skip take an unsigned integer?
- Why is the while loop in collections_impl.cpp:397 necessary if initCollection is called before? I think there might be a bug in initCollection. Specifically, the else block in line 298 is never called because the CollectionIterator is not rewritten anymore (i.e. theChildren.size() will always return one)
- state->theIteratorOpened == false => !state->theIteratorOpened
- could we implement and test skip for the EvalIterator as well?
- What about the index probe iterators (e.g. ProbeIndexPointValueIterator), they also skip

review: Needs Fixing

[Nicolae Brinza (nbrinza)]

> - Shouldn't skip take an unsigned integer?

The parameters to fn:subsequence and co are all signed. E.g. you can pass a negative skip.

> - Why is the while loop in collections_impl.cpp:397 necessary if
> initCollection is called before? I think there might be a bug in
> initCollection. Specifically, the else block in line 298 is never called
> because the CollectionIterator is not rewritten anymore (i.e.
> theChildren.size() will always return one)

You're very right. It was not really a bug, but the skip optimization didn't really happen because it was the while() that was doing the skipping instead of the getIterator(skip). I've fixed that. But the else block at line 298 cannot be deleted as the 3-parameter function was not only used by the rewriter, e.g. the collections/paging_1 to _5 use it.

> - state->theIteratorOpened == false => !state->theIteratorOpened

Fixed.

> - could we implement and test skip for the EvalIterator as well?

All iterators have the default implementation of skip in plan_iterator.cpp. How can we improve that implementation for EvalIterator?

> - What about the index probe iterators (e.g. ProbeIndexPointValueIterator),
> they also skip

I was not aware they do. I'll write an optimization for them as well.

--

[lp:~zorba-coders/zorba/skip-items]

11551. By Nicolae Brinza on 2013-07-23

Fixed review issues.

11552. By Nicolae Brinza on 2013-07-23

Merged with Zorba trunk

[Nicolae Brinza (nbrinza)]

I've looked into the index probe iterators but they already optimize any skipping. They take a Skip parameter and then they push it into an underlying iterator e.g. ProbeValueTreeIndexIterator which handles the skip internally. So there is nothing to be done there.

I've pushed the pending fixes and the branch is ready for merging.

[Matthias Brantner (matthias-brantner)]

Looks good to me. Only two issues.

- The thing with the probe iterators is that there are additional functions called probe-*-skip that allow
you to skip explicitly. However, subsequence is not rewritten into these functions. We will need to override the skip function in all the probe iterators (similar to count) such that subsequence(probe-*) will be executed efficiently. We can do this is a second step though. I'll create a follow up story for this.

- In ZorbaCollectionIterator::initCollection lines 307 and 326, you eventually add skipCount twice. Something smells spooky here.

review: Needs Information

[lp:~zorba-coders/zorba/skip-items]

11553. By Nicolae Brinza on 2013-07-25

Fixed the double addition of skipCount in ZorbaCollectionIterator::initCollection; added skip() to the EvalIterator.

[Nicolae Brinza (nbrinza)]

> - In ZorbaCollectionIterator::initCollection lines 307 and 326, you eventually
> add skipCount twice. Something smells spooky here.

Yes, it seems it was added twice. I've fixed it.

I've also added the skip() function to the EvalIterator.

--

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for the following merge proposals:
https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723

Progress dashboard at http://jenkins.lambda.nu/view/ValidationQueue

[Zorba Build Bot (zorba-buildbot)]

Validation queue result for https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723

Stage "TestZorbaUbuntu" failed.
1 tests failed (8415 total tests run).

Check test results at http://jenkins.lambda.nu/job/TestZorbaUbuntu/123/testReport/ to view the results.

[lp:~zorba-coders/zorba/skip-items]

11554. By Nicolae Brinza on 2013-07-25

Fixed a regression

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for the following merge proposals:
https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723

Progress dashboard at http://jenkins.lambda.nu/view/ValidationQueue

[Zorba Build Bot (zorba-buildbot)]

Voting criteria failed for the following merge proposals:

https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723 :
Votes: {'Needs Information': 1, 'Approve': 1}

[Zorba Build Bot (zorba-buildbot)]

Validation queue result for https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723

Stage "CommitZorba" failed.

Check console output at http://jenkins.lambda.nu/job/CommitZorba/56/console to view the results.

[Matthias Brantner (matthias-brantner)]

The following query crashes:

xquery version "3.0";

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";
import module namespace e = "http://www.zorba-xquery.com/modules/reflection";

ddl:create(xs:QName("ddl:test2"),(<center1/>,<oldlast/>));

subsequence(e:eval("dml:collection(xs:QName('ddl:test2'))"), 2, 1)

review: Needs Fixing

[lp:~zorba-coders/zorba/skip-items]

11555. By Nicolae Brinza on 2013-07-26

Fixed a bug with eval() and the new skip() functionality.

11556. By Nicolae Brinza on 2013-07-26

Cleanup

[Nicolae Brinza (nbrinza)]

I've fixed the crash and added the query as zorba/eval/eval16.xq.

--

[Matthias Brantner (matthias-brantner)]

review: Approve

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for the following merge proposals:
https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723

Progress dashboard at http://jenkins.lambda.nu/view/ValidationQueue

[Zorba Build Bot (zorba-buildbot)]

Validation queue result for https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723

Stage "AddTestSuitesUbuntu" failed.

Check console output at http://jenkins.lambda.nu/job/AddTestSuitesUbuntu/125/console to view the results.

[Zorba Build Bot (zorba-buildbot)]

Validation queue starting for the following merge proposals:
https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723

Progress dashboard at http://jenkins.lambda.nu/view/ValidationQueue

[Zorba Build Bot (zorba-buildbot)]

Validation queue succeeded - proposal merged!

=== modified file 'src/functions/func_sequences_impl.cpp'
--- src/functions/func_sequences_impl.cpp	2013-05-14 05:21:11 +0000
+++ src/functions/func_sequences_impl.cpp	2013-07-26 12:54:41 +0000
@@ -43,127 +43,6 @@
 
 
 /*******************************************************************************
-
-********************************************************************************/
-bool rewriteSubsequenceCollection(
-    static_context* aSctx,
-    const QueryLoc& aLoc,
-    std::vector<PlanIter_t>& aArgs,
-    bool aIsIntSubsequence)
-{
-  ZorbaCollectionIterator* collIter = 
-  dynamic_cast<ZorbaCollectionIterator*>(aArgs[0].getp());
-  assert(collIter);
-
-  std::vector<PlanIter_t>& lCollectionArgs = collIter->getChildren();
-
-  SingletonIterator* lPosIter = dynamic_cast<SingletonIterator*>(aArgs[1].getp());
-  if (lPosIter == NULL)
-  {
-    return false;
-  }
-
-  xs_long pos;
-  const store::Item_t& lPosItem = lPosIter->getValue();
-
-  try
-  {
-    if (aIsIntSubsequence)
-    {
-      pos = lPosItem->getLongValue();
-    }
-    else
-    {
-      xs_double dpos = lPosItem->getDoubleValue().round();
-      xs_integer ipos(dpos.getNumber());
-      pos = to_xs_long(ipos);
-    }
-  }
-  catch (std::exception const&)
-  {
-    return false;
-  }
-
-  if (pos <= 1)
-  {
-    // if the start position is less than 1 we can't push this down into
-    // the collection skip parameter because the result won't be equivalent.
-    return false;
-  }
-
-  // prepare helper
-  store::Item_t lItemOne;
-  GENV_ITEMFACTORY->createInteger(lItemOne, Integer(1));
-
-  int lNumCollArgs = lCollectionArgs.size();
-  if (lNumCollArgs == 1)
-  {
-    // argument is of type collection(qname)
-    // simply move the (pos-1) of subsequence into the skip of 
-    // collection function
-    // subsequence(collection(qname), 10, 20) 
-    //   -> subsequence(collection(qname, 10-1), 1, 20)
-    PlanIter_t& lNewCollSkipIter = aArgs[1];
-    PlanIter_t lOneIter = new SingletonIterator(aSctx, aLoc, lItemOne);
-
-    lCollectionArgs.push_back(
-      new NumArithIterator<zorba::SubtractOperation>(aSctx,
-                                                     collIter->getLocation(),
-                                                     lNewCollSkipIter,
-                                                     lOneIter)
-                              );
-  }
-  else if (lNumCollArgs <= 3 && lNumCollArgs != 0)
-  {
-    // argument is of type collection(qname,skip) or 
-    // collection(qname,start_uri,skip)
-    int lSkipPosition = 1;
-    if (lNumCollArgs == 3) 
-    {
-      // collection function with start reference -> skip is the 3rd param
-      lSkipPosition = 2;
-    }
-      
-    // add position-1 of subsequence to collection skip
-    // subsequence(collection(qname, 10), 10, 20) 
-    //   -> subsequence(collection(qname, 10+10-1), 10, 20)
-    PlanIter_t& lOldCollSkipIter = lCollectionArgs[lSkipPosition];
-    PlanIter_t lOneIter = new SingletonIterator (aSctx, aLoc, lItemOne);
-    PlanIter_t& lSubseqPosIter = aArgs[1];
-
-    PlanIter_t lCollSkipAdditionIter = 
-    new NumArithIterator<zorba::SubtractOperation>(aSctx,
-                                                   collIter->getLocation(),
-                                                   lSubseqPosIter,
-                                                   lOneIter);
-    lCollectionArgs[lSkipPosition] = 
-    new NumArithIterator<zorba::AddOperation>(aSctx,
-                                              collIter->getLocation(), 
-                                              lOldCollSkipIter,
-                                              lCollSkipAdditionIter);
-  }
-  else
-  {
-    // no collection function with 0 or >3 params
-    assert(false);
-  }
-
-  aArgs[0] = new ZorbaCollectionIterator(aSctx,
-                                         collIter->getLocation(),
-                                         lCollectionArgs,
-                                         collIter->isDynamic());
-
-  // after pushing the position param down we need to rewrite the actual 
-  // position to 1:
-  // subsequence(collection(qname, 10+10-1), 10, 20) 
-  //   -> subsequence(collection(qname, 10+10-1), 1, 20)
-  aArgs[1] = new SingletonIterator(aSctx, aLoc, lItemOne);
-
-  return true;
-}
-
-
-/*******************************************************************************
 ********************************************************************************/
 xqtref_t fn_unordered::getReturnType(const fo_expr* caller) const
 {
@@ -573,8 +452,7 @@
     const QueryLoc& aLoc,
     std::vector<PlanIter_t>& aArgs,
     expr& aAnn) const
-{
-  const std::type_info& lFirstArgType = typeid(*aArgs[0]);
+{  
   fo_expr& subseqExpr = static_cast<fo_expr&>(aAnn);
   const expr* inputExpr = subseqExpr.get_arg(0);
   const expr* posExpr = subseqExpr.get_arg(1);
@@ -619,23 +497,8 @@
         return aArgs[0];
     }
   }
-  else if (typeid(ZorbaCollectionIterator) == lFirstArgType)
-  {
-    // push down position param into collection skip if possible
-    if (rewriteSubsequenceCollection(aSctx, aLoc, aArgs, false /*no int*/))
-    {
-      // we have rewritten the subsequence to start at the beginning.
-      // if there is no length param we can remove the entire
-      // subsequence function
-      // subsequence(collection(qname, 10), 1) -> collection(qname, 10)
-      if (aArgs.size() == 2)
-      {
-        return aArgs[0];
-      }
-    }
-  }
 
- done:
+done:
   return new FnSubsequenceIterator(aSctx, aLoc, aArgs);
 }
 
@@ -736,23 +599,8 @@
       return aArgs[0];
     }
   }
-  else if (typeid(ZorbaCollectionIterator) == lFirstArgType)
-  {
-    // push down position param into collection skip if possible
-    if (rewriteSubsequenceCollection(aSctx, aLoc, aArgs, true /*flag int*/))
-    {
-      // we have rewritten the subsequence to start from the beginning.
-      // if there is no length param we can remove the entire
-      // subsequence function
-      // subsequence(collection(qname, 10), 1) -> collection(qname, 10)
-      if (aArgs.size() == 2)
-      {
-        return aArgs[0];
-      }
-    }
-  }
 
- done:
+done:
   return new SubsequenceIntIterator(aSctx, aLoc, aArgs);
 }
 

=== modified file 'src/runtime/api/plan_wrapper.cpp'
--- src/runtime/api/plan_wrapper.cpp	2013-03-17 13:55:28 +0000
+++ src/runtime/api/plan_wrapper.cpp	2013-07-26 12:54:41 +0000
@@ -162,6 +162,17 @@
 /*******************************************************************************
 
 ********************************************************************************/
+bool PlanWrapper::skip(int64_t count)
+{
+  ZORBA_ASSERT(theIsOpen);
+
+  return theIterator->skip(count, *thePlanState);
+}
+
+
+/*******************************************************************************
+
+********************************************************************************/
 void PlanWrapper::reset()
 {
   ZORBA_ASSERT(theIsOpen);

=== modified file 'src/runtime/api/plan_wrapper.h'
--- src/runtime/api/plan_wrapper.h	2013-03-04 21:00:58 +0000
+++ src/runtime/api/plan_wrapper.h	2013-07-26 12:54:41 +0000
@@ -97,6 +97,8 @@
 
   bool next(store::Item_t& item);
 
+  bool skip(int64_t count);
+
   void reset();
 
   void close();

=== modified file 'src/runtime/base/plan_iterator.cpp'
--- src/runtime/base/plan_iterator.cpp	2013-05-28 18:20:54 +0000
+++ src/runtime/base/plan_iterator.cpp	2013-07-26 12:54:41 +0000
@@ -31,6 +31,10 @@
 
 #include "diagnostics/util_macros.h"
 
+#ifndef NDEBUG
+#include "system/properties.h"
+#endif
+
 namespace zorba
 {
 
@@ -180,6 +184,18 @@
 }
 
 
+bool PlanIterator::skip(int64_t count, PlanState& planState) const
+{
+  bool have_more_items = true;
+  store::Item_t item;
+
+  while (count-- > 0 && (have_more_items = consumeNext(item, this, planState)))
+    ;
+
+  return have_more_items;
+}
+
+
 #ifndef NDEBUG
 bool PlanIterator::consumeNext(
     store::Item_t& result,
@@ -196,7 +212,12 @@
 
   if (planState.theCompilerCB->theConfig.print_item_flow)
   {
-    std::cout << "next (" << iter << " = " << typeid (*iter).name()
+    if (Properties::instance()->stableIteratorIds())
+      std::cout << "next (" << iter->getId();
+    else
+      std::cout << "next (" << iter;
+
+    std::cout << " = " << typeid (*iter).name()
               << ") -> "
               << "status: " << status << " -> "
               << ((status && result != NULL) ? result->show().c_str() : "null")

=== modified file 'src/runtime/base/plan_iterator.h'
--- src/runtime/base/plan_iterator.h	2013-05-28 18:20:54 +0000
+++ src/runtime/base/plan_iterator.h	2013-07-26 12:54:41 +0000
@@ -401,6 +401,19 @@
   virtual bool count(store::Item_t& result, PlanState& planState) const;
 
   /**
+   * Skip a number of items from the Plan's sequence. Classes can overwrite
+   * this functions to optimize the skipping by jumping directly to the
+   * desired position in the sequence.
+   *
+   * Returns true if the entire sequence has been consumed, false otherwise.
+   *
+   * @param count the number of items to be skipped
+   * @param planState the state plan
+   *
+   */
+  virtual bool skip(int64_t count, PlanState &planState) const;
+
+  /**
    * Produce the next item and return it to the caller. Implicitly, the first
    * call of 'producNext' initializes the iterator and allocates resources
    * (main memory, file descriptors, etc.).

=== modified file 'src/runtime/collections/collections_impl.cpp'
--- src/runtime/collections/collections_impl.cpp	2013-05-02 18:34:27 +0000
+++ src/runtime/collections/collections_impl.cpp	2013-07-26 12:54:41 +0000
@@ -277,17 +277,13 @@
 }
 
 
-bool ZorbaCollectionIterator::nextImpl(
-    store::Item_t& result,
-    PlanState& planState) const
+void ZorbaCollectionIterator::initCollection(PlanState& planState, int64_t skipCount) const
 {
   store::Item_t name;
+  xs_integer lSkip;
   store::Collection_t collection;
-  xs_integer lSkip;
-  zstring lStart;
 
-  ZorbaCollectionIteratorState* state;
-  DEFAULT_STACK_INIT(ZorbaCollectionIteratorState, state, planState);
+  ZorbaCollectionIteratorState* state = StateTraitsImpl<ZorbaCollectionIteratorState>::getState(planState, theStateOffset);
 
   consumeNext(name, theChildren[0].getp(), planState);
 
@@ -295,16 +291,20 @@
 
   if (theChildren.size() == 1)
   {
-    state->theIterator = collection->getIterator();
+    if (skipCount < 0)
+      skipCount = 0;
+    lSkip = skipCount;
+    state->theIterator = collection->getIterator(lSkip);
   }
   else
   {
+    zstring lStart;
     bool lRefPassed = theChildren.size() >= 3;
-    
+
     // read positional skip parameter
     store::Item_t lSkipItem;
     consumeNext(lSkipItem, theChildren[(lRefPassed ? 2 : 1)].getp(), planState);
-    lSkip = lSkipItem->getIntegerValue(); 
+    lSkip = lSkipItem->getIntegerValue() + skipCount;
 
     // negative skip is not allowed
     if (lSkip.sign() < 0)
@@ -320,7 +320,7 @@
     {
       store::Item_t lRefItem;
       consumeNext(lRefItem, theChildren[1].getp(), planState);
-      lStart = lRefItem->getString(); 
+      lStart = lRefItem->getString();
       try
       {
         state->theIterator = collection->getIterator(lSkip, lStart);
@@ -346,6 +346,29 @@
   }
 
   state->theIteratorOpened = true;
+}
+
+
+bool ZorbaCollectionIterator::nextImpl(
+    store::Item_t& result,
+    PlanState& planState) const
+{
+  store::Item_t name;
+  xs_integer lSkip;
+  store::Collection_t collection;
+
+  ZorbaCollectionIteratorState* state;
+  DEFAULT_STACK_INIT(ZorbaCollectionIteratorState, state, planState);
+
+  if (state->theIterator.getp() != NULL && state->theIteratorOpened == false)
+  {
+    ZORBA_ASSERT (false && "nextImpl() called past iterator end");
+    return false;
+  }
+  else if ( ! state->theIteratorOpened)
+  {
+    initCollection(planState, 0);
+  }
 
   while (state->theIterator->next(result))
     STACK_PUSH(true, state);
@@ -358,6 +381,25 @@
 }
 
 
+bool ZorbaCollectionIterator::skip(int64_t count, PlanState& planState) const
+{  
+  ZorbaCollectionIteratorState* state = StateTraitsImpl<ZorbaCollectionIteratorState>::getState(planState, theStateOffset);
+
+  if (state->theIterator.getp() != NULL && state->theIteratorOpened == false)
+  {
+    ZORBA_ASSERT (false && "nextImpl() called past iterator end");
+    return false;
+  }
+  else if ( ! state->theIteratorOpened)
+  {
+    initCollection(planState, count);
+    return true;
+  }
+  else
+    return false;
+}
+
+
 bool ZorbaCollectionIterator::count(store::Item_t& result, PlanState& planState) const
 {
   if (!isCountOptimizable())

=== modified file 'src/runtime/collections/pregenerated/collections.h'
--- src/runtime/collections/pregenerated/collections.h	2013-03-24 20:40:03 +0000
+++ src/runtime/collections/pregenerated/collections.h	2013-07-26 12:54:41 +0000
@@ -291,6 +291,8 @@
 public:
   bool isCountOptimizable() const;
   bool count(store::Item_t& result, PlanState& planState) const;
+  bool skip(int64_t count, PlanState& planState) const;
+  void initCollection(PlanState& planState, int64_t skipCount) const;
   void accept(PlanIterVisitor& v) const;
 
   bool nextImpl(store::Item_t& result, PlanState& aPlanState) const;

=== modified file 'src/runtime/eval/eval.cpp'
--- src/runtime/eval/eval.cpp	2013-03-24 20:40:03 +0000
+++ src/runtime/eval/eval.cpp	2013-07-26 12:54:41 +0000
@@ -71,6 +71,17 @@
 /****************************************************************************//**
 
 ********************************************************************************/
+void EvalIteratorState::reset(PlanState& planState)
+{
+  PlanIteratorState::reset(planState);
+
+  thePlanWrapper = NULL;
+}
+
+
+/****************************************************************************//**
+
+********************************************************************************/
 EvalIterator::EvalIterator(
     static_context* sctx,
     const QueryLoc& loc,
@@ -126,6 +137,73 @@
 /****************************************************************************//**
 
 ********************************************************************************/
+void EvalIterator::init(
+    bool doCount,
+    PlanState& planState) const
+{
+  store::Item_t item;
+  EvalIteratorState* state = StateTraitsImpl<EvalIteratorState>::getState(planState, theStateOffset);
+
+  CONSUME(item, 0);
+
+  // Create the "import" sctx. The importOuterEnv() method (called below) will
+  // register into the importSctx (a) the outer vars of the eval query and (b)
+  // the expression-level ns bindings of the outer query at the place where
+  // the eval call appears at.
+  static_context* importSctx = theSctx->create_child_context();
+
+  // Create the root sctx for the eval query
+  static_context* evalSctx = importSctx->create_child_context();
+
+  // Create the ccb for the eval query
+  CompilerCB* evalCCB = new CompilerCB(*planState.theCompilerCB);
+  evalCCB->theIsEval = true;
+  evalCCB->theRootSctx = evalSctx;
+  evalCCB->theConfig.for_serialization_only = !theDoNodeCopy;
+  (evalCCB->theSctxMap)[1] = evalSctx;
+
+  state->ccb.reset(evalCCB);
+
+  // Create the dynamic context for the eval query
+  dynamic_context* evalDctx = new dynamic_context(planState.theGlobalDynCtx);
+  state->dctx.reset(evalDctx);
+
+  // Import the outer environment.
+  ulong maxOuterVarId;
+  importOuterEnv(planState, evalCCB, importSctx, evalDctx, maxOuterVarId);
+
+  // If we are here after a reset, we must set state->thePlanWrapper to NULL
+  // before reseting the state->thePlan. Otherwise, the current state->thePlan
+  // will be destroyed first, and then we will attempt to close it when
+  // state->thePlanWrapper is reset later.
+  state->thePlanWrapper = NULL;
+
+  // Compile
+  state->thePlan = compile(evalCCB, item->getStringValue(), maxOuterVarId, doCount);
+
+  planState.theCompilerCB->theNextVisitId = evalCCB->theNextVisitId + 1;
+
+  // Set the values for the (explicit) external vars of the eval query
+  setExternalVariables(evalCCB, importSctx, evalSctx, evalDctx);
+
+  // Execute
+  state->thePlanWrapper = new PlanWrapper(state->thePlan,
+                                          evalCCB,
+                                          evalDctx,
+                                          planState.theQuery,
+                                          planState.theStackDepth + 1,
+                                          state->ccb->theHaveTimeout,
+                                          state->ccb->theTimeout);
+
+  state->thePlanWrapper->checkDepth(loc);
+
+  state->thePlanWrapper->open();
+}
+
+
+/****************************************************************************//**
+
+********************************************************************************/
 bool EvalIterator::nextORcount(
     bool doCount,
     store::Item_t& result,
@@ -136,62 +214,8 @@
 
   DEFAULT_STACK_INIT(EvalIteratorState, state, planState);
 
-  CONSUME(item, 0);
-
-  {
-    // Create the "import" sctx. The importOuterEnv() method (called below) will
-    // register into the importSctx (a) the outer vars of the eval query and (b)
-    // the expression-level ns bindings of the outer query at the place where
-    // the eval call appears at.
-    static_context* importSctx = theSctx->create_child_context();
-
-    // Create the root sctx for the eval query
-    static_context* evalSctx = importSctx->create_child_context();
-
-    // Create the ccb for the eval query
-    CompilerCB* evalCCB = new CompilerCB(*planState.theCompilerCB);
-    evalCCB->theIsEval = true;
-    evalCCB->theRootSctx = evalSctx;
-    evalCCB->theConfig.for_serialization_only = !theDoNodeCopy;
-    (evalCCB->theSctxMap)[1] = evalSctx;
-
-    state->ccb.reset(evalCCB);
-
-    // Create the dynamic context for the eval query
-    dynamic_context* evalDctx = new dynamic_context(planState.theGlobalDynCtx);
-    state->dctx.reset(evalDctx);
-
-    // Import the outer environment.
-    ulong maxOuterVarId;
-    importOuterEnv(planState, evalCCB, importSctx, evalDctx, maxOuterVarId);
-
-    // If we are here after a reset, we must set state->thePlanWrapper to NULL
-    // before reseting the state->thePlan. Otherwise, the current state->thePlan
-    // will be destroyed first, and then we will attempt to close it when
-    // state->thePlanWrapper is reset later.
-    state->thePlanWrapper = NULL;
-
-    // Compile
-    state->thePlan = compile(evalCCB, item->getStringValue(), maxOuterVarId, doCount);
-
-    planState.theCompilerCB->theNextVisitId = evalCCB->theNextVisitId + 1;
-
-    // Set the values for the (explicit) external vars of the eval query
-    setExternalVariables(evalCCB, importSctx, evalSctx, evalDctx);
-
-    // Execute
-    state->thePlanWrapper = new PlanWrapper(state->thePlan,
-                                            evalCCB,
-                                            evalDctx,
-                                            planState.theQuery,
-                                            planState.theStackDepth + 1,
-                                            state->ccb->theHaveTimeout,
-                                            state->ccb->theTimeout);
-
-    state->thePlanWrapper->checkDepth(loc);
-
-    state->thePlanWrapper->open();
-  }
+  if (state->thePlanWrapper.getp() == NULL)
+    init(doCount, planState);
 
   while (state->thePlanWrapper->next(result))
   {
@@ -205,6 +229,20 @@
 
 
 /****************************************************************************//**
+
+********************************************************************************/
+bool EvalIterator::skip(int64_t count, PlanState &planState) const
+{
+  EvalIteratorState* state = StateTraitsImpl<EvalIteratorState>::getState(planState, theStateOffset);
+
+  if (state->thePlanWrapper.getp() == NULL)
+    init(false, planState);
+
+  return state->thePlanWrapper->skip(count);
+}
+
+
+/****************************************************************************//**
   This method imports a static and dynamic environment from the quter query into
   the eval query. In particular:
 

=== modified file 'src/runtime/eval/eval.h'
--- src/runtime/eval/eval.h	2013-03-24 20:40:03 +0000
+++ src/runtime/eval/eval.h	2013-07-26 12:54:41 +0000
@@ -38,6 +38,8 @@
   EvalIteratorState();
 
   ~EvalIteratorState();
+
+  void reset(PlanState& planState);
 };
 
 
@@ -130,7 +132,11 @@
     return nextORcount(true, result, planState);
   }
 
+  bool skip(int64_t count, PlanState &planState) const;
+
 private:
+  void init(bool doCount, PlanState& planState) const;
+
   void importOuterEnv(
       PlanState& planState,
       CompilerCB* evalCCB,

=== modified file 'src/runtime/sequences/sequences_impl.cpp'
--- src/runtime/sequences/sequences_impl.cpp	2013-06-18 18:55:33 +0000
+++ src/runtime/sequences/sequences_impl.cpp	2013-07-26 12:54:41 +0000
@@ -533,11 +533,8 @@
     goto done;
 
   // Consume and skip all input items that are before the startPos
-  for (; startPos > 1; --startPos)
-  {
-    if (!CONSUME(result, 0))
-      goto done;
-  }
+  if (!theChildren[0]->skip(startPos-1, planState))
+    goto done;
 
   if (theChildren.size() < 3 || lengthDouble.isPosInf())
   {
@@ -619,11 +616,8 @@
     goto done;
 
   // Consume and skip all input items that are before the startPos
-  for (; startPos > 0; --startPos)
-  {
-    if (!CONSUME(result, 0))
-      goto done;
-  }
+  if (!theChildren[0]->skip(startPos, planState))
+    goto done;
 
   if (theChildren.size() < 3)
   {
@@ -694,11 +688,8 @@
   --startPos;
 
   // Consume and skip all input items that are before the startPos
-  for (; startPos > 0; --startPos)
-  {
-    if (!CONSUME(result, 0))
-      goto done;
-  }
+  if (!theChildren[0]->skip(startPos, planState))
+    goto done;
 
   if (CONSUME(result, 0))
   {

=== modified file 'src/runtime/spec/collections/collections.xml'
--- src/runtime/spec/collections/collections.xml	2013-04-02 22:54:20 +0000
+++ src/runtime/spec/collections/collections.xml	2013-07-26 12:54:41 +0000
@@ -313,6 +313,16 @@
        <zorba:param name="result" type="store::Item_t&amp;"/>
        <zorba:param name="planState" type="PlanState&amp;"/>
     </zorba:method>
+    
+    <zorba:method name="skip" const="true" return="bool">
+      <zorba:param name="count" type="int64_t"/>
+      <zorba:param name="planState" type="PlanState&amp;"/>
+    </zorba:method>
+    
+    <zorba:method name="initCollection" const="true" return="void">
+      <zorba:param name="planState" type="PlanState&amp;"/>
+      <zorba:param name="skipCount" type="int64_t"/>
+    </zorba:method>
 
     <zorba:state generateInit="false" generateReset="false" generateDestructor="false">
       <zorba:member type="store::Iterator_t" name="theIterator"/>

=== added file 'test/rbkt/ExpQueryResults/zorba/eval/eval16.xml.res'
--- test/rbkt/ExpQueryResults/zorba/eval/eval16.xml.res	1970-01-01 00:00:00 +0000
+++ test/rbkt/ExpQueryResults/zorba/eval/eval16.xml.res	2013-07-26 12:54:41 +0000
@@ -0,0 +1,2 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<oldlast/>

=== added file 'test/rbkt/Queries/zorba/eval/eval16.xq'
--- test/rbkt/Queries/zorba/eval/eval16.xq	1970-01-01 00:00:00 +0000
+++ test/rbkt/Queries/zorba/eval/eval16.xq	2013-07-26 12:54:41 +0000
@@ -0,0 +1,9 @@
+xquery version "3.0";
+
+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";
+import module namespace e = "http://www.zorba-xquery.com/modules/reflection";
+
+ddl:create(xs:QName("ddl:test2"),(<center1/>,<oldlast/>));
+
+subsequence(e:eval("dml:collection(xs:QName('ddl:test2'))"), 2, 1)