need to call notmuch_threads_get (..) to actually move iterator

need to call notmuch_threads_get (..) to actually move iterator

[Gaute Hope]

Hi,

it seems to be necessary to actually call notmuch_threads_get (threads)
to move the thread iterator from a query object, just calling
notmuch_threads_move_to_next (..) is not enough:

```

notmuch_query_t *query;
notmuch_threads_t *threads;
notmuch_thread_t *thread;

query = notmuch_query_create (database, query_string);
threads = notmuch_query_search_threads (query);

int i = 0;

for (;
     notmuch_threads_valid (threads);
     notmuch_threads_move_to_next (threads))
{
    /*
     * with this line commented out the iterator seems to remain in
     * place, and if I below do another loop it will start from the
     * beginning.

    thread = notmuch_threads_get (threads);
    ....
    notmuch_thread_destroy (thread);
    */

    i++;
    if (i > 100) break;
}

for (;
     notmuch_threads_valid (threads);
     notmuch_threads_move_to_next (threads))
{
    /* the thread acquired here will be the first thread in the query.
     * it should be the 101th. */

    thread = notmuch_threads_get (threads);
    ....
    notmuch_thread_destroy (thread);

}


notmuch_query_destroy (query);
```

It is quite slow to skip the threads in this way, might it be faster if
move_to_next works correctly?

Regards, Gaute

Re: need to call notmuch_threads_get (..) to actually move iterator

[Gaute Hope]

Gaute Hope writes on February 24, 2016 13:08:
> Hi,
> 
> it seems to be necessary to actually call notmuch_threads_get (threads)
> to move the thread iterator from a query object, just calling
> notmuch_threads_move_to_next (..) is not enough:

A test-case demonstrating this (for the 'astroid' code-base) is located
here: https://github.com/gauteh/astroid/blob/ti-stateless-query/test/test_notmuch.cc#L66

At the moment, this test depends on the rest of the astroid test-suite,
but it may be useful as a demonstration or skeleton for a notmuch
test-case anyway.

Regards, Gaute

Re: need to call notmuch_threads_get (..) to actually move iterator

[Gaute Hope]

Gaute Hope writes on February 28, 2016 13:36:
> Gaute Hope writes on February 24, 2016 13:08:
>> Hi,
>> 
>> it seems to be necessary to actually call notmuch_threads_get (threads)
>> to move the thread iterator from a query object, just calling
>> notmuch_threads_move_to_next (..) is not enough:
> 
> A test-case demonstrating this (for the 'astroid' code-base) is located
> here: https://github.com/gauteh/astroid/blob/ti-stateless-query/test/test_notmuch.cc#L66
> 
> At the moment, this test depends on the rest of the astroid test-suite,
> but it may be useful as a demonstration or skeleton for a notmuch
> test-case anyway.

Disregard that, my test-case was erroneous. 

-Gaute

Re: need to call notmuch_threads_get (..) to actually move iterator

[Franz Fellner]

It might be I found the issue:
One big thing notmuch_threads_get does is remove the thread_id from the
match_set. Playing with astroid (branch ti-skip-and-load) i see that it
is not entirely true that the iterator doesn't move. It just seems to
duplicate some messages. There definitely are coming new messages.
So my explanation is that match_set contains duplicate thread_ids. If
that is true a solution to Gautes problem might be to deduplicate the
match_set.

