Bug Hunting Session
Bug 96370 - BASE: filter doesn't work for query fields which are sum
Summary: BASE: filter doesn't work for query fields which are sum
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
5.0.3.2 release
Hardware: All All
: medium normal
Assignee: Lionel Elie Mamane
URL:
Whiteboard: target:6.0.0
Keywords:
Depends on:
Blocks: Base-UX
  Show dependency treegraph
 
Reported: 2015-12-09 20:30 UTC by senya
Modified: 2017-07-31 16:40 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
testcase file (11.91 KB, application/vnd.oasis.opendocument.database)
2015-12-09 20:30 UTC, senya
Details
bt with debug symbols (6.21 KB, text/plain)
2017-07-27 12:13 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description senya 2015-12-09 20:30:09 UTC
Created attachment 121180 [details]
testcase file

Given
A database with a table(id, text, date, int fields of corresponding types) and a query(table.date and SUM(table.int)). A form for the query with a table control element.

Steps to reproduce:
1) Activate filter setting (ctrl-shift-l)
2) type some value for the SUM(table.int) field
3) Apply filter

Expected:
Filter applied, we see filtered data.

We got:
No data in the table. Filter panel is blocked, we can't even clear filter settings, since the button is blocked. The only way out - reopening the form.
Comment 1 senya 2015-12-09 20:30:45 UTC
OS Ubuntu 14.04 64 bit, LibreOffice installed from ppa:libreoffice
Comment 2 Alex Thurgood 2015-12-10 10:23:00 UTC
@senya : so this bug is against a PPA provided version of LibreOffice and not the TDF version ? Have you tried with a TDF download ?
Comment 3 Alex Thurgood 2015-12-10 10:31:36 UTC
Confirming on

Version: 5.0.3.2
Build ID: e5f16313668ac592c1bfb310f4390624e3dbfb75
Locale : fr-FR (fr.UTF-8)

OSX 10.11.1
Comment 4 Robert Großkopf 2015-12-10 16:57:39 UTC
Filter creates something like this:
SELECT "date", SUM( "int" ) "total" FROM "table" WHERE SUM( "int" ) = 44 GROUP BY "date"
This query won't work in internal HSQLDB.
Workaround: Switch from the query to a view. The GUI will create a query, which doesn't notice anything about sum("int") or GROUP BY "date".

Don't know why it works different when filter is set in the query directly. But this filter has also problems and could only be resetted by deleting the filter. But the behavior in queries is better than showing nothing in a form.

