I was working on this feature for the custom-wizards plugin and I wanted to fetch all the wizards.
The key used is a string which is unique per wizard. This case was handled by fetching all the rows for the plugin and filtering them based on a for the value which is fine for small number of records but not ideal for a big plugin. It looks like plugin_store_rows is meant to store settings-like data for the plugin.
I personally think, this would open the idea of storing non-settings like data in the plugin_store_rows table.
The main reason the pattern is used is because the plugin_store_rows table is used by multiple plugins (and some core services), so the identifying columns, i.e. id and plugin_name, can’t be used for identification internally within each system using the PluginStore. So a string-based system is used in the key column instead.
In terms of changing the database structure from within a plugin, @gdpelican has a good post on this:
Personally, I’m still quite wary of doing this as working on a third party plugin you have no control over namespacing, whether your plugin is removed and what potentially conflicting changes are made to core Discourse.
As @gdpelican mentions, you need to provide a way for your plugin user to remove the db changes if they uninstall the plugin.
Provide a method for users to clean up your database changes if they don’t want your plugin anymore. I did it with a rake task.
I feel this is too in the weeds for most plugin users and poses a risk if they’re not aware of this detail.
Moreover, I haven’t found a real need to go outside the bounds of the PluginStore and CustomFields yet.
All that said, personally I’d be in favour of a new method along these lines in PluginStore, as I find the pattern useful.
@david Would be interested in your thoughts on the above as well.
As @eviltrout has said, we’re using migrations and dedicated tables in a number of plugins now, with great success. Having the ability to enforce database constraints has helped improve performance (lookups in any column) and data integrity (through unique indexes). These two things have proved especially important at the scale of some of our hosted customers - not really something I considered before joining the team.
The first substantial plugin I worked on was chat-integration, and I implemented a very fiddly “fake activerecord”, which leans on the plugin store. In hindsight, dedicated tables would have been a far better choice, and I might look at migrating the plugin to that in future.
I would agree, when it comes to modifying core tables. Adding/modifying columns on existing tables could have unintended consequences later down the line, and will stick around even if the plugin is uninstalled. I would strongly recommend against doing this.
Dedicated tables, on the other hand, are fairly low risk. If the plugin is uninstalled, they will just stick around, without any negative side effects (as long as you don’t introduce any foreign key constraints). Leaving data lying around is “no worse” than using the plugin store.
In terms of cleanup, we could look at providing rake tasks which “reverse” plugin migrations to cleanup. But to be honest, I don’t think this will see much use. My assumption is that people rarely uninstall plugins, and when they do, they would rather keep the data around in case they want to reinstall it again.
As the author of that code I should maybe clarify it a little bit. The priority ids are constants and not relational data. I would absolutely recommend against using something_id when the ids are not known in advance. In this case each priority is thought of as a singleton, and I figured that any table I’d create would essentially be a clone of the PluginStore anyway!