Re: [Zorba-coders] [Merge] lp:~davidagraf/zorba/paging_index_probe into lp:zorba
- the probe-index-point-value-skip and probe-index-range-value-skip functions in dml.xq don't declare the $skip parameters (but their documentation does) Fixed. - idml:probe-index-range-value-skip(xs:QName(data:idx-age-range-val), 2, 3, true(), true(), true(), true()) reports 'data:idx-age-range-val: invalid number of arguments to index operation; given 5 expected multiple of 6;' but I'm counting 7 parameters Fixed (was not my fault :-) ). Unfortunately, when trying to write a test for this, I found a bug: https://bugs.launchpad.net/zorba/+bug/1051897 Can you please double check if it is really a bug? I commented on the bug. - It would be great if the iterator plan could mention if the iterator does skipping and/or counting. For example, ./test/apitest -i paging.xq contains the following snippet ProbeIndexRangeValueIterator id=0x1a2fb50 SingletonIterator value=xs:QName(http://www.test.com/,data,idx-age- range- val) id=0x1a146a0/ SingletonIterator value=xs:integer(7) id=0x1a2c830/ SingletonIterator value=xs:integer(2) id=0x1a2c880/ SingletonIterator value=xs:integer(3) id=0x1a2c8d0/ SingletonIterator value=xs:boolean(true) id=0x1a2c920/ SingletonIterator value=xs:boolean(true) id=0x1a2c970/ SingletonIterator value=xs:boolean(true) id=0x1a2c9c0/ SingletonIterator value=xs:boolean(true) id=0x1a2ca10/ /ProbeIndexRangeValueIterator for count(idml:probe-index-range-value-skip(xs:QName(data:idx-age-range- val), 7, 2, 3, true(), true(), true(), true())) Is it ok to create a bug entry for this? Because this has to be done in the collection iterator too. It can be assigned to me. But I would like to get those merge proposals thru. Does it take long to do this. It would be really nice and I'm afraid it will not get done if we create a bug for it. ;-) Ok, it was not that hard. Do you like it? Nevertheless, it also has to be done for collections. - Should a negative skip value be an error? If not, some documentation is missing here. No. Negative skip means no skip. - ChangeLog entry is missing. done -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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:~davidagraf/zorba/paging_index_probe into lp:zorba
Review: Approve Very nicely done feature. Approving... but there are some conflicts. -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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:~davidagraf/zorba/paging_index_probe into lp:zorba
The attempt to merge lp:~davidagraf/zorba/paging_index_probe into lp:zorba failed. Below is the output from the failed tests. CMake Error at /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake:274 (message): Validation queue job paging_index_probe-2012-09-18T16-33-49.228Z is finished. The final status was: 2 tests did not succeed - changes not commited. Error in read script: /home/ceej/zo/testing/zorbatest/tester/TarmacLander.cmake -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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:~davidagraf/zorba/paging_index_probe into lp:zorba
- the probe-index-point-value-skip and probe-index-range-value-skip functions in dml.xq don't declare the $skip parameters (but their documentation does) Fixed. - idml:probe-index-range-value-skip(xs:QName(data:idx-age-range-val), 2, 3, true(), true(), true(), true()) reports 'data:idx-age-range-val: invalid number of arguments to index operation; given 5 expected multiple of 6;' but I'm counting 7 parameters Fixed (was not my fault :-) ). Unfortunately, when trying to write a test for this, I found a bug: https://bugs.launchpad.net/zorba/+bug/1051897 Can you please double check if it is really a bug? - the branch seems to undo some changes in the trunk (see diagnostic_en.xml) - value$1: does not reference a node in collection $2./value + valueNode reference $1 doesn't reference a node in collection $2./value Fixed. - It would be great if the iterator plan could mention if the iterator does skipping and/or counting. For example, ./test/apitest -i paging.xq contains the following snippet ProbeIndexRangeValueIterator id=0x1a2fb50 SingletonIterator value=xs:QName(http://www.test.com/,data,idx-age-range- val) id=0x1a146a0/ SingletonIterator value=xs:integer(7) id=0x1a2c830/ SingletonIterator value=xs:integer(2) id=0x1a2c880/ SingletonIterator value=xs:integer(3) id=0x1a2c8d0/ SingletonIterator value=xs:boolean(true) id=0x1a2c920/ SingletonIterator value=xs:boolean(true) id=0x1a2c970/ SingletonIterator value=xs:boolean(true) id=0x1a2c9c0/ SingletonIterator value=xs:boolean(true) id=0x1a2ca10/ /ProbeIndexRangeValueIterator for count(idml:probe-index-range-value-skip(xs:QName(data:idx-age-range-val), 7, 2, 3, true(), true(), true(), true())) Is it ok to create a bug entry for this? Because this has to be done in the collection iterator too. It can be assigned to me. But I would like to get those merge proposals thru. -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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:~davidagraf/zorba/paging_index_probe into lp:zorba
- the probe-index-point-value-skip and probe-index-range-value-skip functions in dml.xq don't declare the $skip parameters (but their documentation does) Fixed. - idml:probe-index-range-value-skip(xs:QName(data:idx-age-range-val), 2, 3, true(), true(), true(), true()) reports 'data:idx-age-range-val: invalid number of arguments to index operation; given 5 expected multiple of 6;' but I'm counting 7 parameters Fixed (was not my fault :-) ). Unfortunately, when trying to write a test for this, I found a bug: https://bugs.launchpad.net/zorba/+bug/1051897 Can you please double check if it is really a bug? I commented on the bug. - It would be great if the iterator plan could mention if the iterator does skipping and/or counting. For example, ./test/apitest -i paging.xq contains the following snippet ProbeIndexRangeValueIterator id=0x1a2fb50 SingletonIterator value=xs:QName(http://www.test.com/,data,idx-age- range- val) id=0x1a146a0/ SingletonIterator value=xs:integer(7) id=0x1a2c830/ SingletonIterator value=xs:integer(2) id=0x1a2c880/ SingletonIterator value=xs:integer(3) id=0x1a2c8d0/ SingletonIterator value=xs:boolean(true) id=0x1a2c920/ SingletonIterator value=xs:boolean(true) id=0x1a2c970/ SingletonIterator value=xs:boolean(true) id=0x1a2c9c0/ SingletonIterator value=xs:boolean(true) id=0x1a2ca10/ /ProbeIndexRangeValueIterator for count(idml:probe-index-range-value-skip(xs:QName(data:idx-age-range-val), 7, 2, 3, true(), true(), true(), true())) Is it ok to create a bug entry for this? Because this has to be done in the collection iterator too. It can be assigned to me. But I would like to get those merge proposals thru. Does it take long to do this. It would be really nice and I'm afraid it will not get done if we create a bug for it. ;-) - Should a negative skip value be an error? If not, some documentation is missing here. - ChangeLog entry is missing. -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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:~davidagraf/zorba/paging_index_probe into lp:zorba
Problems: - Based on the paging.xqlib module (in the tests), the following query should return the empty sequence: idml:probe-index-range-value-skip(xs:QName(data:idx-age-range-val), 10, 2, 3, true(), true(), true(), true()) Fixed But it returns all the items with age 3. This seems wrong. - idml:probe-index-range-value sorts the nodes implicitly in document order, i.e. they are not returned in the order of the index keys. The skipping is done before the sorting. This might be really confusing to the user. Essentially, he doesn't have a way to know which items are skipped. We have talked to Markos and he agreed that we remove the sorting from the range-value functions and only add it if the probe is used to answer queries. Fixed by you - The module in modules/com/zorba- xquery/www/modules/store/static/indexes/dml.xq doesn't contain the -skip functions. It looks like the probe-index-point-value appears twice and is missing the -skip suffix. The same seems to be the case for the probe-index- range-value function. Also, the documentation for what is probably supposed to be the -skip function is very long. Maybe it can make a reference to the original probe function and only mention the fact that it returns the values for the keys. done Minor issues: - I wasn't able to merge the trunk into this branch because there are tons of merge conflicts in translator.cpp. Also, bzr reports that a criss-cross merge was encountered. Probably because the collection-skipping branch was merged in the meantime. I don't understand why the merge proposal doesn't show any conflict. Without merging the trunk, I can't link because of the uuid_generate problem. done - There seem to be plenty of whitespace-only changes in src/functions/signature.cpp? I couldn't figure out whether there are actually semantic changes in that file. Are they necessary (e.g. removing tabs)? In signature.cpp are not whitespace changes only. There is an additional constructor too. But I do not understand how I can avoid the whitespace changes. If I do a simple change in the file with vi, I get them. -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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:~davidagraf/zorba/paging_index_probe into lp:zorba
Review: Needs Fixing - the probe-index-point-value-skip and probe-index-range-value-skip functions in dml.xq don't declare the $skip parameters (but their documentation does) - idml:probe-index-range-value-skip(xs:QName(data:idx-age-range-val), 2, 3, true(), true(), true(), true()) reports 'data:idx-age-range-val: invalid number of arguments to index operation; given 5 expected multiple of 6;' but I'm counting 7 parameters - the branch seems to undo some changes in the trunk (see diagnostic_en.xml) - value$1: does not reference a node in collection $2./value + valueNode reference $1 doesn't reference a node in collection $2./value - It would be great if the iterator plan could mention if the iterator does skipping and/or counting. For example, ./test/apitest -i paging.xq contains the following snippet ProbeIndexRangeValueIterator id=0x1a2fb50 SingletonIterator value=xs:QName(http://www.test.com/,data,idx-age-range-val) id=0x1a146a0/ SingletonIterator value=xs:integer(7) id=0x1a2c830/ SingletonIterator value=xs:integer(2) id=0x1a2c880/ SingletonIterator value=xs:integer(3) id=0x1a2c8d0/ SingletonIterator value=xs:boolean(true) id=0x1a2c920/ SingletonIterator value=xs:boolean(true) id=0x1a2c970/ SingletonIterator value=xs:boolean(true) id=0x1a2c9c0/ SingletonIterator value=xs:boolean(true) id=0x1a2ca10/ /ProbeIndexRangeValueIterator for count(idml:probe-index-range-value-skip(xs:QName(data:idx-age-range-val), 7, 2, 3, true(), true(), true(), true())) -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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:~davidagraf/zorba/paging_index_probe into lp:zorba
Review: Needs Fixing Problems: - Based on the paging.xqlib module (in the tests), the following query should return the empty sequence: idml:probe-index-range-value-skip(xs:QName(data:idx-age-range-val), 10, 2, 3, true(), true(), true(), true()) But it returns all the items with age 3. This seems wrong. - idml:probe-index-range-value sorts the nodes implicitly in document order, i.e. they are not returned in the order of the index keys. The skipping is done before the sorting. This might be really confusing to the user. Essentially, he doesn't have a way to know which items are skipped. We have talked to Markos and he agreed that we remove the sorting from the range-value functions and only add it if the probe is used to answer queries. - The module in modules/com/zorba-xquery/www/modules/store/static/indexes/dml.xq doesn't contain the -skip functions. It looks like the probe-index-point-value appears twice and is missing the -skip suffix. The same seems to be the case for the probe-index-range-value function. Also, the documentation for what is probably supposed to be the -skip function is very long. Maybe it can make a reference to the original probe function and only mention the fact that it returns the values for the keys. Minor issues: - I wasn't able to merge the trunk into this branch because there are tons of merge conflicts in translator.cpp. Also, bzr reports that a criss-cross merge was encountered. Probably because the collection-skipping branch was merged in the meantime. I don't understand why the merge proposal doesn't show any conflict. Without merging the trunk, I can't link because of the uuid_generate problem. - There seem to be plenty of whitespace-only changes in src/functions/signature.cpp? I couldn't figure out whether there are actually semantic changes in that file. Are they necessary (e.g. removing tabs)? -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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:~davidagraf/zorba/paging_index_probe into lp:zorba
- idml:probe-index-range-value sorts the nodes implicitly in document order, i.e. they are not returned in the order of the index keys. The skipping is done before the sorting. This might be really confusing to the user. Essentially, he doesn't have a way to know which items are skipped. We have talked to Markos and he agreed that we remove the sorting from the range-value functions and only add it if the probe is used to answer queries. I have proposed the removal of the automatic sorting for merge at https://code.launchpad.net/~zorba-coders/zorba/probe-no-doc-order-sort/+merge/124322. -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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:~davidagraf/zorba/paging_index_probe into lp:zorba
Review: Approve -- https://code.launchpad.net/~davidagraf/zorba/paging_index_probe/+merge/122621 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
