Re: [Zorba-coders] [Merge] lp:~matthias-brantner/zorba/caching into lp:zorba
I think there is a bug in user_function::computeResultCaching, starting at line 542. The condition: if (lExplicitCacheRequest) appears twice and theCacheResults will actually be set to true if the udf is sequential or non-deterministic. -- https://code.launchpad.net/~matthias-brantner/zorba/caching/+merge/82787 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:~matthias-brantner/zorba/caching into lp:zorba
Matthias, can you change the ownership to zorba-coders so that I can do some small changes (documentation and style)? -- https://code.launchpad.net/~matthias-brantner/zorba/caching/+merge/82787 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:~matthias-brantner/zorba/caching into lp:zorba
The attempt to merge lp:~matthias-brantner/zorba/caching into lp:zorba failed. Below is the output from the failed tests. CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:272 (message): Validation queue job caching-2011-11-21T22-11-29.225Z is finished. The final status was: 3 tests did not succeed - changes not commited. Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake -- https://code.launchpad.net/~matthias-brantner/zorba/caching/+merge/82787 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:~matthias-brantner/zorba/caching into lp:zorba
There are additional revisions which have not been approved in review. Please seek review and approval of these new revisions. -- https://code.launchpad.net/~matthias-brantner/zorba/caching/+merge/82483 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:~matthias-brantner/zorba/caching into lp:zorba
Review: Approve -- https://code.launchpad.net/~matthias-brantner/zorba/caching/+merge/82483 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:~matthias-brantner/zorba/caching into lp:zorba
Adressed comments 1-4. -- https://code.launchpad.net/~matthias-brantner/zorba/caching/+merge/82482 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:~matthias-brantner/zorba/caching into lp:zorba
Here are a few problems I can see so far: 1. I don't think we can do function caching for variadic functions. The current implementation of user_function::computeResultCaching certainly does not support variadic functions, but more importantly, we need a fixed number of params to form the index key (unless we start thinking towards having more than one cache for variadic functions). 2. The user_function::computeResultCaching does the full computation every time it is invoked (and it may be invoked multiple times from the codegen). It should cache the result of the 1st computation and simply return it afterwards. 3. In UDFunctionCallIterator::createCache, the line: lSpec.theKeyTypes[i] = sig[i]->get_qname().getp(); is not correct if the type of the param is a user-defined type (because the store will not understand its name). You must call getBaseBuiltinType() of the param type first (like the translator does in line 4401). 4. It's probably worthwhile to do something so that we don't have to recompute the args in case of a cache miss. What we have to do is create a temp sequence for each arg, and fill it with the corresponding arg value; then bind the arg reference to that temp seq, instead the arg wrapper. 5. It's probably worthwhile to allow subtypes of xs:anyAtomicType? as param types. Index creation already supports "null" keys, but we will need a new probe function. This we should probably do as a phase-2 task. -- https://code.launchpad.net/~matthias-brantner/zorba/caching/+merge/81304 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:~matthias-brantner/zorba/caching into lp:zorba
The attempt to merge lp:~matthias-brantner/zorba/caching into lp:zorba failed. Below is the output from the failed tests. CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:272 (message): Validation queue job caching-2011-11-04T17-29-08.827Z is finished. The final status was: 31 tests did not succeed - changes not commited. Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake -- https://code.launchpad.net/~matthias-brantner/zorba/caching/+merge/81304 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
