* Replace SELECT * with explicit column lists in channel store

Migrates channel_store.go away from SELECT * patterns to explicit column
lists for better performance, maintainability, and schema safety.

- Replace GetPinnedPosts raw SQL with query builder using postSliceColumns()
- Replace "cc.*" in group channel search with channelSliceColumns()
- Replace GetChannelsBatchForIndexing raw SQL with query builder
- Replace channel member and team queries with respective column helpers
- Use SelectBuilder helper instead of manual ToSql() calls

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

* Replace SELECT * with COUNT(*) in user_test.go

Replaces unnecessary SELECT * queries with SELECT COUNT(*) in
TestPermanentDeleteUser bot count verification. Only needs to check
the count of bots, not retrieve full bot records.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

---------

Co-authored-by: Claude <noreply@anthropic.com>
Этот коммит содержится в:
Jesse Hallam
2025-07-02 12:35:54 -03:00
коммит произвёл GitHub
родитель fb9b05b764
Коммит ebe03c1d45
3 изменённых файлов: 63 добавлений и 34 удалений

Просмотреть файл

@@ -1248,12 +1248,12 @@ func TestPermanentDeleteUser(t *testing.T) {
}) })
assert.Nil(t, err) assert.Nil(t, err)
bots1 := []*model.Bot{} var botCount1 int
bots2 := []*model.Bot{} var botCount2 int
err1 := th.SQLStore.GetMaster().Select(&bots1, "SELECT * FROM Bots") err1 := th.SQLStore.GetMaster().Get(&botCount1, "SELECT COUNT(*) FROM Bots")
assert.NoError(t, err1) assert.NoError(t, err1)
assert.Equal(t, 1, len(bots1)) assert.Equal(t, 1, botCount1)
// test that bot is deleted from bots table // test that bot is deleted from bots table
retUser1, err := th.App.GetUser(bot.UserId) retUser1, err := th.App.GetUser(bot.UserId)
@@ -1262,9 +1262,9 @@ func TestPermanentDeleteUser(t *testing.T) {
err = th.App.PermanentDeleteUser(th.Context, retUser1) err = th.App.PermanentDeleteUser(th.Context, retUser1)
assert.Nil(t, err) assert.Nil(t, err)
err1 = th.SQLStore.GetMaster().Select(&bots2, "SELECT * FROM Bots") err1 = th.SQLStore.GetMaster().Get(&botCount2, "SELECT COUNT(*) FROM Bots")
assert.NoError(t, err1) assert.NoError(t, err1)
assert.Equal(t, 0, len(bots2)) assert.Equal(t, 0, botCount2)
scheduledPost1 := &model.ScheduledPost{ scheduledPost1 := &model.ScheduledPost{
Draft: model.Draft{ Draft: model.Draft{

Просмотреть файл

@@ -199,7 +199,8 @@ func channelMemberToSlice(member *model.ChannelMember) []any {
type channelMemberWithSchemeRolesList []channelMemberWithSchemeRoles type channelMemberWithSchemeRolesList []channelMemberWithSchemeRoles
func getChannelRoles(schemeGuest, schemeUser, schemeAdmin bool, defaultTeamGuestRole, defaultTeamUserRole, defaultTeamAdminRole, defaultChannelGuestRole, defaultChannelUserRole, defaultChannelAdminRole string, func getChannelRoles(schemeGuest, schemeUser, schemeAdmin bool, defaultTeamGuestRole, defaultTeamUserRole, defaultTeamAdminRole, defaultChannelGuestRole, defaultChannelUserRole, defaultChannelAdminRole string,
roles []string) rolesInfo { roles []string,
) rolesInfo {
result := rolesInfo{ result := rolesInfo{
roles: []string{}, roles: []string{},
explicitRoles: []string{}, explicitRoles: []string{},
@@ -761,13 +762,11 @@ func (s SqlChannelStore) saveChannelT(transaction *sqlxTxWrapper, channel *model
} }
insertResult, err := transaction.Exec(query, params...) insertResult, err := transaction.Exec(query, params...)
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "save_channel: id=%s", channel.Id) return nil, errors.Wrapf(err, "save_channel: id=%s", channel.Id)
} }
rowAffected, err := insertResult.RowsAffected() rowAffected, err := insertResult.RowsAffected()
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "save_channel: id=%s", channel.Id) return nil, errors.Wrapf(err, "save_channel: id=%s", channel.Id)
} }
@@ -874,7 +873,6 @@ func (s SqlChannelStore) GetChannelUnread(channelId, userId string) (*model.Chan
AND UserId = ? AND UserId = ?
AND DeleteAt = 0`, AND DeleteAt = 0`,
channelId, userId) channelId, userId)
if err != nil { if err != nil {
if err == sql.ErrNoRows { if err == sql.ErrNoRows {
return nil, store.NewErrNotFound("Channel", fmt.Sprintf("channelId=%s,userId=%s", channelId, userId)) return nil, store.NewErrNotFound("Channel", fmt.Sprintf("channelId=%s,userId=%s", channelId, userId))
@@ -895,8 +893,19 @@ func (s SqlChannelStore) InvalidateChannelByName(teamId, name string) {
func (s SqlChannelStore) GetPinnedPosts(channelId string) (*model.PostList, error) { func (s SqlChannelStore) GetPinnedPosts(channelId string) (*model.PostList, error) {
pl := model.NewPostList() pl := model.NewPostList()
query := s.getQueryBuilder().
Select(postSliceColumns()...).
Column("(SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyCount").
From("Posts p").
Where(sq.Eq{
"IsPinned": true,
"ChannelId": channelId,
"DeleteAt": 0,
}).
OrderBy("CreateAt ASC")
posts := []*model.Post{} posts := []*model.Post{}
if err := s.GetReplica().Select(&posts, "SELECT *, (SELECT count(Posts.Id) FROM Posts WHERE Posts.RootId = (CASE WHEN p.RootId = '' THEN p.Id ELSE p.RootId END) AND Posts.DeleteAt = 0) as ReplyCount FROM Posts p WHERE IsPinned = true AND ChannelId = ? AND DeleteAt = 0 ORDER BY CreateAt ASC", channelId); err != nil { if err := s.GetReplica().SelectBuilder(&posts, query); err != nil {
return nil, errors.Wrap(err, "failed to find Posts") return nil, errors.Wrap(err, "failed to find Posts")
} }
for _, post := range posts { for _, post := range posts {
@@ -1459,7 +1468,6 @@ func (s SqlChannelStore) GetChannelCounts(teamId string, userId string) (*model.
AND (TeamId = ? OR TeamId = '') AND (TeamId = ? OR TeamId = '')
AND DeleteAt = 0 AND DeleteAt = 0
ORDER BY DisplayName`, userId, teamId) ORDER BY DisplayName`, userId, teamId)
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "failed to get channels count with teamId=%s and userId=%s", teamId, userId) return nil, errors.Wrapf(err, "failed to get channels count with teamId=%s and userId=%s", teamId, userId)
} }
@@ -2116,7 +2124,6 @@ func (s SqlChannelStore) GetChannelMembersTimezones(channelId string) ([]model.S
Users ON ChannelMembers.UserId = Id Users ON ChannelMembers.UserId = Id
WHERE ChannelId = ? WHERE ChannelId = ?
`, channelId) `, channelId)
if err != nil { if err != nil {
return nil, errors.Wrapf(err, "failed to find user timezones for users in channels with channelId=%s", channelId) return nil, errors.Wrapf(err, "failed to find user timezones for users in channels with channelId=%s", channelId)
} }
@@ -2514,7 +2521,6 @@ func (s SqlChannelStore) GetPinnedPostCount(channelId string, allowFromCache boo
IsPinned = true IsPinned = true
AND ChannelId = ? AND ChannelId = ?
AND DeleteAt = 0`, channelId) AND DeleteAt = 0`, channelId)
if err != nil { if err != nil {
return 0, errors.Wrapf(err, "failed to count pinned Posts with channelId=%s", channelId) return 0, errors.Wrapf(err, "failed to count pinned Posts with channelId=%s", channelId)
} }
@@ -2683,7 +2689,7 @@ func (s SqlChannelStore) UpdateLastViewedAt(channelIds []string, userId string)
return times, nil return times, nil
} }
var msgCountQuery, msgCountQueryRoot, lastViewedQuery = sq.Case("ChannelId"), sq.Case("ChannelId"), sq.Case("ChannelId") msgCountQuery, msgCountQueryRoot, lastViewedQuery := sq.Case("ChannelId"), sq.Case("ChannelId"), sq.Case("ChannelId")
for _, t := range lastPostAtTimes { for _, t := range lastPostAtTimes {
times[t.Id] = t.LastPostAt times[t.Id] = t.LastPostAt
@@ -2904,7 +2910,6 @@ func (s SqlChannelStore) IncrementMentionCount(channelId string, userIDs []strin
"ChannelId": channelId, "ChannelId": channelId,
}). }).
ToSql() ToSql()
if err != nil { if err != nil {
return errors.Wrap(err, "IncrementMentionCount_Tosql") return errors.Wrap(err, "IncrementMentionCount_Tosql")
} }
@@ -3888,7 +3893,7 @@ func (s SqlChannelStore) searchGroupChannelsQuery(userId, term string, isPostgre
}). }).
GroupBy("c.Id") GroupBy("c.Id")
return s.getQueryBuilder().Select("cc.*"). return s.getQueryBuilder().Select(channelSliceColumns(true, "cc")...).
FromSelect(cc, "cc"). FromSelect(cc, "cc").
Join("ChannelMembers cm on cc.Id = cm.ChannelId"). Join("ChannelMembers cm on cc.Id = cm.ChannelId").
Join("Users u on u.Id = cm.UserId"). Join("Users u on u.Id = cm.UserId").
@@ -4338,22 +4343,21 @@ func (s SqlChannelStore) GetAllDirectChannelsForExportAfter(limit int, afterId s
} }
func (s SqlChannelStore) GetChannelsBatchForIndexing(startTime int64, startChannelID string, limit int) ([]*model.Channel, error) { func (s SqlChannelStore) GetChannelsBatchForIndexing(startTime int64, startChannelID string, limit int) ([]*model.Channel, error) {
query := query := s.getQueryBuilder().
`SELECT Select(channelSliceColumns(false)...).
* From("Channels").
FROM Where(sq.Or{
Channels sq.Gt{"CreateAt": startTime},
WHERE sq.And{
CreateAt > ? sq.Eq{"CreateAt": startTime},
OR sq.Gt{"Id": startChannelID},
(CreateAt = ? AND Id > ?) },
ORDER BY }).
CreateAt ASC, Id ASC OrderBy("CreateAt ASC", "Id ASC").
LIMIT Limit(uint64(limit))
?`
channels := []*model.Channel{} channels := []*model.Channel{}
err := s.GetSearchReplicaX().Select(&channels, query, startTime, startTime, startChannelID, limit) err := s.GetSearchReplicaX().SelectBuilder(&channels, query)
if err != nil { if err != nil {
return nil, errors.Wrap(err, "failed to find Channels") return nil, errors.Wrap(err, "failed to find Channels")
} }
@@ -4398,7 +4402,7 @@ func (s SqlChannelStore) UpdateMembersRole(channelID string, adminIDs []string)
// A SELECT and a UPDATE query are needed. // A SELECT and a UPDATE query are needed.
// Once we only support PostgreSQL, this can be done in a single query using RETURNING. // Once we only support PostgreSQL, this can be done in a single query using RETURNING.
query, args, err := s.getQueryBuilder(). query, args, err := s.getQueryBuilder().
Select("*"). Select(channelMemberSliceColumns()...).
From("ChannelMembers"). From("ChannelMembers").
Where(sq.Eq{"ChannelID": channelID}). Where(sq.Eq{"ChannelID": channelID}).
Where(sq.Or{sq.Eq{"SchemeGuest": false}, sq.Expr("SchemeGuest IS NULL")}). Where(sq.Or{sq.Eq{"SchemeGuest": false}, sq.Expr("SchemeGuest IS NULL")}).
@@ -4507,7 +4511,7 @@ func (s SqlChannelStore) GetTeamForChannel(channelID string) (*model.Team, error
return nil, errors.Wrap(err, "get_team_for_channel_nested_tosql") return nil, errors.Wrap(err, "get_team_for_channel_nested_tosql")
} }
query, args, err := s.getQueryBuilder(). query, args, err := s.getQueryBuilder().
Select("*"). Select(teamSliceColumns()...).
From("Teams").Where(sq.Expr("Id = ("+nestedQ+")", nestedArgs...)).ToSql() From("Teams").Where(sq.Expr("Id = ("+nestedQ+")", nestedArgs...)).ToSql()
if err != nil { if err != nil {
return nil, errors.Wrap(err, "get_team_for_channel_tosql") return nil, errors.Wrap(err, "get_team_for_channel_tosql")

Просмотреть файл

@@ -7076,7 +7076,7 @@ func testChannelStoreSearchGroupChannels(t *testing.T, rctx request.CTX, ss stor
require.NoError(t, nErr) require.NoError(t, nErr)
for _, userID := range userIds { for _, userID := range userIds {
_, err := ss.Channel().SaveMember(rctx, &model.ChannelMember{ _, err = ss.Channel().SaveMember(rctx, &model.ChannelMember{
ChannelId: gc2.Id, ChannelId: gc2.Id,
UserId: userID, UserId: userID,
NotifyProps: model.GetDefaultChannelNotifyProps(), NotifyProps: model.GetDefaultChannelNotifyProps(),
@@ -7092,6 +7092,20 @@ func testChannelStoreSearchGroupChannels(t *testing.T, rctx request.CTX, ss stor
_, nErr = ss.Channel().Save(rctx, &gc3, -1) _, nErr = ss.Channel().Save(rctx, &gc3, -1)
require.NoError(t, nErr) require.NoError(t, nErr)
// Make gc3 policy enforced
_, err = ss.AccessControlPolicy().Save(rctx, &model.AccessControlPolicy{
ID: gc3.Id,
Version: model.AccessControlPolicyVersionV0_1,
Type: model.AccessControlPolicyTypeChannel,
Rules: []model.AccessControlPolicyRule{
{
Actions: []string{},
Expression: "",
},
},
})
require.NoError(t, err)
for _, userID := range userIds { for _, userID := range userIds {
_, err := ss.Channel().SaveMember(rctx, &model.ChannelMember{ _, err := ss.Channel().SaveMember(rctx, &model.ChannelMember{
ChannelId: gc3.Id, ChannelId: gc3.Id,
@@ -7108,6 +7122,16 @@ func testChannelStoreSearchGroupChannels(t *testing.T, rctx request.CTX, ss stor
} }
}() }()
// assertChannelListPopulated verifies that the channel objects in the given channel list
// are fully populated.
assertChannelListPopulated := func(t *testing.T, channelList model.ChannelList) {
for _, actualChannel := range channelList {
expectedChannel, err := ss.Channel().Get(actualChannel.Id, false)
require.NoError(t, err)
assert.Equal(t, expectedChannel, actualChannel, "channel %q in channel list missing metadata", actualChannel.Id)
}
}
testCases := []struct { testCases := []struct {
Name string Name string
UserID string UserID string
@@ -7169,6 +7193,7 @@ func testChannelStoreSearchGroupChannels(t *testing.T, rctx request.CTX, ss stor
} }
require.ElementsMatch(t, tc.ExpectedResult, resultIds) require.ElementsMatch(t, tc.ExpectedResult, resultIds)
assertChannelListPopulated(t, result)
}) })
} }
} }