The buggy behavior is the same in every LO-version I have installed here (SUSE 42.1, 64bit rpm Linux, LO 3.6, 4.1 ...)
Comment 5 Lionel Elie Mamane 2015-12-10 17:12:26 UTC
(In reply to robert from comment #4)
> Filter creates something like this:
> SELECT "date", SUM( "int" ) "total" FROM "table" WHERE SUM( "int" ) = 44
> GROUP BY "date"
> This query won't work in internal HSQLDB.

Should be "HAVING  SUM( "int" ) = 44" instead of WHERE.
Comment 6 Julien Nabet 2015-12-10 20:49:42 UTC
I don't understand the steps to reproduce this.
For step 1, what do you use? Table, Query, Form? Edit it or just open it?

On pc Debian x86-64 with master sources updated today, I tried option "open form", then Ctrl-Shift-L
=> a window appears with just a hierarchy:
"
form
  Or
"
Nothing else

I double click 2 times in "Or", crash!
Comment 7 Julien Nabet 2015-12-10 20:50:26 UTC
I noticed these logs:
warn:legacy.osl:1552:1:svx/source/fmcomp/gridcell.cxx:2857: DbListBox::updateFromModel: not implemented yet (how the hell did you reach this?)!
warn:legacy.osl:1552:1:svx/source/fmcomp/gridcell.cxx:2857: DbListBox::updateFromModel: not implemented yet (how the hell did you reach this?)!
warn:legacy.osl:1552:1:unotools/source/config/moduleoptions.cxx:524: unknown factory

(soffice:1552): Gdk-WARNING **: gdk_window_set_icon_list: icons too large
warn:legacy.osl:1552:1:svl/source/items/poolitem.cxx:255: There is no implementation for QueryValue for this item!
warn:legacy.tools:1552:1:svx/source/form/filtnav.cxx:1184: FmFilterNavigator::EditedEntry() wrong entry
Comment 8 Robert Großkopf 2015-12-11 14:12:09 UTC
Steps in detail:
1. Open the attached database
2. open the form
3. click into one field of "total" - for example '44'
4. click on AutoFilter in the navigationbar.
→ Navigationbar will be greyed out - no way to get out except closing the form.

Now do another thing:
2.a after opening the form click "Datasource as Table"
3. click into one field of "total" of the table - for example '44'
4. click on AutoFilter in the tables navigationbar.
→ Works, but couldn't set back by switching filter on and off - another bug

How to get the command, which creates the crash:
2.a after opening the form click "Datasource as Table"
3. click into one field of "total" of the form - for example '44'
4. click on AutoFilter in the navigationbar.
→ Error appears: Not a condition in statement [SELECT "date", SUM( "int" ) "total" FROM "table" WHERE SUM( "int" ) = 44 GROUP BY "date"]

It will work right, if the code would be:
SELECT "date", SUM( "int" ) "total" FROM "table" GROUP BY "date" HAVING SUM( "int" ) = 44

... but this code wont be created.
Comment 9 QA Administrators 2017-01-03 19:48:11 UTC Comment hidden (obsolete)
Comment 10 Robert Großkopf 2017-01-04 07:48:20 UTC
Bug still exists in
Version: 5.2.4.2
Build-ID: 3d5603e1122f0f102b62521720ab13a38a4e0eb0
CPU-Threads: 4; BS-Version: Linux 4.1; UI-Render: Standard; VCL: kde4; 
Gebietsschema: de-DE (de_DE.UTF-8); Calc: group
Comment 11 Julien Nabet 2017-07-25 07:58:31 UTC
On pc Debian x86-64 with master sources updated yesterday, I could reproduce this.

Just for information, it works if I do this:
diff --git a/dbaccess/source/core/api/RowSet.cxx b/dbaccess/source/core/api/RowSet.cxx
index 68da787a81dc..43fbd979235c 100644
--- a/dbaccess/source/core/api/RowSet.cxx
+++ b/dbaccess/source/core/api/RowSet.cxx
@@ -2320,9 +2320,8 @@ void ORowSet::impl_initComposer_throw( OUString& _out_rCommandToExecute )
 
     m_xComposer->setCommand( m_aCommand,m_nCommandType );
     m_aActiveCommand = m_xComposer->getQuery();
-
-    m_xComposer->setFilter( m_bApplyFilter ? m_aFilter : OUString() );
-    m_xComposer->setHavingClause( m_bApplyFilter ? m_aHavingClause : OUString() );
+    // m_xComposer->setFilter( m_bApplyFilter ? m_aFilter : OUString() );
+    m_xComposer->setHavingClause( m_bApplyFilter ? m_aFilter : OUString() );
 
     if ( m_bIgnoreResult )
     {   // append a "0=1" filter

Of course, it's just for the test, not a fix.
Comment 12 Lionel Elie Mamane 2017-07-25 12:38:45 UTC
> Just for information, it works if I do this:
> diff --git a/dbaccess/source/core/api/RowSet.cxx
> b/dbaccess/source/core/api/RowSet.cxx
> index 68da787a81dc..43fbd979235c 100644
> --- a/dbaccess/source/core/api/RowSet.cxx
> +++ b/dbaccess/source/core/api/RowSet.cxx
> @@ -2320,9 +2320,8 @@ void ORowSet::impl_initComposer_throw( OUString&
> _out_rCommandToExecute )
>  
>      m_xComposer->setCommand( m_aCommand,m_nCommandType );
>      m_aActiveCommand = m_xComposer->getQuery();
> -
> -    m_xComposer->setFilter( m_bApplyFilter ? m_aFilter : OUString() );
> -    m_xComposer->setHavingClause( m_bApplyFilter ? m_aHavingClause :
> OUString() );
> +    // m_xComposer->setFilter( m_bApplyFilter ? m_aFilter : OUString() );
> +    m_xComposer->setHavingClause( m_bApplyFilter ? m_aFilter : OUString() );

Yes, the problem is that
 SUM( "int" ) = 44
ends up in m_aFilter, but should go into m_aHavingClause. If you find the bit of code that puts it in m_aFilter, it is a great start to fixing it.
Comment 13 Julien Nabet 2017-07-27 12:13:28 UTC
Created attachment 134898 [details]
bt with debug symbols

First try to rewind where "SUM( \"int\" ) = 44" comes from.

I must keep on rewinding with m_xParser (uno::Reference to (dbaccess::OSingleSelectQueryComposer *)
Comment 14 Julien Nabet 2017-07-27 12:18:18 UTC
Oups, forget last line of my previous comment, no need to keep on unwinding, "SUM( \"int\" ) = 44" is ok.
The problem is, it shoudn't be used in a Where but in Having clause here.
Comment 15 Julien Nabet 2017-07-27 16:53:32 UTC
I'm stuck now.

Indeed, I see lots of PROPERTY_FILTER_ID/PROPERTY_FILTER/PROPERTY_APPLYFILTER, but few PROPERTY_HAVING_CLAUSE but I really don't see where PROPERTY_HAVING_CLAUSE should be added.

I don't know if it's related but I compared "getFilter" and "getHavingClause" occurences in Opengrok. The last one is nowhere to be seen in dbaccess/source/core/inc/querycomposer.hxx and in whole subdir forms/ too.
Comment 16 Lionel Elie Mamane 2017-07-28 05:21:33 UTC
Looks like:

 * frame 16 (frm::FormOperations::impl_executeAutoFilter_throw)
   should use _both_ filter (set property "Filter" from
   m_xParser->getFilter()) *and* having clause (set property HavingClause
   from m_xParser->getHavingClause()

 * this particular bit should be in m_xParser->getHavingClause(),
   not m_xParser->getFilter()

I'll try to take a look at it this week-end
Comment 17 Lionel Elie Mamane 2017-07-30 08:08:51 UTC
interestingly and surprisingly, in the query view (I mean when one *opens* the query, not edit it, and sees the results and see a ... table control!) the autofilter works correctly (it makes a HAVING clause)
Comment 18 Lionel Elie Mamane 2017-07-30 17:30:09 UTC
A patch is at https://gerrit.libreoffice.org/#/c/40567
I'll push it when I find how to silence spurious compiler warnings (which are treated as errors by our policy)
Comment 19 Commit Notification 2017-07-30 18:24:35 UTC
Lionel Elie Mamane committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2b1d6f0d3b0b025148c81986ba7f109659d838af

tdf#96370 rework filtering to be aware of WHERE vs HAVING clause

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 20 Julien Nabet 2017-07-31 16:40:45 UTC
With master sources updated today, I confirm it's ok now.
Thank you Lionel! :-)