Bug 9819 - stopwords related code should be removed
Summary: stopwords related code should be removed
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
: 11110 (view as bug list)
Depends on: 7440
Blocks:
  Show dependency treegraph
 
Reported: 2013-03-14 18:35 UTC by Tomás Cohen Arazi
Modified: 2017-06-14 22:02 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 9819 - 'stopwords'-related code removed (68.66 KB, patch)
2013-04-10 14:34 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 9819 - Remove 'stopwords' table from kohastructure.sql (1.22 KB, patch)
2013-04-10 14:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table (1.22 KB, patch)
2013-04-10 14:35 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 9819 - 'stopwords'-related code removed (68.71 KB, patch)
2013-04-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9819 - Remove 'stopwords' table from kohastructure.sql (1.29 KB, patch)
2013-04-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table (1.29 KB, patch)
2013-04-19 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9819 - QA Followup: Stopwords vestiges in Pg and es-ES removed (2.75 KB, patch)
2013-04-22 12:11 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 9819 - 'stopwords'-related code removed (64.20 KB, patch)
2015-12-15 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9819 - Remove 'stopwords' table from kohastructure.sql (1.32 KB, patch)
2015-12-15 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table (1.42 KB, patch)
2015-12-15 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9819 - QA Followup: Stopwords vestiges in es-ES removed (2.17 KB, patch)
2015-12-15 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9819: Make the tests pass (1.82 KB, patch)
2015-12-15 10:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 9819 - 'stopwords'-related code removed (64.25 KB, patch)
2015-12-17 23:59 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9819 - Remove 'stopwords' table from kohastructure.sql (1.38 KB, patch)
2015-12-17 23:59 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table (1.47 KB, patch)
2015-12-18 00:00 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9819 - QA Followup: Stopwords vestiges in es-ES removed (2.23 KB, patch)
2015-12-18 00:00 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9819: Make the tests pass (1.87 KB, patch)
2015-12-18 00:00 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9819 - 'stopwords'-related code removed (64.28 KB, patch)
2015-12-18 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9819 - Remove 'stopwords' table from kohastructure.sql (1.38 KB, patch)
2015-12-18 18:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table (1.47 KB, patch)
2015-12-18 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9819 - QA Followup: Stopwords vestiges in es-ES removed (2.29 KB, patch)
2015-12-18 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9819: Make the tests pass (1.94 KB, patch)
2015-12-18 18:01 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9819 [QA Followup] - Removed missed stopwords refs, remove Schema file (3.01 KB, patch)
2015-12-30 15:55 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2013-03-14 18:35:41 UTC

    
Comment 1 Katrin Fischer 2013-04-10 09:19:01 UTC
When stopwords related code is removed this should include the database table stopwords.
Comment 2 Tomás Cohen Arazi 2013-04-10 14:34:56 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2013-04-10 14:35:02 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2013-04-10 14:35:12 UTC Comment hidden (obsolete)
Comment 5 Marcel de Rooy 2013-04-10 14:40:06 UTC
Hi Tomas,
Could you also explain on this bug report why the stopwords should be removed?
Comment 6 Katrin Fischer 2013-04-10 14:49:25 UTC
The use of stopwords was limited to NoZebra - so this bug is a consequence of removing all NoZebra related code.
Comment 7 Kyle M Hall 2013-04-19 18:01:14 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2013-04-19 18:01:22 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2013-04-19 18:01:28 UTC Comment hidden (obsolete)
Comment 10 Jonathan Druart 2013-04-22 10:39:48 UTC
QA comment:

Great job Tomás. All looks good.
But there are 2 smalls things
1/ the structure file for PostgreSQL contains the "create table" statement for the stopwords table. I know this file is not updated, but while you're at it you could delete it.
2/ The stopwords sql file for spanish has been introduced into master before you provided your first patch. It should be deleted.


Marked as Failed QA for 2.
Comment 11 Tomás Cohen Arazi 2013-04-22 12:11:36 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2013-04-22 13:14:49 UTC
Tomás, thank you for your followup!

I found another issue: some unit tests in t/db_dependent/Search.t fail:

#   Failed test 'getRecords on _ALLRECORDS PQF returned all records'
#   at t/db_dependent/Search.t line 387.
#          got: undef
#     expected: '178'

#   Failed test 'getRecords PQF author search for Lessig returned proper number of matches'
#   at t/db_dependent/Search.t line 393.
#          got: undef
#     expected: '4'

#   Failed test 'getRecords CCL author search for Lessig returned proper number of matches'
#   at t/db_dependent/Search.t line 399.
#          got: undef
#     expected: '4'

