Page MenuHomePhabricator

Drop unused rc_cur_time field from recentchanges table in version 1.24
Closed, ResolvedPublic

Description

From tables.sql:

  • This is no longer used rc_cur_time varbinary(14) NOT NULL default '',

So please clean up and remove it. Thanks.


Version: 1.20.x
Severity: normal

Details

Reference
bz40667

Event Timeline

bzimport raised the priority of this task from to Low.
bzimport set Reference to bz40667.

Commited gerrit 35659

(In reply to comment #1)

Commited Gerrit change #35659

No longer support for this commit by me.

Change 42278 restored by Umherirrender:
(bug 40667) deprecate rc_cur_time of recentchanges

Reason:
Column is still unused and should dropped same day.
Start with removing it from sql statements.

https://gerrit.wikimedia.org/r/42278

Change 42278 had a related patch set uploaded by Umherirrender:
Deprecate rc_cur_time of recentchanges

https://gerrit.wikimedia.org/r/42278

Is there a bug tracking dropping database columns from MediaWiki core?

Change 42278 merged by jenkins-bot:
Remove recentchanges.rc_cur_time from sql statements

https://gerrit.wikimedia.org/r/42278

Field was removed from sql statements.

Milestone is now 1.24

Keeping the field until 1.24 allows easier rollback on version update from 1.22 -> 1.23, when something gets wrong with the new version.

saper added a comment.Dec 4 2013, 10:58 AM

I am now getting

Query: INSERT INTO "recentchanges" (rc_timestamp,rc_namespace,rc_title,rc_type,rc_source,rc_minor,rc_cur_id,rc_user,rc_user_text,rc_comment,rc_this_oldid,rc_last_oldid,rc_bot,rc_ip,rc_patrolled,rc_new,rc_old_len,rc_new_len,rc_deleted,rc_logid,rc_log_type,rc_log_action,rc_params,rc_id) VALUES ('2013-12-04 10:54:26 GMT','0','Main_Page','1','mw.new','0','1','0','MediaWiki default',,'1','0','0','127.0.0.1','0','1','0','524','0','0',NULL,,,'1')
Function: RecentChange::save
Error: 23502 ERROR: null value in column "rc_cur_time" violates not-null constraint

when installing git master (now at 0a54e70cdd2162be4b331aacb79ad5a3926dd2c6) on PostgreSQL.

saper added a comment.Dec 4 2013, 11:19 AM

Submitted gerrit change 99068 to revert this for now.

saper added a comment.Dec 5 2013, 10:59 AM

To reiterate what I said in the code review for gerrit change 99068:

I think that the commit should remove the field from both SQL as well as all table definitions and add updater definitions for three major databases as you did in the original gerrit change. WMF is using neither installer nor updater so I believe they can live with it.

This way was used to avoid the third party updaters to restore his database,
when he have to downgrade (or roll back the upgrade).

Change 125769 had a related patch set uploaded by Umherirrender:
Drop rc_cur_time from recentchanges

https://gerrit.wikimedia.org/r/125769

Change 131852 had a related patch set uploaded by EBernhardson:
Stop populating rc_cur_time

https://gerrit.wikimedia.org/r/131852

Change 131852 merged by jenkins-bot:
Stop populating rc_cur_time

https://gerrit.wikimedia.org/r/131852

Change 125769 merged by jenkins-bot:
Drop rc_cur_time from recentchanges

https://gerrit.wikimedia.org/r/125769

Field is now dropped in master