As described in the hijack of #1054168-2: EntityFieldQuery fails for entities that have no bundle if you have something like 

SELECT column + 1 AS calculated_column
HAVING calculated_column > 1

convert this into DBTNG and make it a countQuery then it removes the column + 1 AS calculated_column expression and replaces with COUNT(*) which fails the HAVING.

CommentFileSizeAuthor
#15 d7-1258000-14.patch4.79 KBxjm
#10 1258000_POST-APOCALYPSE.patch4.83 KBxjm
#7 1258000_7.patch4.79 KBchx
#4 1258000_4.patch4.91 KBchx
#2 1258000-2.patch2.3 KBbfroehle
#1 1258000_test_only.patch847 byteschx

Comments

chx’s picture

chx commented
Status: Active » Needs review
StatusFileSize
new 1258000_test_only.patch847 bytes

Test, fails.

bfroehle’s picture

bfroehle commented
StatusFileSize
new 1258000-2.patch2.3 KB

Is this sort of approach what we are looking for?

chx’s picture

chx commented
Status: Needs review » Needs work

Thanks for jumping on the issue so quick! HAVING foo = 1 AND ( bar = 1 OR bar > 3) will break (Edit: for bar). I think you need to refactor the having handling to collect fields. Edit: and even that might not be enough because if you have several levels of DatabaseConditions then havingCondition will only see the top ones. :/

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new 1258000_4.patch4.91 KB

I needed to merge #1196292: Allow Pager count queries to utilise GROUP BY into this one to make the test pass. We dont remove any expression if there is HAVING.

damien tournoud’s picture

damien tournoud commented

I am not sure the original query is valid in the first place. In strict SQL, WHERE and GROUP BY can only contain references to columns, not to aliases. It is likely the case for HAVING too.

chx’s picture

chx commented
Status: Needs review » Needs work

Sure. The having methods fixes are still valid even if the countQuery isn't.

chx’s picture

chx commented
Status: Needs work » Needs review
StatusFileSize
new 1258000_7.patch4.79 KB

I studied http://www.postgresql.org/docs/8.1/static/tutorial-agg.html and http://www.postgresql.org/docs/9.0/static/sql-select.html#SQL-HAVING and I think that you can run HAVING on an expression provided the column (or the expression itself?? MySQL and PostgreSQL seems to disagree on that) is selected.

xjm’s picture

xjm
she/her
Primary language English
commented

Tracking.

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +Needs backport to D7

Tagging for backport.

xjm’s picture

xjm
she/her
Primary language English
commented
StatusFileSize
new 1258000_POST-APOCALYPSE.patch4.83 KB

Rerolled for core/.

Crell’s picture

Crell commented
Status: Needs review » Reviewed & tested by the community

Looking at the patch itself, it's a major "duh". :-) (chx talked to me about this a while back; it just fell off my radar.)

catch’s picture

catch
he/him
Primary language English
commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Looks great, don't see anything to complain about. Committed to 8.x, moving back to 7.x for which this will need a re-roll.

xjm’s picture

xjm
she/her
Primary language English
commented
Status: Patch (to be ported) » Reviewed & tested by the community
StatusFileSize
new d7-1258000-14.patch4.79 KB

Nothing particular to D8 so it's a straight reroll.

dries’s picture

dries commented

Committed to 7.x. Thanks!

aspilicious’s picture

aspilicious commented
Status: Reviewed & tested by the community » Fixed

:)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

changed to #1054168-2 instead of #1054168:2