Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/skip-items into lp:zorba
Review: Approve -- https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/skip-items into lp:zorba
I've fixed the crash and added the query as zorba/eval/eval16.xq. -- -- https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/skip-items into lp:zorba
Review: Needs Fixing
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"),(,));
subsequence(e:eval("dml:collection(xs:QName('ddl:test2'))"), 2, 1)
--
https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723
Your team Zorba Coders is subscribed to branch lp:zorba.
--
Mailing list: https://launchpad.net/~zorba-coders
Post to : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/skip-items into lp:zorba
> - 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. -- -- https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/skip-items into lp:zorba
Review: Needs Information 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. -- https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/skip-items into lp:zorba
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. -- https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/skip-items into lp:zorba
> - 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. -- -- https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/skip-items into lp:zorba
Review: Needs Fixing - 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 -- https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
Re: [Zorba-coders] [Merge] lp:~zorba-coders/zorba/skip-items into lp:zorba
Review: Approve -- https://code.launchpad.net/~zorba-coders/zorba/skip-items/+merge/174723 Your team Zorba Coders is subscribed to branch lp:zorba. -- Mailing list: https://launchpad.net/~zorba-coders Post to : zorba-coders@lists.launchpad.net Unsubscribe : https://launchpad.net/~zorba-coders More help : https://help.launchpad.net/ListHelp
