Add migrate destination for sql table

importing to a standalone table is a common task

IMO, if you're doing that in Drupal 8 then you're doing it wrong. The only reason would be if you had a table that wasn't managed by Drupal.

I think this should go in contrib, we don't want to encourage people to migrate directly into Drupal 8 tables, otherwise we'll end up seeing weird bug reports for people who circumvented API's.

I hope to test this patch later today, but I'm excited about it as I have some Drupal 7 modules that I need to migrate to Drupal 8. I don't have to transform their database tables; I just have to copy them from the Drupal 7 database to the Drupal 8 database "as is". I basically want to do INSERT INTO d8database.mytable SELECT * FROM d7database.mytable after creating the table, or mysqldump --user=user1 --password=password1 database1 table1 | mysql --user=user2 --password=password2 database2 in a few lines of code/configuration. While I could use either of these commands, I'd prefer to integrate this in my migrate process/workflow so now manual steps are required. This improves testability, reduces potential errors, etc.

@benjy, I think it would be ok to put this in migrate_plus, however, I can also see this being a very common use case for core. In the future, we hopefully won't change every single database table in core, and core itself will probably have to copy tables from one database to another. I believe it is a common task in contrib today, and it will hopefully become a more common task for core as well.

@hussainweb: I'm not an expert on the migration system, but can't the migration system assume there is a _schema() for each table? If so, can't we omit having to specify id_fields? I believe that would improve the developer experience significantly. I feel the ideal developer experience would be this:

destination:
  plugin: table
  table_name: (table name)

@Dries, there is a pretty serious problem with bypassing API's, they handle how the data is to be saved it's Drupal's opportunity to normalise the data, make changes, set changed timestamps etc etc. If we start offering ways to bypass that we'll find weird and wonderful bug reports from people that get their database into unusual situations.

I just have to copy them from the Drupal 7 database to the Drupal 8 database "as is"

I can't support this issue, I see it offering an easy way out for your current situation but to be frank, if you have contrib modules that require that then they're doing something wrong. Everyone should now be using some form of config/entity API to store their data and we have destinations for those.

I discussed this issue with @benjy in IRC.

The pros of adding this to core are:

• Not everything in custom and contrib has well defined storage APIs
• We still hook_schema in core - it is perfectly possible to manage data outside of the entity or config APIs

The cons are:

• core doesn't need it - and if it did then we'd be doing it wrong because it would mean we have a storage layer with no API
• there are reasonable concerns about supporting this type of plugin and making it a core option - this could encourage people to write migrations that make assumptions they shouldn't

There also is the argument that it is not migrate's job to police contrib and customs API. Yes it is bad practice for a migrate destination to not use an API - but so what?

I think, however, there might be a reasonable middle way. We could add the destination to the migrate_plus module. This way contrib and custom could have access to it and if it proves something most migrations need then we can move it into core at a later point.

@benjy You might be 100% right. I don't understand what you mean with "bypassing APIs", for example. In my case, I'm managing a list of images in a database table. Are you implying that this should be managed through the entity system / entity storage system, for example? If so, I don't agree with that.

This is a set of stand-alone, isolated tables managed by its own data management system in my custom Drupal. Maybe you are saying that I should re-create the table in Drupal 8 through my module's custom data management APIs? For example, re-save each of the images through my Image.class' API? I could do that but it is not necessary as I know the tables haven't changed between Drupal 7 and Drupal 8, and no modules (can) plug into the data management system to alter its behavior. I can see how we might not be able to make that assumption in core.

Managing a table on your own is an advanced use case and the Table destination works for you because it is custom code, you know the schema hasn't changed.

You should be providing a simple abstraction on-top of writing to that table (sounds like your image.class), you wouldn't want other modules writing to your images table you'd want them using your abstraction.

The closest example in core is the url_alias table which has Drupal\Core\Path\AliasStorage as a thin layer on-top of db reads/writes. And migrate provides a specific destination Drupal\path\Plugin\migrate\destination\UrlAlias to give that abstraction a chance to act before the data is saved. This is important because 3rd party code could be listening to events that happen in your storage abstraction, maybe to track stats for example. If we bypass the storage and write straight to the db, the other code is broken.

You control all the moving parts so you can make assumptions to bypass your own API but we can't do that in core or contrib which is why I don't want to encourage this approach.

I'm OK with this going into migrate_plus although mikeryan has the final say on that.

