Issue 2551119
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:19 | rouilj | set | nosy:
+ rouilj messages: + msg7129 |
2021-03-19 08:35:49 | ber | set | messages: + msg7128 |
2021-03-18 19:30:33 | schlatterbeck | set | messages: + msg7127 |
2021-03-18 17:12:36 | ber | set | files:
+ empty_multilink_expr.diff keywords: + patch messages: + msg7126 |
2021-03-18 14:59:46 | ber | set | messages: + msg7125 |
2021-03-18 12:13:36 | schlatterbeck | create |