Skip to content

Commit e88a4e5

Browse files
authored
Fix the behavior of HAVING clause (#154)
The current implementation has a special handling for `HAVING` clauses without `GROUP BY` (this is [valid in MySQL](https://stackoverflow.com/questions/6924896/having-without-group-by) in some SQL modes). This implementation takes advantage of the equivalence of ungrouped HAVING with WHERE and rewrites such a `HAVING <condition>` clause to `AND <condition>`. However, the current implementation has some issues: 1. The presence of `GROUP BY` in a query is incorrectly detected. The detection always fails, and thus every HAVING query is considered to be ungrouped. I added a fix in the first commit. 2. Rewriting `HAVING <condition>` to `AND <condition>` fails with an error `near "AND": syntax error` when no WHERE is included in the query. This is fixed in the second commit. 3. When an aggregate function is used in HAVING (e.g, `COUNT(*) > 1`), the query fails with: `misuse of aggregate function COUNT()`. This is because aggregate functions can't be used in the WHERE clause. This is fixed in the second commit. To fix all of these issues, I implemented a fix for the `GROUP BY` detection (case 1), and changed the rewriting of `HAVING <condition>` without `GROUP BY` to `GROUP BY 1 HAVING <condition>` (which fixes both case 2 and 3).
1 parent a7fd7c3 commit e88a4e5

File tree

2 files changed

+90
-8
lines changed

2 files changed

+90
-8
lines changed

tests/WP_SQLite_Translator_Tests.php

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3133,6 +3133,85 @@ public function testCurrentTimestamp() {
31333133
$this->assertQuery( 'DELETE FROM _dates WHERE option_value = CURRENT_TIMESTAMP()' );
31343134
}
31353135

3136+
public function testGroupByHaving() {
3137+
$this->assertQuery(
3138+
'CREATE TABLE _tmp_table (
3139+
name varchar(20)
3140+
);'
3141+
);
3142+
3143+
$this->assertQuery(
3144+
"INSERT INTO _tmp_table VALUES ('a'), ('b'), ('b'), ('c'), ('c'), ('c')"
3145+
);
3146+
3147+
$result = $this->assertQuery(
3148+
'SELECT name, COUNT(*) as count FROM _tmp_table GROUP BY name HAVING COUNT(*) > 1'
3149+
);
3150+
$this->assertEquals(
3151+
array(
3152+
(object) array(
3153+
'name' => 'b',
3154+
'count' => '2',
3155+
),
3156+
(object) array(
3157+
'name' => 'c',
3158+
'count' => '3',
3159+
),
3160+
),
3161+
$result
3162+
);
3163+
}
3164+
3165+
public function testHavingWithoutGroupBy() {
3166+
$this->assertQuery(
3167+
'CREATE TABLE _tmp_table (
3168+
name varchar(20)
3169+
);'
3170+
);
3171+
3172+
$this->assertQuery(
3173+
"INSERT INTO _tmp_table VALUES ('a'), ('b'), ('b'), ('c'), ('c'), ('c')"
3174+
);
3175+
3176+
// HAVING condition satisfied
3177+
$result = $this->assertQuery(
3178+
"SELECT 'T' FROM _tmp_table HAVING COUNT(*) > 1"
3179+
);
3180+
$this->assertEquals(
3181+
array(
3182+
(object) array(
3183+
':param0' => 'T',
3184+
),
3185+
),
3186+
$result
3187+
);
3188+
3189+
// HAVING condition not satisfied
3190+
$result = $this->assertQuery(
3191+
"SELECT 'T' FROM _tmp_table HAVING COUNT(*) > 100"
3192+
);
3193+
$this->assertEquals(
3194+
array(),
3195+
$result
3196+
);
3197+
3198+
// DISTINCT ... HAVING, where only some results meet the HAVING condition
3199+
$result = $this->assertQuery(
3200+
'SELECT DISTINCT name FROM _tmp_table HAVING COUNT(*) > 1'
3201+
);
3202+
$this->assertEquals(
3203+
array(
3204+
(object) array(
3205+
'name' => 'b',
3206+
),
3207+
(object) array(
3208+
'name' => 'c',
3209+
),
3210+
),
3211+
$result
3212+
);
3213+
}
3214+
31363215
/**
31373216
* @dataProvider mysqlVariablesToTest
31383217
*/

wp-includes/sqlite/class-wp-sqlite-translator.php

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2659,23 +2659,19 @@ private function capture_group_by( $token ) {
26592659
! $token->matches(
26602660
WP_SQLite_Token::TYPE_KEYWORD,
26612661
WP_SQLite_Token::FLAG_KEYWORD_RESERVED,
2662-
array( 'GROUP' )
2662+
array( 'GROUP BY' )
26632663
)
26642664
) {
26652665
return false;
26662666
}
2667-
$next = $this->rewriter->peek_nth( 2 )->value;
2668-
if ( 'BY' !== strtoupper( $next ?? '' ) ) {
2669-
return false;
2670-
}
26712667

26722668
$this->has_group_by = true;
26732669

26742670
return false;
26752671
}
26762672

26772673
/**
2678-
* Translate WHERE something HAVING something to WHERE something AND something.
2674+
* Translate HAVING without GROUP BY to GROUP BY 1 HAVING.
26792675
*
26802676
* @param WP_SQLite_Token $token The token to translate.
26812677
*
@@ -2694,8 +2690,15 @@ private function translate_ungrouped_having( $token ) {
26942690
if ( $this->has_group_by ) {
26952691
return false;
26962692
}
2697-
$this->rewriter->skip();
2698-
$this->rewriter->add( new WP_SQLite_Token( 'AND', WP_SQLite_Token::TYPE_KEYWORD ) );
2693+
2694+
// GROUP BY is missing, add "GROUP BY 1" before the HAVING clause.
2695+
$having = $this->rewriter->skip();
2696+
$this->rewriter->add( new WP_SQLite_Token( ' ', WP_SQLite_Token::TYPE_DELIMITER ) );
2697+
$this->rewriter->add( new WP_SQLite_Token( 'GROUP BY', WP_SQLite_Token::TYPE_KEYWORD ) );
2698+
$this->rewriter->add( new WP_SQLite_Token( ' ', WP_SQLite_Token::TYPE_DELIMITER ) );
2699+
$this->rewriter->add( new WP_SQLite_Token( '1', WP_SQLite_Token::TYPE_NUMBER ) );
2700+
$this->rewriter->add( new WP_SQLite_Token( ' ', WP_SQLite_Token::TYPE_DELIMITER ) );
2701+
$this->rewriter->add( $having );
26992702

27002703
return true;
27012704
}

0 commit comments

Comments
 (0)