Commerce Kickstart V2 beta2+2 was working almost perfect on PostgreSQL database :-)

The Commerce Kickstart 7.x-2.0-RC1 is installable and you see landing page of the demo store but
since version 7.x-2.0-RC1 the following error appears when you click any (menu) items.

PDOException: SQLSTATE[42803]: Grouping error: 7 ERROR: column "ml.link_title" must appear in the GROUP BY clause or be used in an aggregate function LINE 1: SELECT ml.link_title AS link_title, ml.menu_name AS menu_nam... ^: SELECT ml.link_title AS link_title, ml.menu_name AS menu_name FROM {menu_links} ml WHERE (link_path = :db_condition_placeholder_0) GROUP BY menu_name; Array ( [:db_condition_placeholder_0] => user ) in menu_CrumbsMultiPlugin_link_title->findTitle() (line 69 of /srv/www/clients/client0/web1/web/ks7x20rc1/profiles/commerce_kickstart/modules/contrib/crumbs/plugins/crumbs.menu.inc).

This error you even get when you try to login as administrator. Therefore the Commerce Kickstart is unusable for PostgreSQL users at the moment :.-(

I guess this error is related to the new CRUMBS-module.

CommentFileSizeAuthor
#7 crumbs.menu_.inc_.zip1002 bytesMetulski
#4 crumbs-1788214-4-menu-plugin-findTitle-postgre.patch908 bytesdonquixote

Comments

bojanz’s picture

bojanz commented
Title: Commerce Kickstart V2 completly UNUSABLE since RC1-Version on POSTGRESQL 9.1.5 database (due to CRUMBS-module?) » Crumbs compatibility issue with PostgreSQL
Project: Commerce Kickstart » Crumbs, the Breadcrumbs suite
Version: 7.x-2.0-rc1 » 7.x-1.x-dev
Priority: Critical » Major

That's a lot of bold lettering.

Moving to Crumbs. We'll get it fixed there.

donquixote’s picture

donquixote commented

There was something similar over here. I thought I had fixed it..
#1431564: Enabling module on sqlsrv DB driver results in an error

The problem is what I do there to pick one out of multiple rows in the db result. I am pretty sure that whatever I am doing there, there are smarter ways to do the same (and have it work outside of mysql).

donquixote’s picture

donquixote commented

Grr, what an awkward abuse of foreach() I made there.
http://drupalcode.org/project/crumbs.git/blob/refs/heads/7.x-1.x:/plugin...
(lines 43, 44)

Basically, the findTitle() in line 61 should use the same logic as findParent() further up. But we want to write that in a less awkward way.

The "problem" is that more than one menu item (even in the same menu) might have the same link path.
What we want to do (and is already implemented for findParent()) is to only consider the one with the smallest depth.
And if two items have the same link path and the same depth? Then we just pick whatever the database delivers first.
(we could sort by weight, but even that might be equal, if items are in separate submenus)

-----------

That aside, we could also avoid one query if we reuse the query result of findParent() for findTitle().
On the other hand, we still want them in separate classes, because we want the plugin engine to skip one of them if it is disabled.

donquixote’s picture

donquixote commented
Status: Active » Needs review
StatusFileSize
new crumbs-1788214-4-menu-plugin-findTitle-postgre.patch908 bytes

This patch should be enough to fix it.
It does the same order as in findParent(), but in a more sane way.

donquixote’s picture

donquixote commented

If this works, we can do the same for findParent().

@bojanz, can you test?

[EDIT]
Note for testing:
I am pretty sure it does remove the conflict with postgre or other db engines.
What I am more concerned about is whether it really does achieve the order that we intend.

We should probably also write some test - but I would rather postpone this in favour of a quick bugfix.
(I guess it would be a web test with some heavy work on setting up a test menu. far less trivial than the bugfix itself)

Metulski’s picture

Metulski commented

@donquixote

Thanks for delivering the patch so fast.

As far as I could review your patch it seems to be the solution for the CRUMBS<->POSTGRESQL incompatibility.
Commerce Kickstart seems to run as good as before RC1-release.

Metulski’s picture

Metulski commented
StatusFileSize
new crumbs.menu_.inc_.zip1002 bytes

@donquixote

today I installed kickstart7.x-2.0-rc1+13-dev that includes the latest crumbs-7.x-1.4+20-dev module. But the patch you have provided (#4) seems not to be applied to the crumbs.menu.inc file.

Therefore the following error still appears:
PDOException: SQLSTATE[42803]: Grouping error: 7 ERROR: column "ml.link_title" must appear in the GROUP BY clause or be used in an aggregate function LINE 1: SELECT ml.link_title AS link_title, ml.menu_name AS menu_nam... ^: SELECT ml.link_title AS link_title, ml.menu_name AS menu_name FROM {menu_links} ml WHERE (link_path = :db_condition_placeholder_0) GROUP BY menu_name; Array ( [:db_condition_placeholder_0] => admin ) in menu_CrumbsMultiPlugin_link_title->findTitle() (line 69 of /srv/www/clients/client0/web1/web/ks7x20rc1plus13/profiles/commerce_kickstart/modules/contrib/crumbs/plugins/crumbs.menu.inc).

So I must still apply the patch manually :-(

Are there still any reasons / doubts for not applying the patch in the next development version of crumbs?

Please have a look at the patched crumbs.menu.inc file as attachment that is working for me.

donquixote’s picture

donquixote commented

The reason was me being on a travel marathon.
I returned home in this hour :)

My infrastructure at home makes it much easier to apply this stuff.

donquixote’s picture

donquixote commented
Status: Needs review » Fixed

Committed.
The equivalent should be done for menu_CrumbsMultiPlugin_hierarchy::findParent().
This should be discussed in #1431564: Enabling module on sqlsrv DB driver results in an error

Status: Fixed » Closed (fixed)

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