Add method to take clauses out of boolean query

[umweltinfo]

This helps with rewriting, for example to replace the default score combiner used by boolean queries, insofer it avoids the need to box_clone the sub queries.

For us, this replaces #2600 insofar we will keep carrying a CustomBooleanQuery to overwrite the default score combiner.

[fulmicoton]

can we make that an Into<Vec<(Occur, Box<dyn Query>)>> implementation?

[umweltinfo]

Certainly. impl Iterator<Item=(Occur, Box<dyn Query>) might be even more future-proof though?

[umweltinfo]

Add such a method and also generalized the return type of the existing method.

[umweltinfo]

Sorry for the formatting SNAFU. It said something about nightly and then reformatted everything when I tried cargo fmt, so I tried to adjust things manually to match the CI job.

[fulmicoton]

Add such a method and also generalized the return type of the existing method.

This is not doing what I asked for though.
Why

pub fn take_clauses(&mut self) -> impl Iterator<Item = (Occur, Box<dyn Query>)> {

And not

impl Into<Vec<..>> 

if the argument is about not making it Vec<> why fn take_clauses(&mut self)?
Why not fn into_clauses(self)?

[umweltinfo]

This is not doing what I asked for though.

So should I revert this part then?

And not impl Into<Vec<..>>

I understood that you wanted to isolate the signature from the internal representation as a Vec. Into<Vec> is a limited form of isolated as it requires a materialized data structure to exist, hence the choice of an iterator which could be built on-the-fly if it ever becomes necessary.

For me personally, -> impl Into<Vec<..>> does certainly suffice and I can change if you prefer that?

Why not fn into_clauses(self)?

We use to fix up queries after the query parser has build them up, i.e. we have boxed trait objects and need to downcast to access the clauses. Hence, I don't think I can move out of this trait objects. Even if I can, I would have allocate a placeholder query object to take its place while I create a new query. With the take(&mut self) approach, the emptied query basically is that placeholder and is already in place.

For completeness, the code I would like to replace using this is:

if let Some(boolean_query) = query.downcast_ref::<BooleanQuery>() {
    let clauses = boolean_query
        .clauses()
        .iter()
        .map(|(occur, clause)| (*occur, clause.box_clone()))
        .collect::<Vec<_>>();

    query = Box::new(CustomBooleanQuery::new(clauses));
}

which should become something like

if let Some(boolean_query) = query.downcast_mut::<BooleanQuery>() {
    let clauses = boolean_query
        .take_clauses()
        .collect::<Vec<_>>();

    query = Box::new(CustomBooleanQuery::new(clauses));
}

(AFAIK, some_vec.into_iter().collect::<Vec<_>>() will not actually allocate as the Vec is reused, so impl Iterator does cost more than impl Into.)