weixin_39630410
weixin_39630410
2020-12-29 22:08

dev/core#664 - CRM/Logging - Add indexes when updating log schema

Overview

The System.updatelogtables API makes it possible to update log table schemas based on the definitions set by the alterLogTables hook.

Note: This is built on top of https://github.com/civicrm/civicrm-core/pull/13441 and should be merged afterwards.

Before

When the alterLogTables hook defines a new index, it is only applied if the engine is also changed from the one currently used by the log tables.

After

New indexes are created regardless of the engine. https://lab.civicrm.org/dev/core/issues/664

该提问来源于开源项目:civicrm/civicrm-core

  • 点赞
  • 写回答
  • 关注问题
  • 收藏
  • 复制链接分享
  • 邀请回答

11条回答

  • weixin_39630410 weixin_39630410 4月前

    Rebased against master.

    I didn't realise we were copying over all indexes from the main table - I thought we were more selective

    I don't think we're doing that? If I'm reading it right, $tableSpec['indexes'] should hold only triggers set via hook_civicrm_alterLogTables.

    点赞 评论 复制链接分享
  • weixin_39851974 weixin_39851974 4月前

    ah my bad

    点赞 评论 复制链接分享
  • weixin_39851974 weixin_39851974 4月前

    I took another look at this & it makes sense & test cover is appropriate. Easy to read now there are not other changes in there.

    点赞 评论 复制链接分享
  • weixin_39851008 weixin_39851008 4月前

    (Standard links)

    点赞 评论 复制链接分享
  • weixin_39630410 weixin_39630410 4月前

    Test failure seems unrelated.

    点赞 评论 复制链接分享
  • weixin_39630410 weixin_39630410 4月前

    Seems I forgot to add the API test, added now.

    点赞 评论 复制链接分享
  • weixin_39889329 weixin_39889329 4月前

    Lots of the changes in this PR look like they are https://github.com/civicrm/civicrm-core/pull/13441 Is this PR having everything that was in 13441, but with a few more changes? Should we close that one, in favor of this PR?

    点赞 评论 复制链接分享
  • weixin_39630410 weixin_39630410 4月前

    I wanted to keep #13441 a bugfix-only thing, and this one builds on top of these fixes. This PR should merge cleanly if #13441 is merged first, but I'm fine with whatever option makes review easier.

    点赞 评论 复制链接分享
  • weixin_39630410 weixin_39630410 4月前

    One more slightly related fix in this PR: https://github.com/civicrm/civicrm-core/pull/13675

    Can merge with this one if it's easier.

    点赞 评论 复制链接分享
  • weixin_39630410 weixin_39630410 4月前

    resolved conflict.

    点赞 评论 复制链接分享
  • weixin_39851974 weixin_39851974 4月前

    maybe rebase this to keep it clean now I've merged the other one.

    I didn't realise we were copying over all indexes from the main table - I thought we were more selective

    点赞 评论 复制链接分享

相关推荐