#   Failed test 'getRecords CQL author search for Lessig returned proper number of matches'
#   at t/db_dependent/Search.t line 405.
#          got: undef
#     expected: '4'
# Looks like you failed 4 tests of 75.

They passed with success before yours patches.
Comment 13 Tomás Cohen Arazi 2013-04-22 13:30:41 UTC
(In reply to comment #12)
> They passed with success before yours patches.

What data set can I use to test it in my box?
Comment 14 Jonathan Druart 2013-04-22 13:56:37 UTC
(In reply to comment #13)
> (In reply to comment #12)
> > They passed with success before yours patches.
> 
> What data set can I use to test it in my box?

No idea!
On master, the prove t/db_dependent/Search.t command does not return 0?
Comment 15 Katrin Fischer 2014-11-09 23:22:32 UTC
It looks like we got stuck on this... but would be a nice clean-up? :)
Some of the code seems to have been removed since - patches don't apply.
Comment 16 Katrin Fischer 2014-11-10 00:25:02 UTC
*** Bug 11110 has been marked as a duplicate of this bug. ***
Comment 17 Nicole C. Engard 2015-03-06 08:33:33 UTC
I'd also like to see this make it's way in to Koha
Comment 18 Jonathan Druart 2015-12-15 10:08:38 UTC Comment hidden (obsolete)
Comment 19 Jonathan Druart 2015-12-15 10:08:42 UTC Comment hidden (obsolete)
Comment 20 Jonathan Druart 2015-12-15 10:08:45 UTC Comment hidden (obsolete)
Comment 21 Jonathan Druart 2015-12-15 10:08:48 UTC Comment hidden (obsolete)
Comment 22 Jonathan Druart 2015-12-15 10:08:51 UTC Comment hidden (obsolete)
Comment 23 Jonathan Druart 2015-12-15 10:09:26 UTC
Patches rebased against master, please test.
Comment 24 Chris Cormack 2015-12-17 23:59:34 UTC Comment hidden (obsolete)
Comment 25 Chris Cormack 2015-12-17 23:59:46 UTC Comment hidden (obsolete)
Comment 26 Chris Cormack 2015-12-18 00:00:00 UTC Comment hidden (obsolete)
Comment 27 Chris Cormack 2015-12-18 00:00:13 UTC Comment hidden (obsolete)
Comment 28 Chris Cormack 2015-12-18 00:00:25 UTC Comment hidden (obsolete)
Comment 29 Kyle M Hall 2015-12-18 18:00:48 UTC
Created attachment 45854 [details] [review]
Bug 9819 - 'stopwords'-related code removed

This patch removes code related to stopwords usage. The following methods are removed:

C4::Search->remove_stopwords
C4::Context->stopwords
C4::Context->_new_stopwords

And the buildQuery API was changed (removed the \@removed_stopwords return value).
A follow-up is provided for database changes, to make rebasing easier.

To test:
- Apply this patch
- Do some searches in both intranet and opac interfaces
- Nothing should break

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 30 Kyle M Hall 2015-12-18 18:00:58 UTC
Created attachment 45855 [details] [review]
Bug 9819 - Remove 'stopwords' table from kohastructure.sql

Just that.

To test:
- Apply the patch
- Do a fresh install
- The database structure should be created flawlessly
- There should not be a 'stopwords' table created

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 31 Kyle M Hall 2015-12-18 18:01:01 UTC
Created attachment 45856 [details] [review]
Bug 9819 - Make updatedatabase.pl remove the 'stopwords' table

To test:
- Apply the patch
- Will be prompted to run the updater
- The 'stopwords' database should be removed
  ( test on a mysql prompt with
    > SHOW TABLES LIKE 'stopwords';
   and should get no results)

Sponsored-by: Universidad Nacional de Córdoba
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 32 Kyle M Hall 2015-12-18 18:01:04 UTC
Created attachment 45857 [details] [review]
Bug 9819 - QA Followup: Stopwords vestiges in es-ES removed

Gotcha.
To+

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 33 Kyle M Hall 2015-12-18 18:01:07 UTC
Created attachment 45858 [details] [review]
Bug 9819: Make the tests pass

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 34 Kyle M Hall 2015-12-30 15:55:32 UTC
Created attachment 46092 [details] [review]
Bug 9819 [QA Followup] - Removed missed stopwords refs, remove Schema file
Comment 35 Kyle M Hall 2015-12-30 16:14:43 UTC
Pushed to master, thanks Tomas and Jonathan!