Improve action table indices

[zeripath]

Improve the indices on the action table by creating a covering index that covers the
common queries and removes unhelpful indices.

Fix #16665

Signed-off-by: Andrew Thornton art27@cantab.net

[zeripath]

Ideally we might want to wait till some functionality that would allow us to assert the ordering of the index entries eg. https://gitea.com/xorm/xorm/pulls/2137 is merged and xorm is updated

[lunny]

Is there any scene to use user_id as an index? On user's profile and activies page?

[delvh]

@lunny If I see that correctly, notifications?

[lunny]

@lunny If I see that correctly, notifications?

So the first column of a composite index will become a standalone index column?

[delvh]

If I had to guess how the database query optimizers are built, I'd say so.
But perhaps @wxiaoguang can clarify it for us.

[zeripath]

So the first column of a composite index will become a standalone index column?

Yes an index on (A, B, C, D) can provide indexes for (A), (A,B), (A,B,C) and (A,B,C,D).

Is there any scene to use user_id as an index? On user's profile and activies page?

If instead of having, INDEX(u_ua_and_r) INDEX(ua_and_r) INDEX(r) we could reorder things so we could just have one or two indices.

See #16665 (comment):

1. Drop the is_deleted, is_private and created_unix indices - they are useless
2. At the minimum we should add:

• act_user_id repo_id user_id (folded into one of the below)
• repo_id user_id
• act_user_id repo_id

3. Consider adding in:

• act_user_id repo_id user_id is_deleted

4. We could consider adding further composites with is_deleted added in to the above but it will depend on how frequent is_deleted and I think we would need to actually do some live testing on this.
5. We always sort by created_unix which is a good reason to include the created_unix in these proposed indices so always add these in somewhere

One we have the OrderBy fix in we can change these indices to be:

1. ( act_user_id, repo_id, created_unix, user_id) (consider including is_deleted here too)
2. ( repo_id, created_unix, user_id)

But we'd need to check again on MySQL to ensure the position of created_unix is correct. It'd be helpful to check on postgres too.

[lunny]

So the first column of a composite index will become a standalone index column?

Yes an index on (A, B, C, D) can provide indexes for (A), (A,B), (A,B,C) and (A,B,C,D).

Is there any scene to use user_id as an index? On user's profile and activies page?

If instead of having, INDEX(u_ua_and_r) INDEX(ua_and_r) INDEX(r) we could reorder things so we could just have one or two indices.

See #16665 (comment):

1. Drop the is_deleted, is_private and created_unix indices - they are useless
2. At the minimum we should add:

• act_user_id repo_id user_id (folded into one of the below)
• repo_id user_id
• act_user_id repo_id

3. Consider adding in:

• act_user_id repo_id user_id is_deleted

4. We could consider adding further composites with is_deleted added in to the above but it will depend on how frequent is_deleted and I think we would need to actually do some live testing on this.
5. We always sort by created_unix which is a good reason to include the created_unix in these proposed indices so always add these in somewhere

One we have the OrderBy fix in we can change these indices to be:

1. ( act_user_id, repo_id, created_unix, user_id) (consider including is_deleted here too)
2. ( repo_id, created_unix, user_id)

But we'd need to check again on MySQL to ensure the position of created_unix is correct. It'd be helpful to check on postgres too.

Wow, so let's wait #19849

[lunny]

Since #19874 merged, please resolve the conflicts.

[zeripath]

I think we should try to get this in to 1.17

[wxiaoguang]

LGTM since it was scheduled in 1.17 before frozen

[lunny]

I don't know whether a private struct could work with xorm.

[zeripath]

I'm not sure if you're meaning this as a comment, e.g. I didn't know that a private struct would work with xorm, or if you're trying to express doubt that this will work, e.g. I don't think a private struct will work with xorm.

It will work.

If you are given a pointer to a nonExportedStruct:

package someplace

type notExportedStruct {
   Exported string
}

func NewNotExported() *notExportedStruct {
  return &notExportedStruct{}
}

you can always create another copy of it using:

package main

import "someplace"

func main() {
  notExported := somplace.NewNotExported()

	val := reflect.ValueOf(notExported).Elem()
	typ := val.Type()

	newNotExportedVal := reflect.New(typ)
  newNotExportedVal.Elem().FieldByName("Exported").SetString("I have been set by reflection")

	fmt.Println(newNotExportedVal.Interface())
}

Which is what xorm does internally anyway.

Xorm doesn't need to know or be able to cast to the non-exported type - it just uses reflection to look across the exported fields which will always work or casts to interfaces it knows e.g. TableName. newNotExportedVal can be cast to an interface{} using the .Interface() function as normal.