In message <20241202072032.lmfgw7ffzd6mdmqk@runtux.com>,
Ralf Schlatterbeck writes:
>Ralf Schlatterbeck added the comment:
>
>Thanks for updating the docs, see my comment inline.
>And, yes, I think this documents the feature comprehensively and we can
>close the issue!

We still need tests right? (From discussion below, the answer is yes.)

>On Mon, Dec 02, 2024 at 03:30:54AM +0000, John Rouillard wrote:
>[...]
>> Another example is: ``creator=3,-2,1,-2,-3``. This is the same as the
>> expression: ``(not user3) and (not user1)``. Using the rules of logic,
>> this is the same as: ``not (user3 or user1)`` which is expressed in
>> RPN as ``creator=3,1,-4,-2``. Compare this to ``creator=3,1,-2`` which
>> returns issues created by user3 or any user other than user1.
>
>Have actually tried the last example? I'm not sure the implicit 'OR'
>applies when other expressions have been matched. So it may well be that
>creator=3,1,-2 is a syntax error (and would return *all* issues) and you
>really need the final 'OR' like in creator=3,1,-2,-4

I did test it. Both link (using status) and multilink (using keyword)
cases. I just retested manually in case I screwed up the URL
editing. It returns what I said, all entries without user1. The 3 is
effectively a no-op since '1,-2' includes 3 anyway.

Looking at mlink_expr.py:compile_expression, I'll bet '3' is just left
on the stack and "1,-2" is popped/returned rather than a syntax
error. compile_expression probably should check stack depth before
returning and if it's not 1, raise an exception.

Also since "keyword=-2,1,8,-3,-2,-3" doesn't throw an error, I suspect
your intuition is right. (However it acts like the expression was
"keyword=-1,-2" not sure if that's intentional.)

It looks like rdbms_common.py:_filter_multilink_expression has a bare
except statement.

        except:
            # fallback behavior when expression parsing above fails
            orclause = ''

When IndexError is raised by popping a non-existent operand to the
leading '-2' operator, the error is caught here and suppressed. It
looks like this except statement also handles the case where there are
no operators as it seems to be building a default 'or' expression.

I wonder if wrapping the opcodes loop in
mlink_expr.py::compile_expression in a try/except IndexError and
raising a new ExpressionError exception that propigates through
rdbms_common.py:_filter_multilink_expression's bare except would work?

Changing the loop from:

 for opcode in opcodes:

to

 for pos, opcode in enumerate(opcodes):

also allows us to report where in the expression the error happened.

Then changing _filter_multilink_expression to:

   except ExpressionError as e:
          raise e with some additional annotation
                  (e.g. we need the property name 'keyword', 'status' ....)
   except:
    ....

could propagate the error up the stack to the user.

This fix would need to be in rdbms and anydbm back ends.
Also it needs a new issue.

Also a more interesting example could be:

  keyword=-1,-2,1,8,-3,-2,-3

which returns issues that have keyword set and the issue does not have
keyword1 and keyword8 both set. In more standard infix form:

  not empty and not (keyword1 and keyword8)

I'll add this example to the doc.