From e0ee73ef9963ab398bcc6011795ad23e8e003147 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 31 Jan 2018 08:26:40 -0600 Subject: ABC-79: Optimize channel autocomplete query (#8163) * optimize channel autocomplete query * move to new autocomplete endpoint --- api4/channel.go | 22 +++ app/channel.go | 8 ++ store/sqlstore/channel_store.go | 152 ++++++++++++++------ store/store.go | 1 + store/storetest/channel_store.go | 197 +++++++++++++------------- store/storetest/mocks/ChannelStore.go | 16 +++ store/storetest/mocks/UserAccessTokenStore.go | 28 ++-- 7 files changed, 261 insertions(+), 163 deletions(-) diff --git a/api4/channel.go b/api4/channel.go index 9801c9dc0..29dff883f 100644 --- a/api4/channel.go +++ b/api4/channel.go @@ -20,6 +20,7 @@ func (api *API) InitChannel() { api.BaseRoutes.ChannelsForTeam.Handle("/deleted", api.ApiSessionRequired(getDeletedChannelsForTeam)).Methods("GET") api.BaseRoutes.ChannelsForTeam.Handle("/ids", api.ApiSessionRequired(getPublicChannelsByIdsForTeam)).Methods("POST") api.BaseRoutes.ChannelsForTeam.Handle("/search", api.ApiSessionRequired(searchChannelsForTeam)).Methods("POST") + api.BaseRoutes.ChannelsForTeam.Handle("/autocomplete", api.ApiSessionRequired(autocompleteChannelsForTeam)).Methods("GET") api.BaseRoutes.User.Handle("/teams/{team_id:[A-Za-z0-9]+}/channels", api.ApiSessionRequired(getChannelsForTeamForUser)).Methods("GET") api.BaseRoutes.Channel.Handle("", api.ApiSessionRequired(getChannel)).Methods("GET") @@ -489,6 +490,27 @@ func getChannelsForTeamForUser(c *Context, w http.ResponseWriter, r *http.Reques } } +func autocompleteChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) { + c.RequireTeamId() + if c.Err != nil { + return + } + + if !c.App.SessionHasPermissionToTeam(c.Session, c.Params.TeamId, model.PERMISSION_LIST_TEAM_CHANNELS) { + c.SetPermissionError(model.PERMISSION_LIST_TEAM_CHANNELS) + return + } + + name := r.URL.Query().Get("name") + + if channels, err := c.App.AutocompleteChannels(c.Params.TeamId, name); err != nil { + c.Err = err + return + } else { + w.Write([]byte(channels.ToJson())) + } +} + func searchChannelsForTeam(c *Context, w http.ResponseWriter, r *http.Request) { c.RequireTeamId() if c.Err != nil { diff --git a/app/channel.go b/app/channel.go index 0054fe14b..e4bf48654 100644 --- a/app/channel.go +++ b/app/channel.go @@ -1255,6 +1255,14 @@ func (a *App) UpdateChannelLastViewedAt(channelIds []string, userId string) *mod return nil } +func (a *App) AutocompleteChannels(teamId string, term string) (*model.ChannelList, *model.AppError) { + if result := <-a.Srv.Store.Channel().AutocompleteInTeam(teamId, term); result.Err != nil { + return nil, result.Err + } else { + return result.Data.(*model.ChannelList), nil + } +} + func (a *App) SearchChannels(teamId string, term string) (*model.ChannelList, *model.AppError) { if result := <-a.Srv.Store.Channel().SearchInTeam(teamId, term); result.Err != nil { return nil, result.Err diff --git a/store/sqlstore/channel_store.go b/store/sqlstore/channel_store.go index af78b06e0..c2979526c 100644 --- a/store/sqlstore/channel_store.go +++ b/store/sqlstore/channel_store.go @@ -7,6 +7,7 @@ import ( "database/sql" "fmt" "net/http" + "sort" "strconv" "strings" @@ -81,6 +82,7 @@ func NewSqlChannelStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface) func (s SqlChannelStore) CreateIndexesIfNotExists() { s.CreateIndexIfNotExists("idx_channels_team_id", "Channels", "TeamId") s.CreateIndexIfNotExists("idx_channels_name", "Channels", "Name") + s.CreateIndexIfNotExists("idx_channels_displayname", "Channels", "DisplayName") s.CreateIndexIfNotExists("idx_channels_update_at", "Channels", "UpdateAt") s.CreateIndexIfNotExists("idx_channels_create_at", "Channels", "CreateAt") s.CreateIndexIfNotExists("idx_channels_delete_at", "Channels", "DeleteAt") @@ -1257,18 +1259,58 @@ func (s SqlChannelStore) GetMembersForUser(teamId string, userId string) store.S }) } +func (s SqlChannelStore) AutocompleteInTeam(teamId string, term string) store.StoreChannel { + return store.Do(func(result *store.StoreResult) { + queryFormat := ` + SELECT + * + FROM + Channels + WHERE + TeamId = :TeamId + AND Type = 'O' + AND DeleteAt = 0 + %v + LIMIT 50` + + var channels model.ChannelList + + if likeClause, likeTerm := s.buildLIKEClause(term); likeClause == "" { + if _, err := s.GetReplica().Select(&channels, fmt.Sprintf(queryFormat, ""), map[string]interface{}{"TeamId": teamId}); err != nil { + result.Err = model.NewAppError("SqlChannelStore.AutocompleteInTeam", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError) + } + } else { + // Using a UNION results in index_merge and fulltext queries and is much faster than the ref + // query you would get using an OR of the LIKE and full-text clauses. + fulltextClause, fulltextTerm := s.buildFulltextClause(term) + likeQuery := fmt.Sprintf(queryFormat, "AND "+likeClause) + fulltextQuery := fmt.Sprintf(queryFormat, "AND "+fulltextClause) + query := fmt.Sprintf("(%v) UNION (%v) LIMIT 50", likeQuery, fulltextQuery) + + if _, err := s.GetReplica().Select(&channels, query, map[string]interface{}{"TeamId": teamId, "LikeTerm": likeTerm, "FulltextTerm": fulltextTerm}); err != nil { + result.Err = model.NewAppError("SqlChannelStore.AutocompleteInTeam", "store.sql_channel.search.app_error", nil, "term="+term+", "+", "+err.Error(), http.StatusInternalServerError) + } + } + + sort.Slice(channels, func(a, b int) bool { + return channels[a].DisplayName < channels[b].DisplayName + }) + result.Data = &channels + }) +} + func (s SqlChannelStore) SearchInTeam(teamId string, term string) store.StoreChannel { return store.Do(func(result *store.StoreResult) { searchQuery := ` SELECT - * + * FROM - Channels + Channels WHERE - TeamId = :TeamId + TeamId = :TeamId AND Type = 'O' AND DeleteAt = 0 - SEARCH_CLAUSE + SEARCH_CLAUSE ORDER BY DisplayName LIMIT 100` @@ -1305,13 +1347,8 @@ func (s SqlChannelStore) SearchMore(userId string, teamId string, term string) s }) } -func (s SqlChannelStore) performSearch(searchQuery string, term string, parameters map[string]interface{}) store.StoreResult { - result := store.StoreResult{} - - // Copy the terms as we will need to prepare them differently for each search type. - likeTerm := term - fulltextTerm := term - +func (s SqlChannelStore) buildLIKEClause(term string) (likeClause, likeTerm string) { + likeTerm = term searchColumns := "Name, DisplayName" // These chars must be removed from the like query. @@ -1324,56 +1361,77 @@ func (s SqlChannelStore) performSearch(searchQuery string, term string, paramete likeTerm = strings.Replace(likeTerm, c, "*"+c, -1) } + if likeTerm == "" { + return + } + + // Prepare the LIKE portion of the query. + var searchFields []string + for _, field := range strings.Split(searchColumns, ", ") { + if s.DriverName() == model.DATABASE_DRIVER_POSTGRES { + searchFields = append(searchFields, fmt.Sprintf("lower(%s) LIKE lower(%s) escape '*'", field, ":LikeTerm")) + } else { + searchFields = append(searchFields, fmt.Sprintf("%s LIKE %s escape '*'", field, ":LikeTerm")) + } + } + + likeClause = fmt.Sprintf("(%s)", strings.Join(searchFields, " OR ")) + likeTerm += "%" + return +} + +func (s SqlChannelStore) buildFulltextClause(term string) (fulltextClause, fulltextTerm string) { + // Copy the terms as we will need to prepare them differently for each search type. + fulltextTerm = term + + searchColumns := "Name, DisplayName" + // These chars must be treated as spaces in the fulltext query. for _, c := range spaceFulltextSearchChar { fulltextTerm = strings.Replace(fulltextTerm, c, " ", -1) } - if likeTerm == "" { - // If the likeTerm is empty after preparing, then don't bother searching. - searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1) - } else { - // Prepare the LIKE portion of the query. - var searchFields []string - for _, field := range strings.Split(searchColumns, ", ") { - if s.DriverName() == model.DATABASE_DRIVER_POSTGRES { - searchFields = append(searchFields, fmt.Sprintf("lower(%s) LIKE lower(%s) escape '*'", field, ":LikeTerm")) + // Prepare the FULLTEXT portion of the query. + if s.DriverName() == model.DATABASE_DRIVER_POSTGRES { + splitTerm := strings.Fields(fulltextTerm) + for i, t := range strings.Fields(fulltextTerm) { + if i == len(splitTerm)-1 { + splitTerm[i] = t + ":*" } else { - searchFields = append(searchFields, fmt.Sprintf("%s LIKE %s escape '*'", field, ":LikeTerm")) + splitTerm[i] = t + ":* &" } } - likeSearchClause := fmt.Sprintf("(%s)", strings.Join(searchFields, " OR ")) - parameters["LikeTerm"] = fmt.Sprintf("%s%%", likeTerm) - // Prepare the FULLTEXT portion of the query. - if s.DriverName() == model.DATABASE_DRIVER_POSTGRES { - splitTerm := strings.Fields(fulltextTerm) - for i, t := range strings.Fields(fulltextTerm) { - if i == len(splitTerm)-1 { - splitTerm[i] = t + ":*" - } else { - splitTerm[i] = t + ":* &" - } - } - - fulltextTerm = strings.Join(splitTerm, " ") + fulltextTerm = strings.Join(splitTerm, " ") - fulltextSearchClause := fmt.Sprintf("((%s) @@ to_tsquery(:FulltextTerm))", convertMySQLFullTextColumnsToPostgres(searchColumns)) - searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "AND ("+likeSearchClause+" OR "+fulltextSearchClause+")", 1) - } else if s.DriverName() == model.DATABASE_DRIVER_MYSQL { - splitTerm := strings.Fields(fulltextTerm) - for i, t := range strings.Fields(fulltextTerm) { - splitTerm[i] = "+" + t + "*" - } + fulltextClause = fmt.Sprintf("((%s) @@ to_tsquery(:FulltextTerm))", convertMySQLFullTextColumnsToPostgres(searchColumns)) + } else if s.DriverName() == model.DATABASE_DRIVER_MYSQL { + splitTerm := strings.Fields(fulltextTerm) + for i, t := range strings.Fields(fulltextTerm) { + splitTerm[i] = "+" + t + "*" + } - fulltextTerm = strings.Join(splitTerm, " ") + fulltextTerm = strings.Join(splitTerm, " ") - fulltextSearchClause := fmt.Sprintf("MATCH(%s) AGAINST (:FulltextTerm IN BOOLEAN MODE)", searchColumns) - searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", fmt.Sprintf("AND (%s OR %s)", likeSearchClause, fulltextSearchClause), 1) - } + fulltextClause = fmt.Sprintf("MATCH(%s) AGAINST (:FulltextTerm IN BOOLEAN MODE)", searchColumns) } - parameters["FulltextTerm"] = fulltextTerm + return +} + +func (s SqlChannelStore) performSearch(searchQuery string, term string, parameters map[string]interface{}) store.StoreResult { + result := store.StoreResult{} + + likeClause, likeTerm := s.buildLIKEClause(term) + if likeTerm == "" { + // If the likeTerm is empty after preparing, then don't bother searching. + searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "", 1) + } else { + parameters["LikeTerm"] = likeTerm + fulltextClause, fulltextTerm := s.buildFulltextClause(term) + parameters["FulltextTerm"] = fulltextTerm + searchQuery = strings.Replace(searchQuery, "SEARCH_CLAUSE", "AND ("+likeClause+" OR "+fulltextClause+")", 1) + } var channels model.ChannelList diff --git a/store/store.go b/store/store.go index 2742c0889..85f215ab9 100644 --- a/store/store.go +++ b/store/store.go @@ -154,6 +154,7 @@ type ChannelStore interface { AnalyticsTypeCount(teamId string, channelType string) StoreChannel ExtraUpdateByUser(userId string, time int64) StoreChannel GetMembersForUser(teamId string, userId string) StoreChannel + AutocompleteInTeam(teamId string, term string) StoreChannel SearchInTeam(teamId string, term string) StoreChannel SearchMore(userId string, teamId string, term string) StoreChannel GetMembersByIds(channelId string, userIds []string) StoreChannel diff --git a/store/storetest/channel_store.go b/store/storetest/channel_store.go index 12781bdad..121b40a01 100644 --- a/store/storetest/channel_store.go +++ b/store/storetest/channel_store.go @@ -1940,116 +1940,109 @@ func testChannelStoreSearchInTeam(t *testing.T, ss store.Store) { o11.Type = model.CHANNEL_OPEN store.Must(ss.Channel().Save(&o11, -1)) - if result := <-ss.Channel().SearchInTeam(o1.TeamId, "ChannelA"); result.Err != nil { - t.Fatal(result.Err) - } else { - channels := result.Data.(*model.ChannelList) - if len(*channels) != 2 { - t.Fatal("wrong length") - } - } - - if result := <-ss.Channel().SearchInTeam(o1.TeamId, ""); result.Err != nil { - t.Fatal(result.Err) - } else { - channels := result.Data.(*model.ChannelList) - if len(*channels) == 0 { - t.Fatal("should not be empty") - } - } - - if result := <-ss.Channel().SearchInTeam(o1.TeamId, "blargh"); result.Err != nil { - t.Fatal(result.Err) - } else { - channels := result.Data.(*model.ChannelList) - if len(*channels) != 0 { - t.Fatal("should be empty") - } - } - - if result := <-ss.Channel().SearchInTeam(o1.TeamId, "off-"); result.Err != nil { - t.Fatal(result.Err) - } else { - channels := result.Data.(*model.ChannelList) - if len(*channels) != 2 { - t.Fatal("should return 2 channels, not including private channel") - } - - if (*channels)[0].Name != o7.Name { - t.Fatal("wrong channel returned") - } - - if (*channels)[1].Name != o6.Name { - t.Fatal("wrong channel returned") - } - } - - if result := <-ss.Channel().SearchInTeam(o1.TeamId, "off-topic"); result.Err != nil { - t.Fatal(result.Err) - } else { - channels := result.Data.(*model.ChannelList) - if len(*channels) != 1 { - t.Fatal("should return 1 channel") - } - - if (*channels)[0].Name != o6.Name { - t.Fatal("wrong channel returned") - } - } + for name, search := range map[string]func(teamId string, term string) store.StoreChannel{ + "AutocompleteInTeam": ss.Channel().AutocompleteInTeam, + "SearchInTeam": ss.Channel().SearchInTeam, + } { + t.Run(name, func(t *testing.T) { + if result := <-search(o1.TeamId, "ChannelA"); result.Err != nil { + t.Fatal(result.Err) + } else { + channels := result.Data.(*model.ChannelList) + if len(*channels) != 2 { + t.Fatal("wrong length") + } + } - /* - // Disabling this check as it will fail on PostgreSQL as we have "liberalised" channel matching to deal with - // Full-Text Stemming Limitations. - if result := <-ss.Channel().SearchMore(m1. - if result := <-ss.Channel().SearchInTeam(o1.TeamId, "off-topics"); result.Err != nil { - t.Fatal(result.Err) - } else { - channels := result.Data.(*model.ChannelList) - if len(*channels) != 0 { - t.Fatal("should be empty") + if result := <-search(o1.TeamId, ""); result.Err != nil { + t.Fatal(result.Err) + } else { + channels := result.Data.(*model.ChannelList) + if len(*channels) == 0 { + t.Fatal("should not be empty") + } } - } - */ - if result := <-ss.Channel().SearchInTeam(o1.TeamId, "town square"); result.Err != nil { - t.Fatal(result.Err) - } else { - channels := result.Data.(*model.ChannelList) - if len(*channels) != 1 { - t.Fatal("should return 1 channel") - } + if result := <-search(o1.TeamId, "blargh"); result.Err != nil { + t.Fatal(result.Err) + } else { + channels := result.Data.(*model.ChannelList) + if len(*channels) != 0 { + t.Fatal("should be empty") + } + } - if (*channels)[0].Name != o9.Name { - t.Fatal("wrong channel returned") - } - } + if result := <-search(o1.TeamId, "off-"); result.Err != nil { + t.Fatal(result.Err) + } else { + channels := result.Data.(*model.ChannelList) + if len(*channels) != 2 { + t.Fatal("should return 2 channels, not including private channel") + } + + if (*channels)[0].Name != o7.Name { + t.Fatal("wrong channel returned") + } + + if (*channels)[1].Name != o6.Name { + t.Fatal("wrong channel returned") + } + } - if result := <-ss.Channel().SearchInTeam(o1.TeamId, "the"); result.Err != nil { - t.Fatal(result.Err) - } else { - channels := result.Data.(*model.ChannelList) - t.Log(channels.ToJson()) - if len(*channels) != 1 { - t.Fatal("should return 1 channel") - } + if result := <-search(o1.TeamId, "off-topic"); result.Err != nil { + t.Fatal(result.Err) + } else { + channels := result.Data.(*model.ChannelList) + if len(*channels) != 1 { + t.Fatal("should return 1 channel") + } + + if (*channels)[0].Name != o6.Name { + t.Fatal("wrong channel returned") + } + } - if (*channels)[0].Name != o10.Name { - t.Fatal("wrong channel returned") - } - } + if result := <-search(o1.TeamId, "town square"); result.Err != nil { + t.Fatal(result.Err) + } else { + channels := result.Data.(*model.ChannelList) + if len(*channels) != 1 { + t.Fatal("should return 1 channel") + } + + if (*channels)[0].Name != o9.Name { + t.Fatal("wrong channel returned") + } + } - if result := <-ss.Channel().SearchInTeam(o1.TeamId, "Mobile"); result.Err != nil { - t.Fatal(result.Err) - } else { - channels := result.Data.(*model.ChannelList) - t.Log(channels.ToJson()) - if len(*channels) != 1 { - t.Fatal("should return 1 channel") - } + if result := <-search(o1.TeamId, "the"); result.Err != nil { + t.Fatal(result.Err) + } else { + channels := result.Data.(*model.ChannelList) + t.Log(channels.ToJson()) + if len(*channels) != 1 { + t.Fatal("should return 1 channel") + } + + if (*channels)[0].Name != o10.Name { + t.Fatal("wrong channel returned") + } + } - if (*channels)[0].Name != o11.Name { - t.Fatal("wrong channel returned") - } + if result := <-search(o1.TeamId, "Mobile"); result.Err != nil { + t.Fatal(result.Err) + } else { + channels := result.Data.(*model.ChannelList) + t.Log(channels.ToJson()) + if len(*channels) != 1 { + t.Fatal("should return 1 channel") + } + + if (*channels)[0].Name != o11.Name { + t.Fatal("wrong channel returned") + } + } + }) } } diff --git a/store/storetest/mocks/ChannelStore.go b/store/storetest/mocks/ChannelStore.go index c878d602b..5379c2fb4 100644 --- a/store/storetest/mocks/ChannelStore.go +++ b/store/storetest/mocks/ChannelStore.go @@ -45,6 +45,22 @@ func (_m *ChannelStore) AnalyticsTypeCount(teamId string, channelType string) st return r0 } +// AutocompleteInTeam provides a mock function with given fields: teamId, term +func (_m *ChannelStore) AutocompleteInTeam(teamId string, term string) store.StoreChannel { + ret := _m.Called(teamId, term) + + var r0 store.StoreChannel + if rf, ok := ret.Get(0).(func(string, string) store.StoreChannel); ok { + r0 = rf(teamId, term) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(store.StoreChannel) + } + } + + return r0 +} + // CreateDirectChannel provides a mock function with given fields: userId, otherUserId func (_m *ChannelStore) CreateDirectChannel(userId string, otherUserId string) store.StoreChannel { ret := _m.Called(userId, otherUserId) diff --git a/store/storetest/mocks/UserAccessTokenStore.go b/store/storetest/mocks/UserAccessTokenStore.go index b989fa1cc..c5ef0fefe 100644 --- a/store/storetest/mocks/UserAccessTokenStore.go +++ b/store/storetest/mocks/UserAccessTokenStore.go @@ -61,13 +61,13 @@ func (_m *UserAccessTokenStore) Get(tokenId string) store.StoreChannel { return r0 } -// GetAll provides a mock function with given fields: -func (_m *UserAccessTokenStore) GetAll(page int, perPage int) store.StoreChannel { - ret := _m.Called(page, perPage) +// GetAll provides a mock function with given fields: offset, limit +func (_m *UserAccessTokenStore) GetAll(offset int, limit int) store.StoreChannel { + ret := _m.Called(offset, limit) var r0 store.StoreChannel if rf, ok := ret.Get(0).(func(int, int) store.StoreChannel); ok { - r0 = rf(page, perPage) + r0 = rf(offset, limit) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(store.StoreChannel) @@ -109,13 +109,13 @@ func (_m *UserAccessTokenStore) GetByUser(userId string, page int, perPage int) return r0 } -// Search provides a mock function with given fields: -func (_m *UserAccessTokenStore) Search(term string) store.StoreChannel { - ret := _m.Called(term) +// Save provides a mock function with given fields: token +func (_m *UserAccessTokenStore) Save(token *model.UserAccessToken) store.StoreChannel { + ret := _m.Called(token) var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { - r0 = rf(term) + if rf, ok := ret.Get(0).(func(*model.UserAccessToken) store.StoreChannel); ok { + r0 = rf(token) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(store.StoreChannel) @@ -125,13 +125,13 @@ func (_m *UserAccessTokenStore) Search(term string) store.StoreChannel { return r0 } -// Save provides a mock function with given fields: token -func (_m *UserAccessTokenStore) Save(token *model.UserAccessToken) store.StoreChannel { - ret := _m.Called(token) +// Search provides a mock function with given fields: term +func (_m *UserAccessTokenStore) Search(term string) store.StoreChannel { + ret := _m.Called(term) var r0 store.StoreChannel - if rf, ok := ret.Get(0).(func(*model.UserAccessToken) store.StoreChannel); ok { - r0 = rf(token) + if rf, ok := ret.Get(0).(func(string) store.StoreChannel); ok { + r0 = rf(term) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(store.StoreChannel) -- cgit v1.2.3-1-g7c22