Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Support search expression (currently limited) #2826

Conversation

gotmyname2018
Copy link

As part of work to fix riot-web issue 2928, add support of search expression to synapse.
See https://github.com/vector-im/riot-web/issues/4752 for discussion about this feature.
The search expression could contain 3 kind of expression:

  • plain word: to match against message content (same meaning as previous search term)
  • tag: to match against message tags, prefix tag with ':', e.g. ":starred"
  • key value pair: to match against message properties, e.g. "after:17/01/01"
    Multiple expressions could be concated with white space, it means Expression1 AND Expression2...
    Currently only ":starred" tag expression was handled, but it could be combined with plain words.

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

1 similar comment
@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@Half-Shot
Copy link
Collaborator

If we are going to start adding special clauses to search requests, then this definitely needs a thorough spec written for matrix-doc.

I'm kinda erring on the side of using a recognized standard for searching such as Lucene Query format as oppose to writing our own implementation because search is notoriously hard to get right without causing bugs, and it helps other homeserver and client writers out there since comprehensive docs exist for these formats.

@gotmyname2018
Copy link
Author

I'm ok with recognized standard, but there may be some consequences.
I believe this will require quite some efforts to integrate those and replace current implementation, and I'm not an expert of Lucene, so if I'm wrong, pls kindly correct me: I also believe the use case of Lucene is not the same as matrix, for conversation kind of search, bring context with search result is really useful and required some specific design as current implementation did.
And conversation search set is generally much smaller than typical Lucence usecase, too complex search criteria support may be too overbrown.
So I prefer some simple form of search criteria and implemented incrementally, only those are most useful for the users are implemented first, in this way, the end user could enjoy the new features more quickly.

@richvdh
Copy link
Member

richvdh commented Feb 1, 2018

my feeling on this is that, rather than either implementing our own search expression language, or adopting something like lucene, we ought rather to improve upon the "filter" support already provided by the search api (https://matrix.org/docs/spec/client_server/r0.3.0.html#post-matrix-client-r0-search / https://matrix.org/docs/spec/client_server/r0.3.0.html#filter).

[a couple of other notes about PRs in general: firstly, this PR includes a bunch of unrelated changes, which makes it hard to review and unlikely to be accepted. secondly, we require sign-off for any contributions as per https://github.com/matrix-org/synapse/blob/master/CONTRIBUTING.rst#sign-off]

@richvdh
Copy link
Member

richvdh commented Feb 1, 2018

and yes, anything like this will need discussion and agreement before it will be accepted. See https://github.com/matrix-org/matrix-doc/blob/master/CONTRIBUTING.rst#specification-changes for notes on our preferred workflow for this.

// cc also @t3chguy

@gotmyname2018
Copy link
Author

Thanks for the feedback.
I will wait and respect your team's final decision about the way to implement this feature.
This PR includes changes to the README.rst file which is already merged into the synapse/develop branch, I don't modify it, the only changes I made is in synapse/handlers/search.py and synapse/storage/search.py, so I think review the changes should not be problem.
If you think the changes are acceptable after discussion or give more advice after that, I will make required changes and update this pull request or create a new pull request accordingly, and surely with the required sign-off.

…be handled

Signed-off-by: Xiaodong HU <gotmyname2018@outlook.com>
@gotmyname2018 gotmyname2018 force-pushed the support_search_expression branch from ab23e8f to ea2c4f0 Compare February 1, 2018 14:59
@gotmyname2018
Copy link
Author

Whatever, I updated this pull request with squashed commits and get rid of the unrelated changes to make your review more easy, and this time also with my sign-off.

@richvdh
Copy link
Member

richvdh commented Feb 13, 2018

ok I'm going to close this I'm afraid. I'd rather see us improving the filter support.

@richvdh richvdh closed this Feb 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants