[Suggestion] Add `guild_id` to `item_instance`

Suggestion:

Add a field guild_id to item_instance to facilitate data recovery / reorganization

Details:

Earlier this week I was looking into running some SQL to re-guid my item_instance table and realized:

[ul][li]guild_bank_item only relates to item_instance by item_guid[/li]
[li]owner_guid=0 for item_instance entries that are in guild_bank_item[/li][/ul]

Should guild_bank_item become corrupt there is no reliable way to recover the data. You can filter out AHBot entries in item_instance by comparing records with auctionhouse and you can also filter out entries owned by other players by comparing records with character_inventory, but you can’t determine which guild bank the remaining entries belong to.

Summary:

Adding a guild_id field to item_instance would allow server admins to reconstruct a corrupt table by querying the number of tabs owned by the guild (thereby getting the slot count) and inserting the rows from item_instance while incrementing the TabId and SlotId fields in guild_bank_item.

Naturally the result will leave the guild bank in a bit of disarray in game, but at least the items would be restored and the guild leader can resort everything.

No, you can’t protect every table in database like that. Server backups are what is used to protect database from corruption.

Also, how does that table get corrupted? If it is due to admin fault performing actions, then it’s his problem for not backing up table. If it’s a core bug that corrupts table somehow, then that should be fixed instead of making these changes.

No offense but this is very shortsighted, data corruption can happen due to a variety of unexpected reasons outside admin issues. Also, backups are not the only way to maintain data integrity and you know as well as I do that the average user here isn’t a professional DBA doing regular backups, assuming they have the storage for it in the first place. Why not allow another mechanism for people to reconstitute tables?

Consider too that other tables which use item_instance (character_inventory, auctionhouse, item_loot_items, etc) can be linked back to the item’s owner using either owner_id or creatorGuid and not just the item’s GUID, why should guild_bank_item be any different?

You are mistaken here, these fields exist for the purpose of storing values of ITEM_FIELD_OWNER/ITEM_FIELD_CREATOR to send them to client, they are NOT there to link item with its owner, that is the job of character_inventory

Right, I know that’s what character_inventory is for but as a bonus, they can be used to rebuild the character_inventory table if needed.

Consider this:

[ul][li]character_inventory gets corrupt, you can give a character back all its items[/li]If an item_instance entry is not in auctionhouse, guild_bank_item or mail_items then it belongs in character_inventory for that owner_guid

[/ul]

[li]auctionhouse gets corrupt, you can put all the auctions back[/li][ul]If an item_instance entry is not in character_inventory, guild_bank_item or mail_items then it belongs in the auctionhouse for that owner_guid

[/ul]

[li]mail_items gets corrupt, it can be rebuilt[/li][ul]If an item_instance entry is not in character_inventory, guild_bank_item or auctionhouse then it belongs in mail_items for that owner_guid

[/ul]

[li]guild_bank_item gets corrupt, you cannot put the items back[/li][ul]If an item_instance entry is not in character_inventory, auctionhouse or mail_items then it belongs in guild_bank_item except there’s no way to know which guild it belongs to

[/ul]

So all I’m saying is adding guild_id to the item_instance table offers a method of restoring a broken guild_bank_item table that all other inventory / item related tables enjoy. It also allows for an easy time running scripts to reguid the tables.

Setting aside the fact that good DB management requires backups, take a moment to consider that people running the auctionhouse bot are going to reach max GUID in their item_instance table faster than people who don’t.

Yes, it’s a large number and not likely to happen in weeks or months but it is still something that will happen. It’s much harder to reguid item_instance and then match the new guid to guild_bank_item since nothing links back to it besides the item itself.

I’m wondering how this change would improve a renumbering operation?

Presumably all of these tables operate from the guid in item_instance (I’ve not looked heavily into it).

So, a renumber would be as simple as creating a temporary table. Running something like

SET @NewID=0;
SELECT guid, @NewID:=@NewID+1 AS newguid FROM item_instance;

Into the temp table. Then, updating all of the table guids from the old to the new. It wouldn’t be any more efficient if you have the guild owner.

The only thing that would be aided is improving the location of a given item to handle single table failures as you describe. That’s a very specific scenario though, probably not worth adding an otherwise redundant field for.

I have to admit that this thread reminds me of another project and an issue that I’ve seen with it. Their response was “it’s a known bug with encoding software and we refuse to compensate for it” which is so blatantly stupid on their part. They know the problem. They CAN compensate for it, like LESS featured software, but the devs refuse to step beyond spec and compensate for known flaws. Exactly why, who knows. I refer to the project formerly known as XBMC (now being called Kodi, which I think is silly, but it’s not my project to name). I have a couple videos with a pixel aspect “bug” that forced a 16:9 video to be displayed in 4:3 aspect. VLC plays the video correctly, but lacks the convenient menu system for video selection. It’s sad to see a “better” app perform worse than a “lower” app because of developer arrogance.

Then, here I see a very valid feature request/suggestion with supporting facts/arguments. Rather than seeing the obvious conflict of other table doing exactly the same thing as the requested feature, the most responses are along the lines of “it’s not needed because you shouldn’t let anything bad happen without a recent backup”. Ok. Then REMOVE the redundant fields from the rest of the tables that DO have that redundant field. Don’t be hypocritical about the redundancy. Either be against all of it or support the request/suggestion for additional support of the same redundancy. It’s quite stupid to support something in all but 1 area and claim the admin is responsible for covering the remaining area. Do we have that much stupidity in this community? Take the blinders off and really look at something before bashing an idea.

@MrSmite: Having only myself and my brother on my own server that I tinker with (mostly for fun, like seeking out extra terrain that we couldn’t see on live servers), I wouldn’t have personal need for your idea. However, I think you were spot on with the request/suggestion. Amazingly, so many people are oblivious to the failures that can happen. Hard drives don’t work perfectly until they simply quit. Data corruption can/will happen, even when the drive works perfectly. In fact, I had a Win2k system eat it’s SYSTEM registry hive and the drive lasted close to 10 years after I repaired that corruption. The drive didn’t go bad. There was no power outage. I’d simply restarted after a system update was installed and it wouldn’t even boot to safe mode with the bad hive file. I know from first hand experience that data can/will simply go bye-bye, and that isn’t the only experience I’ve had in 15-20 years. Too bad there are so many clueless people that believe backups alone are enough, too. I even have a couple CDs that I burned and verified that wouldn’t even read back on the same system, in the same drive, a few days later. You’d think that you were asking to quadruple the size of the entire database by asking for a single field be added to a single table, if you go by the arguments against your suggestion.

+pete318

It’s not so much beneficial for renumbering, I probably should’ve just left that part out so it wouldn’t detract from the main goal of being able to rebuild a busted table.

I’ve had CDs and DVDs do that, very frustrating.

I don’t disagree that backups are important but even those are subject to the same pitfalls of user / hardware error. I just thought it was a “no brainer” to add a field to protect against data loss, in a similar manner that other tables have.

backups.

data doesn’t get corrupt on its own anyway, if it happens more often than backups then there’s a major issue.

if data gets corrupted then even any additional column you’ll add might be corrupted and unusable. if you know that some data in your db is corrupt, then you can’t trust a single bit in the whole db and you have to throw it away.

this looks more of an hackfix for the “reguid item_instance”, a better proper automatic solution should be implemented instead.

Again, not everyone is a professional DBA. Adding a field to assist in maintaining integrity is not unheard of.

Nobody said data gets corrupt on its own but there are many factors that can corrupt data on a hard drive, from power outages, bad sectors, virii, etc.

Incorrect. I’m talking about adding a column to a foreign table. If the guild_bank_item table gets corrupt, there’s absolutely no reason to think it would corrupt the entire database. MySQL stores each table in its own file so it’s quite feasible that one of those files could become corrupt while the rest of the database survives.

First, just because you disagree with a fix doesn’t make it a hack. Second, I already said that this wouldn’t have much benefit to a reguid script. It’s simply another mechanism to provide data integrity.

Explain then, why does (for example) character_inventory need to know who owns the item? Why did you take such great pains to make it part of the unique key? If item_instance already knows who owns it then it’s redundant data or in your words… a hack. Remember, character_inventory.item = item_instance.guid and item_instance already knows the owner_id.

So far I haven’t seen anyone give good reasons why it shouldn’t be done other than “we just don’t like it”. Simply yelling “backups” and ducking out of the room is not a really good rebuttal. You’re not dealing with enterprise level users so I don’t understand why you’d be against helping them, giving them just one more level of protection.

It would take less than a minute to make the change and exactly what would be the downside? It’s an honest question.

This isn’t exactly aimed at you, MrSmite. As I said, I don’t personally see a need for the change as there is and always will be only 1 guild on whatever server I run. I do, however, see the validity of the suggestion and the supporting facts for the change. The rest of this post is aimed more at the opposers to your suggestion.

CD/DVD media isn’t perfect. Magnetic media (external drives, both HD and tape) can develop errors (flaws in the magnetic surface, for example). Even flash storage can and will go bad after so many write cycles (they are ever increasing that, but there’s still a limit before cell failures occur). Anyone believing backups alone makes data secure is someone with extremely limited experience in the area of data collection, storage, and proper backups. In the brief time that I worked in the computer operations for a warehouse/outsourcing company, several sets of backup media was used in the process. Roughly a week’s worth of backups were kept before any of the media was being reused. For all I know, since I only worked on the weekends, the sets of backup media may have been swapped with other backup media during the week, too. Even there, a single backup wasn’t trusted, so several backups were being kept.

Also, there’s a reason why things like RAID5 and RAID10 exist. In an effort to be 100% reliable, and even this isn’t guaranteed to work 100% of the time, multiple backups of the same data need to be made and stored in different places. Not only is that inconvenient, it’s far more work than someone should need to do for a project like this one. This is not a commercial project. It is not something supported beyond personal testing/exploration. Individuals are not going to be so meticulous about backing up data if/when a failure strikes. So long a humans are involved in the production of an item, flaws/mistakes will happen. The best safeguard against flaws/mistakes is redundancy, as can be seen by the invention of RAID specifications for hard drive storage.

Those of you still believing that a single field being added to a single table (that would take less effort to add than it has been to debate the idea) really need to take the blinders off and look at the big picture rather than the limited view they’ve been looking at.

I’ll look at the bigger picture and say that nobody wants redundancy in their DB http://en.wikipedia.org/wiki/Data_redundancy http://en.wikipedia.org/wiki/Database_normalization

I might as well remove* the other fields like owner in character_inventory since item_instance already has that info…

You have no idea how ridiculous you sound. Hard drives do not have the concept of files, they work in sectors.

item_instance is one of the biggest tables. A new field (that we do not need) does change the performance of this table.

If your database gets corrupted, get some backups ready. If your backups were also corrupted, those were not backups (hey… don’t store them in the same physical place).

Feel free to add this field to your own database (after all, TC is open source and each one of us can have a slightly different code base) but this will not be added to the main repo.

Closing links: http://en.wikipedia.org/wiki/Parkinson%27s_law_of_triviality

* I won’t but Pull Requests are welcome.

well you are against all the reasons I wrote so ofc you don’t see any good one.

summing up what you wrote, these 2 columns are useful to this particular case:

  • someone who doesn’t know how to do backups BUT knows how to write MySQL queries joining multiple tables and also has quite advanced knowledge about TrinityCore Character Database structure

  • a particular hardware issue that corrupts only a particular column of a particular table without affecting any other column in the same table and any other table in character/auth databases. Due to how tables are stored, any hardware issue will most likely affect more than a single column; you mentioned bad sector, a bad sector will surely not just affect that particular column but more columns(even more rows).

with these 2 particular conditions required to make the 2 new columns somehow useful (if you still trust a database with corrupted data), then it’s very clear to me it’s not worth to even consider to apply the change. most important you don’t build your application to fit your disaster recovery plan, it’s the other way around.

on a unrelated note, the definition of hackfix refers to the fact that manually executing a script to reguid item_instance table is only a temporary solution that will need to be executed over and over. A check on startup that would automatically reguid the table (maybe with the option to disable it in configs) seems to be a better solution.

You have no idea how ridiculous you sound. Hard drives do not have the concept of files, they work in sectors.

First, I’m not sure why you of all people resort to insults usually you’re more level-headed. You may not agree with the suggestion but I’m far from being ridiculous.

Second, MySQL stores each table in its own file on the disk. If the hard drive suddenly has a sector (or several) fail at the location of the file that holds the guild_bank_item table, there’s no reason to think that it would corrupt the whole characters databse. That’s what I meant in case it wasn’t clear.

Also, your link about data redundancy can be easily overcome by using cascaded writes. You can’t have inconsistent data if every change that’s made to guild_id in one table is automatically made in child tables (not to mention Trinity already breaks most of the rules you posted re: redundancy and normalization).

Well not really, I’m not against what you wrote but you wrote opinion and ours differ. You didn’t supply any technical reason why it would be a bad idea.

I don’t follow you, or maybe you don’t understand me.

On my drive I have a file named: guild_bank_item.frm which represents the MySQL table: guild_bank_item. If, for whatever reason this file becomes corrupt (hardware failure, power loss during write operation, etc.) and MySQL can no longer access it (the table will still exist in the schema because that’s stored as a separate physical file), you can dump the table from the database and rebuild it.

Past mistakes in the code are not a valid reason to keep doing them.

Thanks for your suggestion, but adding guild_id to item_instance would increase the database redundancy, which is not desired (see posts above).

Have a good evening.