Roundup Tracker - Issues

Issue 2551119

classification
Title: Bug in Query-Generator for filter (rdbms backend)
Type: Severity: normal
Components: Database, Test Versions:
process
Status: new Resolution:
Dependencies: Superseder:
Assigned To: schlatterbeck Nosy List: ber, rouilj, schlatterbeck
Priority: Keywords: patch

Created on 2021-03-18 12:13 by schlatterbeck, last changed 2021-03-22 00:56 by rouilj.

Files
File name Uploaded Description Edit Remove
empty_multilink_expr.diff ber, 2021-03-18 17:12
Messages
msg7124 Author: [hidden] (schlatterbeck) Date: 2021-03-18 12:13
There is a feature in the query generator that can combine ID search with boolean expressions AND, OR, NOT, see commit f1fe6fd0aa61. This change came without a regression test and breaks for certain cases. Without a test of the *intended* feature I dare not touch that code. As far as I understand this uses *negative* IDs to implement a prefix or suffix expression parser.

My use-case that breaks:

We have two tables, time_project and time_wp with the following data structures in roundup (excerpt):

time_project

  wps = Multilink('time_wp')
  status = Link('project_status')
  name = String()

time_wp

  bookers = Multilink(user)

Now a user enters a query with a work-package name that doesn't exist in the web-interface. The resulting call to filter has the following arguments:

sort = [('+', 'name')]
filterspec = {'status': ['2', '1'], 'wps.bookers': ['2428'], 'wps': []}

This produces a traceback:
<class 'psycopg2.errors.UndefinedTable'>: missing FROM-clause entry for table "_time_wp3" LINE 1: ...t from _time_wp where _time_wp.__retired__=0) and _time_wp3.... ^

...

  return render(ob, econtext.vars)
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/PageTemplates/Expressions.py", line 99, in render
    ob = ob()
  File "/usr/local/lib/python2.7/dist-packages/roundup/cgi/templating.py", line 3257, in batch
    l = [id for id in klass.filter(matches, filterspec, sort, group)
  File "/usr/local/lib/python2.7/dist-packages/roundup/backends/rdbms_common.py", line 2858, in filter
    sq = self._filter_sql (search_matches, filterspec, sort, group,
  File "/usr/local/lib/python2.7/dist-packages/roundup/backends/rdbms_common.py", line 233, in sql
    cursor.execute(sql, args)
UndefinedTable: missing FROM-clause entry for table "_time_wp3"
LINE 1: ...t from _time_wp where _time_wp.__retired__=0) and _time_wp3....
                                                             ^

It looks like the mentioned buggy code is triggered by the empty wps multilink in the search.
Note how it tries to materialize the whole time_wp table (which contains 43665 entries in our case!) in a subquery.
And it doesn't seem to correctly plug into the query generator: the from clause is missing an entry which is later referenced.

I'd appreciate if someone using this extended query feature can turn on SQL statement logging (either in roundup or in the backend database) and provide me with pairs of filter calls and the generated SQL. Then I'd write an regression test for the intended behaviour.

@Bernhard Reiter: Do you still use roundup with that feature? Examples of usage would be welcome!
msg7125 Author: [hidden] (ber) Date: 2021-03-18 14:59
Hi Ralf,
you seem to use the postgresl backen.

AFAIK we still use this feature (though I don't know how many searches are done with it). We haven't looked at the code for a long while, the idea is to be able to compose complex searches.

What is the fastest way to get a reproducable problem setup?
msg7126 Author: [hidden] (ber) Date: 2021-03-18 17:12
Ralf,
here is an into-the-blue-patch by one of our devs,
more like a guess, but maybe it helps you anyway.
(Sorry for not being more solid here, this certainly
could have been documented better.)

Best,
Bernhard
msg7127 Author: [hidden] (schlatterbeck) Date: 2021-03-18 19:30
Hi Bernhard, thanks for the quick reaction.

On Thu, Mar 18, 2021 at 02:59:46PM +0000, Bernhard Reiter wrote:
> 
> Hi Ralf,
> you seem to use the postgresl backen.

Yes. But I guess the problem is also reproduceable with mysql, the
problem is in the query generator not backend specific.

> AFAIK we still use this feature (though I don't know how many searches
> are done with it). We haven't looked at the code for a long while, the
> idea is to be able to compose complex searches.

Cool.

> What is the fastest way to get a reproducable problem setup?

It would be nice if you could come up with examples of all the usual
queries involving AND, OR, NOT and send me
- The parameters to the filter function for a specific query
- Optionally the generated sql (I can infer this myself).

So this probable involves specifying some common queries via the web
interface and looking what the filter method in backends/rdbms_common.py
is called with. Unfortunately there is no logging for this, so you have
to either instrument filter or set a breakpoint there in the python
debugger.
Accompanying the examples with the expression specified in the web
interface would be nice, so I won't have to reverse-engineer that :-)

The whole idea of all this is to come up with a test that covers
functionality people (your customers among them :-) rely on before
proceeding.

On Thu, Mar 18, 2021 at 05:12:36PM +0000, Bernhard Reiter wrote:
> 
> Ralf,
> here is an into-the-blue-patch by one of our devs,
> more like a guess, but maybe it helps you anyway.
> (Sorry for not being more solid here, this certainly
> could have been documented better.)

Hmm, such a patch without a proper test is just what I want to avoid at
this time :-) Once the behaviour is nailed down it should be easy to
come up with a proper patch. Note that I have other systems where this
misbehaves. I ideally would find out what goes wrong there and come up
with additional tests (which would fail at this time) and proceed from
there. I already have some analyses on what I *think* is wrong. But
first I have to find out what it does when it's right :-)

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
msg7128 Author: [hidden] (ber) Date: 2021-03-19 08:35
Hi Ralf,
just tried to be helpful as fast as I could yesterday. :)

The original idea was to generally be able to have queries, where
e.g. one status or two statuses, or priorities 
are excluded while others are allowed. One of the
standard templates has the javascript code to form the queries.
So you should be able to find test cases as well.

The implementation idea had to find a few to express this via the url,
and thus the notation was invented.

If I have more time, I'll look it up here.

Best,
Bernhard
msg7129 Author: [hidden] (rouilj) Date: 2021-03-22 00:56
Including link to issue2550648 to see all tickets related to this.
There are 3 or 4 opened tickets.
History
Date User Action Args
2021-03-22 00:56:19rouiljsetnosy: + rouilj
messages: + msg7129
2021-03-19 08:35:49bersetmessages: + msg7128
2021-03-18 19:30:33schlatterbecksetmessages: + msg7127
2021-03-18 17:12:36bersetfiles: + empty_multilink_expr.diff
keywords: + patch
messages: + msg7126
2021-03-18 14:59:46bersetmessages: + msg7125
2021-03-18 12:13:36schlatterbeckcreate