Fix handling of node numbers in queries with multiple alternatives

[matthias-stemmler]

I stumbled upon a bug in the way node numbers are counted in queries with multiple alternatives: In some cases indices local to the current conjunction are compared against global indices. I first noticed it when an unbound variable occurs in an alternative other than the first one. When searching through the module, I found two more instances of this issue.

For the fix, I subtract self.var_idx_offset to convert global to local indices. I turned get_cost_estimates and should_switch_operand_order into methods so they have access to this offset.

[thomaskrause]

Thanks for reporting and fixing this! We had quite a lot of fixes in the handling of the nodes in alternatives, because we keep hitting edges cases. When reviewing this PR, I will also check the existing mechanism for this because it might be exceptionally problematic and I want to check if the mental model of it is sufficient.

Thus it will probable take some time to review it (semester time is quite busy with teaching and meetings right now) but not because of the changes but because of the context. I will keep you updated about the progress.

[thomaskrause]

Looks good to me. I have added some additional comments to existing functions/data structures that should give a better context how the different indices work,

[thomaskrause]

I think I figured out a root cause for the confusion, which is the mix between the "global" indices for nodes in the query and the ones inside a single conjunction. The mechanism to address the nodes by their variable name was added later. E.g. in

#[derive(Debug)]
pub struct BinaryOperatorArguments {
    /// *Global* position int the query of the LHS.
    /// This references not the position in the conjunction, but the position
    /// over all alternatives.
    pub left: usize,
    /// *Global* position int the query of the RHS.
    /// This references not the position in the conjunction, but the position
    /// over all alternatives.
    pub right: usize,
    pub global_reflexivity: bool,
}

it should be possible to refactor this to use the variable name for left and right and use the node indices only for conjunctions internally. This is something I should do in a refactoring/cleanup branch later on to stabilize this part of the code.

Thanks again for the bugfix!