Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FolderMigrationHelper throws DatabaseException when using PostgreSQL #443

Open
TBurleigh opened this issue Apr 14, 2021 · 5 comments
Open

Comments

@TBurleigh
Copy link

TBurleigh commented Apr 14, 2021

I'm migrating a SS 3.7 site which is using silverstripe/postgresql to SS 4.7.2.

A DatabaseException is thrown when I run /dev/tasks/MigrateFileTask

[Emergency] Uncaught SilverStripe\ORM\Connect\DatabaseException: Couldn't run query: SELECT DISTINCT "Filename", "Live"."ID" AS "LiveID", "File"."ClassName", "File"."LastEdited", "File"."Created", "File"."Version", "File"."CanViewType", "File"."CanEditType", "File"."Name", "File"."Title", "File"."ShowInSearch", "File"."ParentID", "File"."OwnerID", "File"."FileHash", "File"."FileFilename", "File"."FileVariant", "File"."ID", CASE WHEN "File"."ClassName" IS NOT NULL THEN "File"."ClassName" ELSE E'SilverStripe\\Assets\\File' END AS "RecordClassName" FROM "File" LEFT JOIN "File_Live" AS "Live" ON "Live"."ID" = "File"."ID" WHERE ("File"."ClassName" IN ($1)) HAVING ("LiveID" IS NULL) ORDER BY "File"."Name" ASC ERROR: column "LiveID" does not exist LINE 6: HAVING ("LiveID" IS NULL) ^
GET /dev/tasks/MigrateFileTask

Line 64 in /opt/silverstripe/webapp/vendor/silverstripe/framework/src/ORM/Connect/DBConnector.php
Source

55         if (!empty($sql)) {
56             $formatter = new SQLFormatter();
57             $formattedSQL = $formatter->formatPlain($sql);
58             $msg = "Couldn't run query:\n\n{$formattedSQL}\n\n{$msg}";
59         }
60 
61         if ($errorLevel === E_USER_ERROR) {
62             // Treating errors as exceptions better allows for responding to errors
63             // in code, such as credential checking during installation
64             throw new DatabaseException($msg, 0, null, $sql, $parameters);
65         } else {
66             user_error($msg, $errorLevel);
67         }
68     }
69 
70     

Trace

    SilverStripe\ORM\Connect\DBConnector->databaseError
    PostgreSQLConnector.php:234
    SilverStripe\PostgreSQL\PostgreSQLConnector->preparedQuery
    Database.php:185
    SilverStripe\ORM\Connect\Database->SilverStripe\ORM\Connect\{closure}
    Database.php:258
    SilverStripe\ORM\Connect\Database->benchmarkQuery
    Database.php:183
    SilverStripe\ORM\Connect\Database->preparedQuery
    DB.php:445
    SilverStripe\ORM\DB::prepared_query
    SQLExpression.php:115
    SilverStripe\ORM\Queries\SQLExpression->execute
    SQLSelect.php:631
    SilverStripe\ORM\Queries\SQLSelect->count
    DataQuery.php:453
    SilverStripe\ORM\DataQuery->count
    DataList.php:881
    SilverStripe\ORM\DataList->count
    FolderMigrationHelper.php:93
    SilverStripe\Assets\Dev\Tasks\FolderMigrationHelper->ss3Migration

PostgreSQL doesn't allow HAVING clauses to reference columns from the SELECT part of the query

A work around is to change FolderMigrationHelper.php#L157

From $q->having(['"LiveID" IS NULL']); // filters all folders without a record in the live table
To $q->where(['"Live"."ID" IS NULL']);

@geekdenz
Copy link

@TBurleigh Thanks!

I've created a custom class and overrode it with Injector:

CustomFolderMigrationHelper.php:

<?php

use SilverStripe\Core\Convert;
use SilverStripe\Assets\Folder;
use SilverStripe\ORM\DataObject;
use SilverStripe\Versioned\Versioned;
use SilverStripe\Core\Injector\Injector;
use SilverStripe\Assets\Dev\Tasks\FolderMigrationHelper;
use SilverStripe\ORM\DataQuery;

class CustomFolderMigrationHelper extends FolderMigrationHelper
{
    /**
     * Get list of File dataobjects to import
     *
     * @return DataList
     */
    protected function getQuery()
    {
        $versionedExtension = Injector::inst()->get(Versioned::class);

        $schema = DataObject::getSchema();
        $baseDataClass = $schema->baseDataClass(Folder::class);
        $baseDataTable = $schema->tableName($baseDataClass);
        $liveDataTable = $versionedExtension->stageTable($baseDataTable, Versioned::LIVE);

        $query = Folder::get()->leftJoin(
            $liveDataTable,
            sprintf('"Live"."ID" = %s."ID"', Convert::symbol2sql($baseDataTable)),
            'Live'
        )->alterDataQuery(static function (DataQuery $q) {
            $q->selectField('"Filename"');  // used later for logging processed folders
            $q->selectField('"Live"."ID"', 'LiveID');  // needed for having clause to work
            $q->where(['"Live"."ID" IS NULL']);  // filters all folders without a record in the live table
        });

        return $query;
    }
}

injector.yaml:

SilverStripe\Core\Injector\Injector:
  SilverStripe\Assets\Dev\Tasks\FolderMigrationHelper:
    class: CustomFolderMigrationHelper

and it works in our project.

@geekdenz
Copy link

HAVING actually has performance drawbacks as it does not use the index and is applied after the GROUP BY.
Seems like without GROUP BY it is basically not worth making the query more complicated with it. Not sure why you would want to do it after grouping, because wouldn't it always be less performant without changing the result?

I guess there could be cases where it changes the result. I just cannot think of how you would require it for that, because you could always do a sub query which I believe is simpler to think of.

@geekdenz
Copy link

So you could delete this line:

$q->selectField('"Live"."ID"', 'LiveID');  // needed for having clause to work

@dhensby
Copy link
Contributor

dhensby commented May 14, 2021

@TBurleigh would you be happy to open a PR fixing this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants