Problem/Motivation

CacheTest as well as RelationshipRepresentativeNodeTest in both, user and taxonomy test group, fails currently with PostgreSQL as database backend. This is because of missing quotes for identifiers. The problem in detail is that conditionNamespace() uses UPPERCASE for namespaces and such we get identifiers like these:

taxonomy_term_data_node__taxonomy_term_field_dataINNER.tid

And this is a problem as long as these identifiers are not quoted, because PostgreSQL will lower case them (unless quoted) and so we will end with an exception.

Proposed resolution

Quote identifiers to ensure case sensitve identifiers.

Remaining tasks

Write patch.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#10 2443699-10.patch1.1 KBbzrudi71
#1 2443699-1.patch980 bytesdaffie

Comments

daffie’s picture

daffie commented
Status: Active » Needs review
StatusFileSize
new 2443699-1.patch980 bytes

This patch fixes the CacheTest for me with PostgreSQL.

bzrudi71’s picture

bzrudi71 commented

Awesome find daffie! Especially as this fixes the RelationshipRepresentativeNodeTest() in user and taxonomy! We had no solution for those so far and your patch fixed them as a side effect, nice :-) Hopefully this get's green for MySQL too so we can update the IS and get this in ASAP.

Status: Needs review » Needs work

The last submitted patch, 1: 2443699-1.patch, failed testing.

bzrudi71’s picture

bzrudi71 commented
Issue summary: View changes

Status: Needs work » Needs review

daffie queued 1: 2443699-1.patch for re-testing.

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented

We should run the patch through SQLite tests as well, and might make sense to just glance at Mongo driver class overrides in Views to make sure we're not causing any headaches there. That's because we're doing specific quoting in a non-Sql class.

bzrudi71’s picture

bzrudi71 commented

I did SQLite testing and all is okay. We have pass in CacheTest and no other new fails or exceptions. I think this patch just needs some commenting love and then is ready to go.

bzrudi71’s picture

bzrudi71 commented

@mradcliffe, I think we should move forward here? Anything left to do from your point of view?

mradcliffe’s picture

mradcliffe
he/him
Primary language English
commented

I wasn't able to find anything in the mongodb driver that overrode this specifically so if there is a conflict in could be addressed later.

bzrudi71’s picture

bzrudi71 commented
StatusFileSize
new 2443699-10.patch1.1 KB

Okay, thanks mradcliffe for the mongodb check. New patch with just some comment additions attached, so I think we are ready to set RTBC now? This patch will also fix remaining exceptions in:

andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/core/modules/views/src/Plugin/views/relationship/GroupwiseMax.php
@@ -322,10 +322,19 @@ protected function alterSubqueryCondition(AlterableInterface $query, &$condition
+    $parts = explode(' = ', $string);
...
+    return implode(' = ', $parts);

why ' = ' is used here?

bzrudi71’s picture

bzrudi71 commented

@andypost: Because $string is always a condition like:

taxonomy_indexINNER.tid = taxonomy_term_field_data_node_field_dataINNER.tid

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

Checked, yes!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed a7a5236 and pushed to 8.0.x. Thanks!

  • alexpott committed a7a5236 on 8.0.x 
    Issue #2443699 by bzrudi71, daffie: PostgreSQL: Fix views\Tests\Plugin\...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.