Roundup Tracker - Issues

Issue 2550698

classification
Document the logical expression search a bit
Type: Severity: normal
Components: Documentation Versions: devel
process
Status: fixed fixed
: Add error handling for filter expressions
View: 2551374
: rouilj : ber, rouilj, schlatterbeck
Priority: normal : Effort-Medium

Created on 2011-04-15 07:33 by ber, last changed 2024-12-02 18:56 by rouilj.

Messages
msg4287 Author: [hidden] (ber) Date: 2011-04-15 07:33
Split out from Issue2550648 (Should be possible to search for matching 
several keywords (keyword1 AND keyword2)):

| BTW: I think the new features deserve some entry in doc/upgrading.txt
| for existing tracker installations...
msg4291 Author: [hidden] (schlatterbeck) Date: 2011-04-15 08:21
When we're at it: It would be nice to have a small regression test for
this, too. I've recently re-factored the sql generation (for
implementing filter_iter) and was unsure if I had broken some of the new
features.

If you can give me a search-URL that uses (if possible *all*) of the new
search features I'll volunteer to put this into (one or several) tests.

original issue2550648 (lowercase so that this generates a link :-)
msg4292 Author: [hidden] (ber) Date: 2011-04-15 09:08
Am Freitag, 15. April 2011 10:21:27 schrieb Ralf Schlatterbeck:
> If you can give me a search-URL that uses (if possible *all*) of the new
> search features

Not right now. Just use the javascript on a new tracker to get one.
Thanks for helping!
msg4295 Author: [hidden] (ber) Date: 2011-04-15 15:31
@schlatterbeck:
Examples, hope this helps. :)
Basics:
1) first AND second keyword:
  /demo/issue?%40action=search&keyword=1,2,-3
2) NOT first keyword
  /demo/issue?%40action=search&keyword=1,-2
3) second OR third keyword:
  /demo/issue?%40action=search&keyword=2,3,-4
  should be equivalent to the old implicit OR behaviour
  /demo/issue?%40action=search&keyword=2,3

More involved:
4) testing together: ( first AND NOT third ) OR fourth keywords
  /demo/issue?%40action=search&keyword=1,3,-2,-3,4,-4
msg5829 Author: [hidden] (rouilj) Date: 2016-07-11 01:38
Ralf,

Did you ever get a chance to generate these tests. I took a quick
look though tests/test_* and I don't see any tests using -2, -3 or-4
in a filter context.
msg5841 Author: [hidden] (schlatterbeck) Date: 2016-07-12 08:44
On Mon, Jul 11, 2016 at 01:38:57AM +0000, John Rouillard wrote:
> 
> Ralf,
> 
> Did you ever get a chance to generate these tests. I took a quick
> look though tests/test_* and I don't see any tests using -2, -3 or-4
> in a filter context.

No, but I see Bernhard has provided some examples. This seems to use a
simple polish notation with the operators being negative numbers.

Maybe I get around doing that today...
I have to do some modifications on a tracker I'm running...

Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   http://www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg7527 Author: [hidden] (rouilj) Date: 2022-05-15 17:26
Hi Ralf:

I see there were updates to the hyperdb filter method documenting the RPN search method.
What else needs to be done to close this?

  additions to the docs under doc (did a fast search for RPN and found nothing)

  tests for the filters using -2 -3 -4 (maybe include the exampels from filter()'s
    docstring?)
msg8201 Author: [hidden] (rouilj) Date: 2024-12-02 03:30
Ralf I added the following to user_guide.txt. I tested the examples using my tracker
with link (for examples that make sense) and multilink properties.

Bern, is there anything you think should be added here?

Do both of you think this looks good enough to close the documentation requirement
for this issue?

We still need a populated db and tests for this codepath.

Advanced Searching with Property Expressions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

