Won't fix 2.1.0 Alpha step 33 SQL error during upgrade

[digitalpoint]

Affected version

2.1.0

Not a huge deal because I already worked through it, but might save you guys a support issue down the road. While the SQL query build is fancy and big, not all storage engines support multiple commands in a single SQL statement.



From the Limitations of NDB online operations section here: https://dev.mysql.com/doc/refman/8.0/en/mysql-cluster-online-operations.html

A given online ALTER TABLE can use only one of ADD COLUMN, ADD INDEX, or DROP INDEX. One or more columns can be added online in a single statement; only one index may be created or dropped online in a single statement.

Not as clean, but at least it works... I just made each function their own SQL statement in step 33, and goes through without any issues.

    public function step33()
    {
        if (!$this->schemaManager()->tableExists('xf_liked_content'))
        {
            // if this table doesn't exist, then this query has been run already, so we can skip it.
            return;
        }

        $this->alterTable('xf_liked_content', function (Alter $table) {
            $table->renameTo('xf_reaction_content');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->renameColumn('like_id', 'reaction_content_id');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->addColumn('reaction_id', 'int')->setDefault(1)->after('reaction_content_id');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->renameColumn('like_user_id', 'reaction_user_id');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->renameColumn('like_date', 'reaction_date');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->dropIndexes('content_type_id_like_user_id');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->dropIndexes('like_user_content_type_id');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->dropIndexes('content_user_id_like_date');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->dropIndexes('like_date');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->addUniqueKey(['content_type', 'content_id', 'reaction_user_id'], 'content_type_id_user_id');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->addKey(['content_type', 'content_id', 'reaction_date'], 'content_type_id_reaction_date');
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->addKey(['content_user_id', 'reaction_date']);
        });

        $this->alterTable('xf_reaction_content', function (Alter $table) {
            $table->addKey('reaction_date');
        });
    }

 

[Mike]

This isn't likely to be something that we intend to account for as it's not a limitation of InnoDB (or MyISAM). More significantly though, splitting those queries would make that step significantly slower for the common case as MySQL would end up doing multiple rebuild passes. In the past, I have tested splitting up alters like this and the cost for 2 operations (that require low level changes) is X in 1 query and 2X in 2 queries.

 

[digitalpoint]

Gross, InnoDB is still a thing (haha j/k)?

Personally, I’d probably make the method that combines the stuff into a single query check the config setting XenForo has for which storage engine is being used. If it’s InnoDB or MyISAM, combine them like it does. XenForo does have an option to use whatever storage engine the user wants after all. Not a huge deal for me, just throwing it out there.