@benjy That all makes sense. I agree it belongs in migrate_plus. Thanks for the explanation.

I have not posted a word yesterday but after thinking on it a lot, I feel I need to:

> You might be 100% right. I don't understand what you mean with "bypassing APIs", for example. In my case, I'm managing a list of images in a database table. Are you implying that this should be managed through the entity system / entity storage system, for example? If so, I don't agree with that.

If the Drupal project lead doesn't agree in this then the field/entity team of which I have been a part of have failed and a significant portion of the field/entity work in the last eight years is brought to question. This is a very sad day. Of course you could say I am biased because I alone pursue alternative database backends but that is not so -- for one, I haven't been working on MongoDB for more than a year, two, the point is encapsulation and backwards and forward compatibility. If you maintain your own table with your own code then all the improvements, be it performance, multilingual, security, migration et al will fall on your shoulders and then why do core and contrib contributors bother with really complicated code covering a lot, hopefully (almost) all use cases? If you want to add new functionality you need to write it because most of contrib will work with the entity system. Or that is the hope. But if the project lead encourages people here and on Twitter to just shovel shit into whatever table you can find then all the advocacy to kill that is for naught. I have been personally speaking several times against that very thing since, I can't even remember, but some time 2010 at least perhaps even earlier. Why bother? I struggle with typed data perhaps more than most because I poke at it on lower levels than ordinary uses and I put up with it because there's the hope that a strong ecosystem will form around the Entity API and that will benefit us all. Rising tide and all that. But if https://twitter.com/Dries/status/755391008897175554 this goes then, again, why bother?

All in all, this plugin needs to come with very heavy warnings to only use when migrating into a non Drupal system. At least that's what I think. But again, I am contradicted above by the project lead himself. I do not know what's going on any more.

@chx, I agreed with @benjy in comment #14. Maybe I wasn't clear enough in #14 but I agreed that we shouldn't support a "direct table copy" in core.

In my case for buytaert.net, I'm working with a custom module not released publicly. I'm the only user of my album module and I know no other modules alter its behavior. I obviously couldn't use a direct table copy if my album module was released on Drupal.org or if there are other sites that have to run the album module's migration code. That is not the case though - no other site needs to go through this migration. In other words, a direct table copy is 100% safe, significantly faster, less complex and less error-prone than writing my own custom migration plugin.

I disagree that I must use the entity system for my album module. Me choosing not to use the entity system is not a failure for the entity system maintainers in any way. I love the entity system -- I'm just not using it for this one feature on this one site. I think we should all be able to make our own architectural design decisions based on our needs, wants and legacies. Having said that, as I shared on Twitter (https://twitter.com/Dries/status/754367543759044608), my desire is to port album module to the entity system. As explained in my "podcast", I decided to do a straight port first, and to migrate to the entity system as phase two.

Just my two cents, since hussainweb brought this issue up on the weekly migration call:

Optimally, any Drupal modules which manage their own non-entity data will have some sort of save() API for writing that data (i.e., it's not just doing db_inserts in a form submit function). And, optimally, when migrating that data there will be a destination plugin using that API to write the data. Thus, I consider raw table inserts edge case-y, and not a use case that needs to be supported in core.

That being said, those edge cases do exist:

1. Under D7, iirc, I've used the MigrateDestinationTable "plugin" a couple of times where clients had some sort of non-Drupal application co-resident in the database and they needed the data populated.
2. As in this case, if you're doing a one-time migration and don't need to maintain the code long-term (technical debt is not an issue), and if you're confident you're not missing any essential hook invocations/related tables/etc. (and of course you're testing in a dev environment to confirm there are no ill effects), copying table data directly may be the path of least resistance. I will not judge you for this;-).

So, I consider this plugin a valid feature for migrate_plus and will review it the next time I run down the issue queue.

+++ b/src/Plugin/migrate/destination/Table.php
@@ -0,0 +1,125 @@
+    $this->dbConnection = $connection;
...
+      $container->get('database')

Finally getting around to reviewing this, it occurs to me there is another use case - populating an external database during a Drupal migration. For example, if in migrating your D7 site to D8, you are moving from Drupal Commerce to an external eCommerce solution (OK, any serious eCommerce solution would have an API you would wrap in a custom destination plugin, but you get the gist).

So, let's make the database connection configurable alongside table_name.

Otherwise this looks good on a quick scan - I'll review in more detail with the next iteration.

Committed, thanks!

Updated page title for google searches.