[AdvancedQueryParser] Move semantic handling of tokens to SearchStore
ClosedPublic

Authored by bruns on Mar 29 2020, 11:13 AM.

Details

Summary

The AdvancedQueryParser has no knowledge about the semantics of a
value, while the searchstore can use KFM::PropertyInfo to determine
if the value should be converted to e.g. an integer or a QDateTime.

This allows to e.g. use CreationTime in queries.

Test Plan

baloosearch 'creationtime>=2010-01-20'
baloosearch 'creationDate>="1997-01-01 10:00:00"' \

AND 'creationDate<=2000-01-01'

Diff Detail

Repository
R293 Baloo
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bruns created this revision.Mar 29 2020, 11:13 AM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptMar 29 2020, 11:13 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
bruns requested review of this revision.Mar 29 2020, 11:13 AM
ngraham accepted this revision.Apr 7 2020, 3:08 PM
This revision is now accepted and ready to land.Apr 7 2020, 3:08 PM
This revision was automatically updated to reflect the committed changes.
kossebau added inline comments.
src/lib/searchstore.cpp
82

This might need an explicit #include <array> on some systems, not exactly sure by the error message.

At least with FreeBSD & Windows KDE CI fails over this: https://build.kde.org/view/Failing/job/Frameworks/job/baloo/

The following is notice that the following reviews/commits are being scheduled to be reverted in 24 hours due to the FTBFS on Windows and FreeBSD:

bruns added a comment.Apr 7 2020, 7:05 PM

The following is notice that the following reviews/commits are being scheduled to be reverted in 24 hours due to the FTBFS on Windows and FreeBSD:

This is complete nonsense. It is the responsibility of the Windows and FreeBSD teams to keep their builds working. If Windows/FreeBSD keep failing, *their builds* should be scheduled for removal.

I have no WIndows license, I have no Visual Studio license, I don't care for WIndows, and I won't invest any time in it.

bruns added inline comments.Apr 7 2020, 7:10 PM
src/lib/searchstore.cpp
82

Thanks for the heads-up.

As you have noticed, the message is vague, so someone with access to one of the affected systems should test it and submit a review.

Sorry, but that isn't how this works. Also, you will notice that one of the failing platforms is FreeBSD. Which is freely available and OSS.

The responsibility of people to keep code compiling rests with those working on it. Should there be platform specific issues they may from time to time get assistance from those who look after those platforms, but in general the responsibility lies with the person working on the code.

The FreeBSD failure is most likely due to Clang being more strict with C++ than GCC is.

As for Windows, that looks to be a namespacing issue.

vkrause added a subscriber: vkrause.Apr 7 2020, 8:00 PM
vkrause added inline comments.
src/lib/searchstore.cpp
82

I don't have either of those here to test it for sure, but I have fixed similar errors elsewhere recently for those platforms by just adding the <array> include, so I think @kossebau is right.

bruns added a comment.Apr 7 2020, 8:00 PM

Sorry, but that isn't how this works. Also, you will notice that one of the failing platforms is FreeBSD. Which is freely available and OSS.

That does not change the fact it requires extra effort to install it and keep it up to date.

The responsibility of people to keep code compiling rests with those working on it. Should there be platform specific issues they may from time to time get assistance from those who look after those platforms, but in general the responsibility lies with the person working on the code.

Thats your opinion. Mine differs.

The FreeBSD failure is most likely due to Clang being more strict with C++ than GCC is.

"most likely". So maybe, maybe not...

As for Windows, that looks to be a namespacing issue.

More guesswork ...