View Issue Details

IDProjectCategoryView StatusLast Update
0026447mantisbtfilterspublic2020-07-27 12:45
Reporterpolzin Assigned To 
PriorityhighSeverityfeatureReproducibilityN/A
Status acknowledgedResolutionopen 
Summary0026447: Enhancement of free text filter function: Search for custom fields, summary-only, id-only and mixing AND and OR.
Description

Attached is a patch for improved free text filter function.

Examples:
i:123 OR s:summary c:custom s:"test test" "jo jo"

i:123 searches only for the bug id (useful if you have a list of IDs and create a search link with 'xargs -n 1 printf "i:%s OR "')
c:text searches only the custom fields
s:text searches only the summary (faster and more focussed results).
OR allows mixing AND and OR in one query

Necessary improvements for incorporation:

  • end-user documentation
  • possible extension to AND
TagsNo tags attached.

Relationships

has duplicate 0027062 closedatrol Use OR and phrase search logical operator in text search 

Activities

polzin

polzin

2019-12-09 10:16

reporter  

filter_patch.txt (4,578 bytes)   
diff --git a/core/classes/BugFilterQuery.class.php b/core/classes/BugFilterQuery.class.php
index 110b5c5..ece5a2c 100644
--- a/core/classes/BugFilterQuery.class.php
+++ b/core/classes/BugFilterQuery.class.php
@@ -1401,22 +1401,32 @@ class BugFilterQuery extends DbQuery {
 		}
 
 		# break up search terms by spacing or quoting
-		preg_match_all( "/-?([^'\"\s]+|\"[^\"]+\"|'[^']+')/", $this->filter[FILTER_PROPERTY_SEARCH], $t_matches, PREG_SET_ORDER );
+		# optional "s:, i:, c:" for faster summary only requests
+		preg_match_all( "/-?([sic]:)?([^'\"\s]+|\"[^\"]+\"|'[^']+')/", $this->filter[FILTER_PROPERTY_SEARCH], $t_matches, PREG_SET_ORDER );
 
-		# organize terms without quoting, paying attention to negation
-		$t_search_terms = array();
-		foreach( $t_matches as $t_match ) {
-			$t_search_terms[trim( $t_match[1], "\'\"" )] = ( $t_match[0][0] == '-' );
-		}
 
+		$t_bug_table = $this->helper_table_alias_for_bugnote();
 		$t_bugnote_table = $this->helper_table_alias_for_bugnote();
 
 		# build a big where-clause and param list for all search terms, including negations
 		$t_first = true;