You can create more advanced searches in your tracker by using Reverse
Polish Notation (`RPN`_) to make property expressions. By default, when
filtering by multiple items, the expression type is 'or'. For
instance, if you filter the property assignedto by adding the query
string element "assignedto=2,3,40", it matches users "2 or 3 or
40". In RPN, this would be written as "2, 3, or, 4, or". Roundup uses
negative numbers to represent operators. For example, using "-1" for a
single value (e.g. the assignedto Link property, but not the keyword
multivalued/MultiLink property) matches an issue where the property is
'not set'.

The operators and their corresponding numbers are:

* 'not' is represented by -2
* 'and' is represented by -3
* 'or' is represented by -4

So, "assignedto=2,3,40" is the same as
"assignedto=2,3,-4,40,-4". While this example is the same as
"2,3,40", the expression "keyword=1,2,-3,-2" filters issues that don't
have both keywords 1 and 2.

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.

Typing these expressions manually can be tiresome, so there's an
expression editor on the search page. You can access it by clicking on
the ``(expr)`` link, which makes creating these expressions a bit
easier.
msg8202 Author: [hidden] (schlatterbeck) Date: 2024-12-02 07:20
Thanks for updating the docs, see my comment inline.
And, yes, I think this documents the feature comprehensively and we can
close the issue!

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

Thanks!
Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg8203 Author: [hidden] (rouilj) Date: 2024-12-02 17:15
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.
msg8204 Author: [hidden] (schlatterbeck) Date: 2024-12-02 17:29
On Mon, Dec 02, 2024 at 05:15:20PM +0000, John Rouillard wrote:
> We still need tests right? (From discussion below, the answer is yes.)

I *did* make a lot of tests for this feature quite some time ago.
But, yes, we probably need to at least make sure that the documented
behavior is tested.
In particular it doesn't have many tests with malformed expressions.

> >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.

Yes that was my hunch here, too, that when in 'expression mode' it would
not do the implicit OR.
Checking the stack depth is a good idea.

> 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.)

That would be an operator on an empty stack... we probably should check
that, too. (Ah it is caught by the bare except statement)

> 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.

Ah yes, I remember.

> 
> 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?
...
> This fix would need to be in rdbms and anydbm back ends.
> Also it needs a new issue.

Yes, I also think fixing the expression parsing deserves its own
issue... Or we just document that an expression error is silently
ignored and returns the OR of all valid issue ids.
I'm not sure the HTML for the expressions can only return valid
expressions, I've never used that.

> 
> Also a more interesting example could be:
> 
>   keyword=-1,-2,1,8,-3,-2,-3

Nice!

Thanks
Ralf
-- 
Dr. Ralf Schlatterbeck                  Tel:   +43/2243/26465-16
Open Source Consulting                  www:   www.runtux.com
Reichergasse 131, A-3411 Weidling       email: office@runtux.com
msg8206 Author: [hidden] (rouilj) Date: 2024-12-02 18:56
Opened issue 2551374 to add better error reporting for expressions.

Added statement that errors are silently dropped to docs and that this is a bug.

Added a disabled test with invalid expressions to use in future for testing this case.
History
Date User Action Args
2024-12-02 18:56:34rouiljsetstatus: open -> fixed
superseder: Add error handling for filter expressions
resolution: fixed
messages: + msg8206
2024-12-02 17:29:10schlatterbecksetmessages: + msg8204
2024-12-02 17:15:20rouiljsetmessages: + msg8203
2024-12-02 07:20:42schlatterbecksetmessages: + msg8202
2024-12-02 03:30:54rouiljsetstatus: new -> open
assignee: rouilj
messages: + msg8201
2022-05-15 17:26:53rouiljsetmessages: + msg7527
2016-07-12 08:44:20schlatterbecksetmessages: + msg5841
2016-07-11 01:38:57rouiljsetkeywords: + Effort-Medium
nosy: + rouilj
messages: + msg5829
2011-04-15 15:31:46bersetmessages: + msg4295
2011-04-15 09:08:05bersetmessages: + msg4292
2011-04-15 08:21:27schlatterbecksetnosy: + schlatterbeck
messages: + msg4291
2011-04-15 07:33:23bercreate