Error updating to Ghost 5.80 from 5.23, blog down, please help

[DigitalLeaves]

Issue Summary

Hello, I just updated my Ghost blog from 5.23 to 5.80.
After the update, I tried to restart Ghost, but it fails with this message:

Ghost instance is not running! Starting...
✖ Starting Ghost
A GhostError occurred.

Message: Ghost was able to start, but errored during boot with: insert into `collections_posts` (`collection_id`, `id`, `post_id`, `sort_order`) select '65f02f0dafb7eeb96c4c90a6' as `collection_id`, '65f02f0dafb7eeb96c4c90a8' as `id`, '637a812a81cfb759fad27265' as `post_id`, 0 as `sort_order` union all select '65f02f0dafb7eeb96c4c90a6' as `collection_id`,
... (many more rows) ...
, '65314decb0c970c6a6eca545' as `post_id`, 0 as `sort_order` - SQLITE_ERROR: too many terms in compound SELECT
Context: [object Object]
Help: Error occurred while executing the following migration: 2023-07-10-05-16-55-add-built-in-collection-posts.js

Debug Information:
OS: Debian GNU/Linux, v11
Node Version: v18.19.1
Ghost Version: 5.80.2
Ghost-CLI Version: 1.25.3
Environment: development
Command: 'ghost restart'
mysql Ver 8.0.27 for Linux on x86_64 (MySQL Community Server - GPL)

Steps to Reproduce

Start with an install of v5.23 or below 5.80
Apparently the number of posts can be a cause, so get more than 500 posts.
Run ghost update and ghost restart

Ghost Version

5.80.2

Node.js Version

v18.19.1

How did you install Ghost?

Command line

Database type

MySQL 5.7

Browser & OS version

Chrome, MacOS

Relevant log / error output

Ghost instance is not running! Starting...
✖ Starting Ghost
A GhostError occurred.
 
Message: Ghost was able to start, but errored during boot with: insert into `collections_posts` (`collection_id`, `id`, `post_id`, `sort_order`) select '65f03420dd3af6bb83e5ad3d' as `collection_id`, '65f03420dd3af6bb83e5ad3f' as `id`, '637a812a81cfb759fad27265' as `post_id`, 0 as `sort_order` union all select '65f03420dd3af6bb83e5ad3d' as `collection_id`, '65f03420dd3af6bb83e5ad40' as `id`, '637a812a81cfb759fad27266' as `post_id`, 0 as `sort_order` union all select '65f03420dd3af6bb83e5ad3d' as `collection_id`, '65f03420dd3af6bb83e5ad41' as `id`, '637a812a81cfb759fad27267'
... (rows ommited)...
, '65f03420dd3af6bb83e5afe3' as `id`, '65314decb0c970c6a6eca545' as `post_id`, 0 as `sort_order` - SQLITE_ERROR: too many terms in compound SELECT
Context: [object Object]
Help: Error occurred while executing the following migration: 2023-07-10-05-16-55-add-built-in-collection-posts.js

Code of Conduct

• I agree to be friendly and polite to people in this repository

[DigitalLeaves]

Ok, I managed to solve it manually. The issue is in the script 2023-07-10-05-16-55-add-built-in-collection-posts.js.

Specifically, in this function:

const insertPostCollections = async (knex, collectionId, postIds) => {
    logging.warn(`Batch inserting ${postIds.length} collection posts for collection ${collectionId}`);

    const collectionPosts = postIds.map((postId) => {
        return {
            id: (new ObjectID()).toHexString(),
            collection_id: collectionId,
            post_id: postId,
            sort_order: 0
        };
    });

    await knex.batchInsert('collections_posts', collectionPosts, 1000);
};

The problem is the batch size. I guess it does not make sense to have it set to 1000 if the maximum size for batch inserts in MySQL is 500. So lowering to 100 for example does the trick.

Now, can you kindly confirm that even by reducing the batch size to 100, all posts would be updated in the database as per the new Ghost 5.80 requirements? (looking at the code, it seems that is the case, but just want to be sure).

Thank you. A PR here would be quite easy I guess, just decreasing the limit to 100.

[allouis]

In order to fix this issue we need a few things:

1. A replication of the error
2. A batchSize which fixes the error
3. A new helper which wraps knex.batchInsert and forces it to use our safe batch size
4. Updating all uses of the knex.batchInsert to use the new helper

[markstos]

@allouis It's possible to take a "good now, better later" approach here.

If the PR #21063 is adjusted down to use a value of 100, it will solve some known, reported case were people are running into this SQLite limit and the code quality will be no worse than the current code which hardcodes a limit of 1,000 rather than 100. That's a good solution that can help now. Per above, the value of 100 has been tested.

What you describe here is a more ideal solution with a test-suite-ready replication and a helper method. Although, the helper method will only help if people remember to use it instead of knex.batchInsert.

An alternate path forward is to merge the PR adjusted to a batch size of 100 while also leaving this issue open for a more ideal approach later.

[allouis]

@markstos Absolutely! If this is still an issue for people and we have a PR that solves it - I'm happy to merge 😁

FWIW I wasn't asking for a test suite replication - a manual one would have been fine - the problem is that the original PR didn't fix the issue - so I'd like to be sure that anything we merge does fix an issue!

I'll reopen that PR for you now - thank you for coming back and taking an interest - it's appreciated!

[markstos]

This can be closed, it was resolved by #21063 in the next release after 5.99.