Silverstripe-framework: DataList::exists() not working correctly

[xini]

Affected Version

Silverstripe 4.7
nginx 1.18.0
MariaDB 10.5.5
PHP 7.4.13

installed modules:
axllent/silverstripe-image-optimiser
axllent/silverstripe-scaled-uploads
betterbrief/silverstripe-googlemapfield
bummzack/silverstripe-emogrify
bummzack/sortablefile
dnadesign/silverstripe-elemental
firebrandhq/silverstripe-phonelink
fromholdio/silverstripe-csshelpers
fromholdio/silverstripe-dbhtmlanchors
fromholdio/silverstripe-elemental-group
fromholdio/silverstripe-elemental-inheritablearea
fromholdio/silverstripe-elemental-multiarea
fromholdio/silverstripe-errorpagesconfig
fromholdio/silverstripe-externalurlfield
fromholdio/silverstripe-featureimage
fromholdio/silverstripe-globalanchors
fromholdio/silverstripe-gridfield-limiter
fromholdio/silverstripe-grouploginredirect
fromholdio/silverstripe-heroic
fromholdio/silverstripe-minigridfield
fromholdio/silverstripe-paged
fromholdio/silverstripe-schedulable
fromholdio/silverstripe-simplevideo
fromholdio/silverstripe-singular
fromholdio/silverstripe-sortable
fromholdio/silverstripe-superlinker-ctas
fromholdio/silverstripe-superlinker-megamenus
fromholdio/silverstripe-superlinker-menus
fromholdio/silverstripe-superlinker-redirection
fromholdio/silverstripe-superlinker-targets
fromholdio/silverstripe-systemlinks
fromholdio/silverstripe-urlsegmenter
heyday/silverstripe-responsive-images
heyday/silverstripe-wkhtml
innoweb/silverstripe-breadcrumbs
innoweb/silverstripe-bundled-userforms
innoweb/silverstripe-cmsstickymenupreference
innoweb/silverstripe-default-home
innoweb/silverstripe-dms
innoweb/silverstripe-email-obfuscator
innoweb/silverstripe-enhancedrss
innoweb/silverstripe-form-validation
innoweb/silverstripe-googleanalytics
innoweb/silverstripe-mailchimp-signup
innoweb/silverstripe-metacounter
innoweb/silverstripe-minify-html
innoweb/silverstripe-page-icons
innoweb/silverstripe-prefix-requirements
innoweb/silverstripe-requirements-resolver
innoweb/silverstripe-robots
innoweb/silverstripe-sitemap
innoweb/silverstripe-social-metadata
innoweb/silverstripe-social-profiles
innoweb/silverstripe-social-share
innoweb/silverstripe-tinymce-clearfloats
jonom/focuspoint
littlegiant/silverstripe-persistentgridfield
nglasl/silverstripe-misdirection
sheadawson/silverstripe-rateable
silverstripe/blog
silverstripe/html5
silverstripe/recipe-cms
silverstripe/userforms
stevie-mayhew/hasoneedit
symbiote-library/silverstripe-spamprotection-honeypot
symbiote/silverstripe-gridfieldextensions
symbiote/silverstripe-grouped-cms-menu
symbiote/silverstripe-multisites
toastnz/silverstripe-gridfieldversionedorderablerows
unclecheese/display-logic
webbuilders-group/silverstripe-cmspreviewpreference
wilr/silverstripe-googlesitemaps

Description

Parts of the site I'm working on are not loading, e.g. the menu items list is empty, the list of blog posts is empty. Other parts of the website load fine.

When I change `DataList::exists()' back to 4.6 behaviour

public function exists()
{
    return $this->count() > 0;
}

everything works fine again.

On my local machine with MariaDB 10.4.11 and PHP 7.4.2 everything works fine.

[ScopeyNZ]

So it is working on your local running Maria 10.4.11 but not with Maria 10.5.5?

[xini]

Correct, yes.

[ScopeyNZ]

Weird. I wonder if there's some related Maria change that's causing this, or a bug in a later version of Maria. The way the query is written it should be no different in terms of end result - we essentially just moved the > 0 check into the DB for efficiency.

I've had a quick scan of Maria changelogs and nothing's jumping out at me.

[3Dgoo]

I am having the same issue on my client's live server.
Silverstripe Framework 4.7.0
PHP 7.2.34
MariaDB 10.1.48

[xini]

So I'm not crazy. Thanks for confirming. ;)

[3Dgoo]

I have another case on different client's server:
Silverstripe Framework 4.7.0
PHP 7.3.11
MySQL 5.5.62
mysqlnd 5.0.12

[xini]

@3Dgoo can you test locally whether this has to do with the live environment setting?

[3Dgoo]

I have tried but I cannot recreate this locally yet.

I have set my local environment type to live but the issue does not occur.

I have also set the live server environment type to dev but the issue still occurs there.

[sminnee]

On environments where this is failing, what does a "SELECT EXISTS(SELECT ...)" query return? This is the form of the query used internally now.

[sminnee]

You could also try debugging the result returned on line 485 of vendor/Silverstripe/framework/src/ORM/DataQuery.php

It's doing a strict-type comparison to a few values; it's possible that another potential return value has been missed.

[3Dgoo]

In the DataList exists() function I have added the following:

if (!$this->dataQuery->exists() && $this->count() > 0) { 
    print $this->dataQuery->sql();
}

Here are a few results.

On a FAQPage with a has_many relationship to a DataObject called FAQQuestion trying to get related FAQQuestions:

SELECT DISTINCT "FAQQuestion"."ClassName", "FAQQuestion"."LastEdited", "FAQQuestion"."Created", "FAQQuestion"."Title", "FAQQuestion"."Content", "FAQQuestion"."SortOrder", "FAQQuestion"."FAQPageID", "FAQQuestion"."ID", CASE WHEN "FAQQuestion"."ClassName" IS NOT NULL THEN "FAQQuestion"."ClassName" ELSE 'FAQQuestion' END AS "RecordClassName" FROM "FAQQuestion" WHERE ("FAQQuestion"."FAQPageID" = ?) ORDER BY "FAQQuestion"."SortOrder" ASC

On the Silverstripe admin Dashboard page trying to get the last 8 edited pages with a Page::get():

SELECT DISTINCT "SiteTree"."ClassName", "SiteTree"."LastEdited", "SiteTree"."Created", "SiteTree"."Priority", "SiteTree"."ShowInSitemap", "SiteTree"."InheritBlockSets", "SiteTree"."CanViewType", "SiteTree"."CanEditType", "SiteTree"."Version", "SiteTree"."URLSegment", "SiteTree"."Title", "SiteTree"."MenuTitle", "SiteTree"."Content", "SiteTree"."MetaDescription", "SiteTree"."ExtraMeta", "SiteTree"."ShowInMenus", "SiteTree"."ShowInSearch", "SiteTree"."Sort", "SiteTree"."HasBrokenFile", "SiteTree"."HasBrokenLink", "SiteTree"."ReportClass", "SiteTree"."ParentID", "Page"."SEOPageSubject", "Page"."SEOSocialType", "Page"."SEOHideSocialData", "Page"."HeaderContent", "Page"."MetaTitle", "Page"."NoMenuLink", "Page"."BackgroundColour", "Page"."ExtraClass", "Page"."CachePage", "Page"."SEOSocialImageID", "Page"."MenuGroupID", "SiteTree"."ID", CASE WHEN "SiteTree"."ClassName" IS NOT NULL THEN "SiteTree"."ClassName" ELSE 'SilverStripe\\CMS\\Model\\SiteTree' END AS "RecordClassName" FROM "SiteTree" LEFT JOIN "Page" ON "Page"."ID" = "SiteTree"."ID" WHERE ("SiteTree"."LastEdited" > ?) AND ("SiteTree"."ClassName" IN (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)) ORDER BY "SiteTree"."LastEdited" DESC LIMIT 8

[3Dgoo]

I have debugged the DataQuery exists function. In these cases it looks like result returned is string(1) "1"

[3Dgoo]

Perhaps line 488 of vendor/Silverstripe/framework/src/ORM/DataQuery.php should be changed from this:

return $result === true || $result === 1 || $result === 't';

to this:

return $result === true || $result === 1 || $result === '1' || $result === 't';

[ScopeyNZ]

You could also try debugging the result returned on line 485 of vendor/Silverstripe/framework/src/ORM/DataQuery.php

It's doing a strict-type comparison to a few values; it's possible that another potential return value has been missed.

Looks like this was correct - but we did consider this when implementing the ->exists patch in the first place - so it looks like something has changed here.

[xini]

Great work @3Dgoo! Could this be wrapped in another statement (like count or something) to get a more reliable return value?

[xini]

I was thinking of something similar to https://github.com/silverstripe/silverstripe-framework/commit/02827a66700f3a1d05ffb4065d1e664da94ee49e

[ScopeyNZ]

Doing a count with SQL defeats the purpose of the SELECT EXISTS query, as the optimisation is the fact that SQL doesn't have to actually care about the number of records for a filter, just that there's at least one record.

I think that a patch that adds the string result to the check would be suitable for now - although fixing the types returned by the adapter properly would be preferable.

[xini]

what about a SELECT CASE WHEN EXISTS(...) THEN 1 ELSE 0 END?

[sminnee]

Adding "1" to the check seems like a reasonable patch here.

[xini]

Ok. The "case when" would solve this more reliably though?

[sminnee]

I'm not sure that it does. The issue is most likely the type conversion of data being pulled from the DB layer (which is non-trivial). So I suspect that the case statement also returns "1" to PhP in that environment.

It would be handy if you could run the ORM unit tests on an environment with this issue and see what fails; we may also want to consider adding it to our build matrix.

[xini]

Hm. But wouldn't that be the case for any count and other commands?
The commit I have referenced above uses the case when to fix exactly the same problem.

[xini]

For anyone stumbling over this:

I had similar issues in other places, but not directly related to exists() (at least I think so).

I was able to fix it by switching from MySQLPDODatabase to MySQLDatabase. see https://github.com/silverstripe/silverstripe-framework/issues/8598 for reference.

[3Dgoo]

Wow. Thank you for that information @xini . That is really good to know.

[xini]

no worries. ;)

[sminnee]

PDO is awful 😆

[HARVS1789UK]

+1 for @xini 's proposal of SELECT CASE WHEN EXISTS(...) THEN 1 ELSE 0 END (albeit in addition to the extra type checking @sminnee proposed and seems to be in 4.7.1 ) because the current implementation of SELECT EXISTS(...) is invalid T-SQL.

Currently any references to DataQuery::exists() (which includes multiple default CMS UI's) within my 4.7.1 project using SQL Server fail with errors such as:

[Emergency] Uncaught SilverStripe\ORM\Connect\DatabaseException: Couldn't run query: SELECT EXISTS SELECT * FROM "ProcessStep" WHERE ("ProcessStep"."ReferAFriendPageID" = ?)) 42000, 156, [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Incorrect syntax near the keyword 'EXISTS'.

Based on the SilverStripe Server Requirements SQL Server is still supported and, rightly or wrongly, the MSSQL module doesn't seem to go anywhere near methods like DataQuery::exists()?

[HARVS1789UK]

Seems to have been introduced by #8915 on 1st Sept 2020