-		$t_textsearch_where_clause = '( ';
-		foreach( $t_search_terms as $t_search_term => $t_negate ) {
-			if( !$t_first ) {
-				$t_textsearch_where_clause .= ' AND ';
+		$t_after_or = false; # if we are after an OR 
+		$t_textsearch_where_clause = "( ( "; # second ( for OR
+		foreach( $t_matches as $t_match ) {
+			# analyse search term
+			$t_search_term = trim( $t_match[2], "\'\"" );
+			$t_negate = ( $t_match[0][0] == '-' );
+			$t_summary_only = ( $t_match[1] && $t_match[1][0] == 's' );
+			$t_id_only = ( $t_match[1] && $t_match[1][0] == 'i' );
+			$t_custom_field_only = ( $t_match[1] && $t_match[1][0] == 'c' );
+			if ( !$t_first && !$t_after_or ) {
+				if ( $t_search_term == 'OR' ) {
+					$t_textsearch_where_clause .= ' ) OR ( ';
+					$t_after_or = true;
+					continue;
+				} else {
+					$t_textsearch_where_clause .= ' AND ';
+				}
 			}
 
 			if( $t_negate ) {
@@ -1424,25 +1434,53 @@ class BugFilterQuery extends DbQuery {
 			}
 
 			$c_search = '%' . $t_search_term . '%';
-			$t_textsearch_where_clause .= '( ' . $this->sql_like( '{bug}.summary', $c_search )
-					. ' OR ' . $this->sql_like( '{bug_text}.description', $c_search )
-					. ' OR ' . $this->sql_like( '{bug_text}.steps_to_reproduce', $c_search )
-					. ' OR ' . $this->sql_like( '{bug_text}.additional_information', $c_search )
-					. ' OR ' . $this->sql_like( '{bugnote_text}.note', $c_search );
+			$t_textsearch_where_clause .= '( ';
+			# search in summary only if not id / custom_field_search
+			if ( !$t_id_only && !$t_custom_field_only ) {
+				$t_textsearch_where_clause .= $this->sql_like( '{bug}.summary', $c_search );
+                        } else {
+				$t_textsearch_where_clause .= '0';
+			}
+			if ( !$t_summary_only && !$t_id_only && !$t_custom_field_only ) {
+				$t_textsearch_where_clause .=
+				  ' OR ' . $this->sql_like( '{bug_text}.description', $c_search ) .
+				  ' OR ' . $this->sql_like( '{bug_text}.steps_to_reproduce', $c_search ) .
+				  ' OR ' . $this->sql_like( '{bug_text}.additional_information', $c_search ) .
+				  ' OR ' . $this->sql_like( '{bugnote_text}.note', $c_search );
+			}
+
 
-			if( is_numeric( $t_search_term ) ) {
+			if( is_numeric( $t_search_term ) && !$t_summary_only && !$t_custom_field_only ) {
 				# Note: no need to test negative values, '-' sign has been removed
 				if( $t_search_term <= DB_MAX_INT ) {
 					$c_search_int = (int)$t_search_term;
 					$t_textsearch_where_clause .= ' OR {bug}.id = ' . $this->param( $c_search_int );
-					$t_textsearch_where_clause .= ' OR ' . $t_bugnote_table . '.id = ' . $this->param( $c_search_int );
+					#  id search only in bugs, not in bugnotes
+					if ( !$t_id_only ) {
+						$t_textsearch_where_clause .= ' OR ' . $t_bugnote_table . '.id = ' . $this->param( $c_search_int );
+					}
 				}
 			}
 
+			if ( $t_custom_field_only ) {
+				$t_textsearch_where_clause .=
+					' OR ' . "({bug}.id IN ( SELECT DISTINCT bug_id from " . db_get_table( 'mantis_custom_field_string_table' ) . " where " . $this->sql_like( "value", $c_search )."))";
+			}
 			$t_textsearch_where_clause .= ' )';
 			$t_first = false;
+			$t_after_or = false;
 		}
-		$t_textsearch_where_clause .= ' )';
+		if ( $t_after_or ) {
+			# avoid SQL error with "XXX OR"
+			$t_textsearch_where_clause .= ' FALSE ';
+		}
+		$t_textsearch_where_clause .= ' ) )';
+
+
+		log_event(LOG_FILTERING,'Clause'.$t_textsearch_where_clause);
+
+
+
 
 		# add text query elements to arrays
 		if( !$t_first ) {

filter_patch.txt (4,578 bytes)   
dregad

dregad

2019-12-09 10:53

developer   ~0063215

@polzin thanks for your contribution

Would you mind submitting your patch as a pull request on Github, to make it easier to test and review ?

polzin

polzin

2019-12-09 17:15

reporter   ~0063221

Last edited: 2019-12-15 06:01

View 2 revisions

Pull request see https://github.com/mantisbt/mantisbt/pull/1597 (updated)

dregad

dregad

2019-12-10 05:29

developer   ~0063227

Thanks !
Are you planning to develop and provide the "necessary improvements" described in the above description ?

emil-tsl

emil-tsl

2019-12-10 06:21

reporter   ~0063231

Work for me. Tested on 2.22.1

Do you mind if I add a suggestion?
Drop-down Search button where you can choose what I want to search for.

564564564.jpg (99,255 bytes)   
564564564.jpg (99,255 bytes)   
polzin

polzin

2019-12-10 07:39

reporter   ~0063237

@dregad Do you have a suggestion, how to implement the end-user documentation? The placeholder has probably not enough space for a syntax description. The tooltip describing the syntax would be an easy addition. If that's wanted, I can do it.

For the other improvement "possible extension to AND" , I checked the code and noticed that it was a mistake to think there is something missing. AND is the default, therefore no further extension is necessary.

I would not use a drop-down, as suggested for @emil-tsl, because this is a different gui concept.

atrol

atrol

2019-12-10 09:16

developer   ~0063241

I don't have time for a deeper look, but please consider my warning 0007183:0059789 and the discussion around previous PRs
https://github.com/mantisbt/mantisbt/pull/1168
https://github.com/mantisbt/mantisbt/pull/1140

Especially the following comment concerning performance and security
https://github.com/mantisbt/mantisbt/pull/1140#issuecomment-320712837

emil-tsl

emil-tsl

2019-12-13 05:00

reporter   ~0063266

@polzin
How to do it so that using the prefix c: I would not only search for custom fields but everything (I mean topic title, description + custom fields). I need it so much.

polzin

polzin

2019-12-13 06:44

reporter   ~0063269

@emil-tsl You could do this using "c:TEST OR TEST"

polzin

polzin

2019-12-13 06:49

reporter   ~0063270

@atrol As the default behaviour is not changed, the performance is not an issue. In fact, for big repositories it is a relevant performance advantage to use "s:XXX" for a search only in the summary.

polzin

polzin

2019-12-15 06:04

reporter   ~0063282

I updated to pull request, so that it contains only one commit and changed the author.email.
https://github.com/mantisbt/mantisbt/pull/1597

atrol

atrol

2019-12-15 07:29

developer   ~0063283

@atrol As the default behaviour is not changed, the performance is not an issue.

Yes concerning existing functionality.
No concerning new functionality, as it allows users to stress your database server in a quite easy way. But I think this is acceptable

There is still the security aspect I mentioned in the old PR

Independant from that, there is a serious conceptual issue.
After the changes, you are able to search based on data where you are not allowed to view the data.

E.g. set Read Access of a custom field to access level manager.
Log in as a developer and search by text.
You will see issues in the list where you are not allowed to see the custom field.

polzin

polzin

2019-12-17 16:54

reporter   ~0063291

@atrol Thanks for the feedback.

The security problem is fixed and tested with a user being able to see the issue, but not the custom field.

The performance is still okay in a setting with > 60000 issues and > 50 custom fields (where > 15 string custom fields are searched):
Search for a word in all issues without prefix (current search) : approx. 8 second
Search for a custom field ("c:"): approx. 4 second
Search for a summary ("s:"): approx. 0.1 second

cproensa

cproensa

2019-12-17 18:10

developer   ~0063294

As the default behaviour is not changed, the performance is not an issue. In fact, for big repositories it is a relevant performance advantage to use "s:XXX" for a search only in the summary.

I think performance may be relevant
For each custom field that need to be filtered, a new join for custom-fields-table is added. The complexity of each of those joins (which accounts for project specific permissions) has not changed. However, previously we only added those joins when a field is explicitly filtered.
With your proposal, the default behaviour is to search in all available custom fields, so we have N complex joins always added for a text search... Unless the user explicitly uses the s:, etc, modifiers, which is not very user friendly for plain users.

cproensa

cproensa

2019-12-17 18:20

developer   ~0063295

Last edited: 2019-12-17 18:21

View 2 revisions

Previously, when this topic has been discussed, the proposals are inthe line of:

  • add some kind of flags or checkboxes where the user can choose whether the search will look in one or several targets: summary, description, notes, custom fields, etc.

your proposal has the potential to do more complex searches, like selectively target different terms in each target: "s:something OR c:otherthing" right?
At the expense of a more complex parsing of the search query.
We need to make sure we want that complexity, as we would need to live with that and support it for the future.

With your scheme, should we have a mapping of the old searches (eg, stored filters), so that the old search for somethingbecomes s:something?
Otherwise the behaviour of pre-existing filters may change.

polzin

polzin

2019-12-17 19:17

reporter   ~0063296

Last edited: 2019-12-17 19:21

View 2 revisions

@cproensa: I am not sure if there is a misunderstanding: The current behaviour is not changed (if not a "[sci]:" or "OR" is used). If a user or a stored filter looked for something, it will now find the same results, with the same joins and the same search performance.

By the way: Compared to the checkboxes, one very usefull sideeffect for power users is: You can use this browser shortcuts/keywords, such that CTRL-L + mantis s:something brings you in fast pace to relevant search result.

cproensa

cproensa

2019-12-17 19:42

developer   ~0063297

ok, sorry, i thought that this searches within custom fields (along all other contexts) by default.

so, would it make more sense for this to include custom fields by default?
Would your current definition have the following behaviour?
For a given text "XXX" in the summary:

  • search for "XXX": found
  • search for "s:XXX": found
    But given text "CCC" in custom field:
  • search for "CCC": not found
  • search for "c:CCC": found
polzin

polzin

2019-12-17 20:12

reporter   ~0063298

@cproensa: Add custom filters by default would bring you: perfomance drops, changes of existing filters, more complex sql by default, all those things you probably don't want to have. :-)

polzin

polzin

2020-01-27 13:36

reporter   ~0063520

@atrol, @cproensa, @dregad: see 0026447:0063291, I think all security and performance issues are addressed now. How do we proceed?

amphetamine

amphetamine

2020-07-06 22:56

reporter   ~0064153

@polzin

is it compatible with current version 2.24.1?

polzin

polzin

2020-07-07 04:17

reporter   ~0064154

@amphetamine
Yes, I use it happily with 2.24.1.

amphetamine

amphetamine

2020-07-07 08:30

reporter   ~0064156

file changed follow by
https://github.com/mantisbt/mantisbt/pull/1597/files
i:100 > found
i:1000 > found
i:1 > not found
i:10 > not found
no problem with s:text
not working by c:text

polzin

polzin

2020-07-07 08:54

reporter   ~0064157

@amphetamine
Thanks for looking into this.
I don't understand fully, what you did and what was your result.
My guess is:

  • You changed my code into something less nested
  • You tested your code and found some results. With i:1, i:10, c:text you expected some results, but got none.
    Is this correct?

If yes, could you try my code and see if it gives the same result?

amphetamine

amphetamine

2020-07-07 09:47

reporter   ~0064158

  • You changed my code into something less nested
    just changed the code (+/-) in BugFilterQuery.class.php
  • You tested your code and found some results. With i:1, i:10, c:text you expected some results, but got none.
    Is this correct?
    YES

no idea how to execute your code (filter_patch.txt) under XAMPP
or would you mind provide your BugFilterQuery.class.php for testing?

polzin

polzin

2020-07-07 11:28

reporter   ~0064159

So, you basically took my code from github, without modifications, right.
Then to your tests:

  1. Are there issues with id 1, 10, existing?
  2. If you search with i:1, are other filters active, like "hide status: resolved"?
  3. If you know the name of issue with id 1, can you search for this?
  4. Do you have an issue with a string custom field, with a value containing "test"? c: searches in string custom fields only.
    Can you provide screenshots of the issues you expect to be found and the search results where they are not found?
amphetamine

amphetamine

2020-07-07 21:12

reporter   ~0064161

problem solved for i:1 and i:10 (the issue can not be hide)

found the root cause for c:search not found

string custom fields have to add to filter

if possible for those fields not add to filter can be searched as well

polzin

polzin

2020-07-27 12:45

reporter   ~0064186

@amphetamine

I find it more consistant to search only those fields that are mentioned in filtered. But if this is does not suite you, you may remove the lines frombuild_custom_field_search:

            if( !$t_field_info['filter_by'] ) {
                continue;
            }

Issue History

Date Modified Username Field Change
2019-12-09 10:16 polzin New Issue
2019-12-09 10:16 polzin File Added: filter_patch.txt
2019-12-09 10:53 dregad Status new => feedback
2019-12-09 10:53 dregad Note Added: 0063215
2019-12-09 17:15 polzin Note Added: 0063221
2019-12-09 17:15 polzin Status feedback => new
2019-12-10 05:29 dregad Severity major => feature
2019-12-10 05:29 dregad Status new => acknowledged
2019-12-10 05:29 dregad Product Version 2.22.2 =>
2019-12-10 05:29 dregad Note Added: 0063227
2019-12-10 06:21 emil-tsl Note Added: 0063231
2019-12-10 06:21 emil-tsl File Added: 564564564.jpg
2019-12-10 07:39 polzin Note Added: 0063237
2019-12-10 09:16 atrol Note Added: 0063241
2019-12-13 05:00 emil-tsl Note Added: 0063266
2019-12-13 06:44 polzin Note Added: 0063269
2019-12-13 06:49 polzin Note Added: 0063270
2019-12-15 06:01 polzin Note Edited: 0063221 View Revisions
2019-12-15 06:04 polzin Note Added: 0063282
2019-12-15 07:29 atrol Note Added: 0063283
2019-12-17 16:54 polzin Note Added: 0063291
2019-12-17 18:10 cproensa Note Added: 0063294
2019-12-17 18:20 cproensa Note Added: 0063295
2019-12-17 18:21 cproensa Note Edited: 0063295 View Revisions
2019-12-17 19:17 polzin Note Added: 0063296
2019-12-17 19:21 polzin Note Edited: 0063296 View Revisions
2019-12-17 19:42 cproensa Note Added: 0063297
2019-12-17 20:12 polzin Note Added: 0063298
2020-01-27 13:36 polzin Note Added: 0063520
2020-06-27 01:37 atrol Relationship added has duplicate 0027062
2020-07-06 22:56 amphetamine Note Added: 0064153
2020-07-07 04:17 polzin Note Added: 0064154
2020-07-07 08:30 amphetamine Note Added: 0064156
2020-07-07 08:54 polzin Note Added: 0064157
2020-07-07 09:47 amphetamine Note Added: 0064158
2020-07-07 11:28 polzin Note Added: 0064159
2020-07-07 21:12 amphetamine Note Added: 0064161
2020-07-27 12:45 polzin Note Added: 0064186