You know better than me the context. Is there a possibility that an installation is already without that column?

If you think yes, maybe DROP COLUMN IF EXISTS

In D25900#24197, @valerio.bozzolan wrote:

You know better than me the context. Is there a possibility that an installation is already without that column?

Theoretically, yes, but in practice no, provided they ran storage upgrade. After a certain point, the deprecations in newer PHP versions would prevent you from running the migrations anyway. Afterwards, the paste table would have been renamed, breaking the upgrade entirely.

(Also I forgot to drop the column from PhabricatorPaste - whoops)

What is (or rather was, I guess) that mailKey thingie good for? Or more relevant, I assume it's intentional after performing the Test Plan steps above that creating a new Paste does not create a new row in phabricator_metamta.metamta_mailproperties?
Apart from that, seems to work as expected. :) Thanks!

In D25900#25554, @aklapper wrote:

What is (or rather was, I guess) that mailKey thingie good for?

According to https://secure.phabricator.com/T13065: This key is used as a seed to generate unique addresses for each <object, user> pair so that I send mail to T123+1+abe2h3f2 and you send mail to T123+2+fo1m213n and I can't just fiddle with my "+1" and impersonate someone else.

Or more relevant, I assume it's intentional after performing the Test Plan steps above that creating a new Paste does not create a new row in phabricator_metamta.metamta_mailproperties?
Apart from that, seems to work as expected. :) Thanks!

I had to go dig for this, but from what I understand of looking at PhabricatorMetaMTAMailProperties, which is the class now responsible for this functionality, creating a mail key only happens when its loadMailKey method is called. That method is only called when email functionality engaged. That does make testing slightly more difficult. The updated test plan (thanks, by the way!) is a good compromise.

@mainframe98: Does ./bin/lipsum generate pastes work as expected for you? I'm seeing errors here:

 OOPS  Generator ("Pastes") was unable to generate an object.
#1364: Field 'mailKey' doesn't have a default value

Works for me: (on rPdfe8539c6a)

$ ./bin/lipsum generate pastes
 GENERATORS  Selected generators: Pastes.
 WARNING  This command generates synthetic test data, including user accounts. It is intended for use in development environments so you can test features more easily. There is no easy way to delete this data or undo the effects of this command. If you run it in a production environment, it will pollute your data with large amounts of meaningless garbage that you can not get rid of.


    Are you sure you want to generate piles of garbage? [y/N] y

 LIPSUM  Generating synthetic test objects forever. Use ^C to stop when satisfied.
Generated "Paste": P1 cancel_user_user.php
Generated "Paste": P2 account_lost_disk.java
Generated "Paste": P3 table_script.java
Generated "Paste": P4 cancel_elder_ancient_shard.java
Generated "Paste": P5 lost_table.php
Generated "Paste": P6 database.java
Generated "Paste": P7 accounts_backup.php
Generated "Paste": P8 tables_and_cancel_legendary_table
Generated "Paste": P9 user_disk_databases_and_assess_account_database_pro.java
Generated "Paste": P10 undo_lost_host_and_administrate_accounts_backup_script.php
Generated "Paste": P11 account_backup.java
^C

They show up properly, too.

Going by the error message, did you run ./bin/storage upgrade? It sounds like the table for Paste still has the mailkey column, but running Phorge with this commit will not write to that column, which is an error, as the column may not be null.

Argh, PEBKAC, right. Thanks!

[acko@fedora phorge (master *$|u=)]$ ./bin/storage upgrade
Target                          Error
phabricator_paste.paste.mailKey Surplus

 SURPLUS SCHEMATA