Excerpts from Gaute Hope's message of Februar 24, 2016 1:08 :
> Hi,
> 
> it seems to be necessary to actually call notmuch_threads_get (threads)
> to move the thread iterator from a query object, just calling
> notmuch_threads_move_to_next (..) is not enough:
> 
> ```
> 
> notmuch_query_t *query;
> notmuch_threads_t *threads;
> notmuch_thread_t *thread;
> 
> query = notmuch_query_create (database, query_string);
> threads = notmuch_query_search_threads (query);
> 
> int i = 0;
> 
> for (;
>      notmuch_threads_valid (threads);
>      notmuch_threads_move_to_next (threads))
> {
>     /*
>      * with this line commented out the iterator seems to remain in
>      * place, and if I below do another loop it will start from the
>      * beginning.
> 
>     thread = notmuch_threads_get (threads);
>     ....
>     notmuch_thread_destroy (thread);
>     */
> 
>     i++;
>     if (i > 100) break;
> }
> 
> for (;
>      notmuch_threads_valid (threads);
>      notmuch_threads_move_to_next (threads))
> {
>     /* the thread acquired here will be the first thread in the query.
>      * it should be the 101th. */
> 
>     thread = notmuch_threads_get (threads);
>     ....
>     notmuch_thread_destroy (thread);
> 
> }
> 
> 
> notmuch_query_destroy (query);
> ```
> 
> It is quite slow to skip the threads in this way, might it be faster if
> move_to_next works correctly?
> 
> Regards, Gaute
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
> 

Re: need to call notmuch_threads_get (..) to actually move iterator

[Franz Fellner]

Really might be the issue: Threads with many messages duplicate more
often than messages with few messages. Threads with only one message
never seem to come up more than twice.
Currently I don't have the time to track down what actually get's stored
in match_set, would be nice if a dev could shed some light on it. Thx!

Excerpts from Franz Fellner's message of Februar 28, 2016 3:26 :
> It might be I found the issue:
> One big thing notmuch_threads_get does is remove the thread_id from the
> match_set. Playing with astroid (branch ti-skip-and-load) i see that it
> is not entirely true that the iterator doesn't move. It just seems to
> duplicate some messages. There definitely are coming new messages.
> So my explanation is that match_set contains duplicate thread_ids. If
> that is true a solution to Gautes problem might be to deduplicate the
> match_set.
> 
> Excerpts from Gaute Hope's message of Februar 24, 2016 1:08 :
>> Hi,
>> 
>> it seems to be necessary to actually call notmuch_threads_get (threads)
>> to move the thread iterator from a query object, just calling
>> notmuch_threads_move_to_next (..) is not enough:
>> 
>> ```
>> 
>> notmuch_query_t *query;
>> notmuch_threads_t *threads;
>> notmuch_thread_t *thread;
>> 
>> query = notmuch_query_create (database, query_string);
>> threads = notmuch_query_search_threads (query);
>> 
>> int i = 0;
>> 
>> for (;
>>      notmuch_threads_valid (threads);
>>      notmuch_threads_move_to_next (threads))
>> {
>>     /*
>>      * with this line commented out the iterator seems to remain in
>>      * place, and if I below do another loop it will start from the
>>      * beginning.
>> 
>>     thread = notmuch_threads_get (threads);
>>     ....
>>     notmuch_thread_destroy (thread);
>>     */
>> 
>>     i++;
>>     if (i > 100) break;
>> }
>> 
>> for (;
>>      notmuch_threads_valid (threads);
>>      notmuch_threads_move_to_next (threads))
>> {
>>     /* the thread acquired here will be the first thread in the query.
>>      * it should be the 101th. */
>> 
>>     thread = notmuch_threads_get (threads);
>>     ....
>>     notmuch_thread_destroy (thread);
>> 
>> }
>> 
>> 
>> notmuch_query_destroy (query);
>> ```
>> 
>> It is quite slow to skip the threads in this way, might it be faster if
>> move_to_next works correctly?
>> 
>> Regards, Gaute
>> 
>> _______________________________________________
>> notmuch mailing list
>> notmuch@notmuchmail.org
>> https://notmuchmail.org/mailman/listinfo/notmuch
>> 
> 

Re: need to call notmuch_threads_get (..) to actually move iterator

[David Bremner]

Gaute Hope <eg@gaute.vetsj.com> writes:

> Hi,
>
> it seems to be necessary to actually call notmuch_threads_get (threads)
> to move the thread iterator from a query object, just calling
> notmuch_threads_move_to_next (..) is not enough:
>

I'm not sure of all the details, but it is true that the
notmuch_threads_get has side effects in the database. In particular it
allocates new thread-ids. So I can easily imagine things failing if you
leave out the call.

d