- Fixes GROUP BY when using non-aggregate columns

lib/Models/Subscriber.php

@@ -304,14 +304,19 @@ class Subscriber extends Model {
     return $orm;
   }
 
-  static function filterWithCustomFieldsForExport($orm) {
+  static function filterWithCustomFieldsForExport($orm, $group_by = false) {
     $orm = $orm->select(MP_SUBSCRIBERS_TABLE.'.*');
     $customFields = CustomField::findArray();
     foreach ($customFields as $customField) {
       $orm = $orm->selectExpr(
-        'CASE WHEN ' .
+        sprintf(
-        MP_CUSTOM_FIELDS_TABLE . '.id=' . $customField['id'] . ' THEN ' .
+          '%s CASE WHEN ' .
-        MP_SUBSCRIBER_CUSTOM_FIELD_TABLE . '.value END as "' . $customField['id'].'"');
+          MP_CUSTOM_FIELDS_TABLE . '.id=' . $customField['id'] . ' THEN ' .
+          MP_SUBSCRIBER_CUSTOM_FIELD_TABLE . '.value END %s as "' . $customField['id'].'"',
+          ($group_by) ? 'MAX(' : '',
+          ($group_by) ? ')' : ''
+        )
+      );
     }
     $orm = $orm
       ->leftOuterJoin(

lib/Subscribers/ImportExport/Export/Export.php

@@ -175,6 +175,10 @@ class Export {
   }
 
   function getSubscribers($offset, $limit) {
+    // if we're not grouping by segments (i.e., resulting query can return the samer
+    // subscriber but with different segments), we should return only unique subscribers
+    // by using a GROUP BY clause on subscriber id
+    $group_by_subscribers = ($this->group_by_segment_option) ? false : true;
     // JOIN subscribers on segment and subscriber_segment tables
     $subscribers = Subscriber::
     left_outer_join(
@@ -191,14 +195,20 @@ class Export {
           '=',
           SubscriberSegment::$_table . '.segment_id'
         ))
-      ->filter('filterWithCustomFieldsForExport');
+      ->filter('filterWithCustomFieldsForExport', $group_by_subscribers);
     if($this->subscribers_without_segment !== false) {
       // if there are subscribers who do not belong to any segment, use
       // a CASE function to group them under "Not In Segment"
       $subscribers = $subscribers
-        ->selectExpr('CASE WHEN ' . Segment::$_table . '.name IS NOT NULL ' .
+        ->selectExpr(
-                     'THEN ' . Segment::$_table . '.name ' .
+        // use an aggregate function when grouping by segments
-                     'ELSE "' . __('Not In Segment') . '" END as segment_name'
+          sprintf(
+            '%s CASE WHEN ' . Segment::$_table . '.name IS NOT NULL ' .
+            'THEN ' . Segment::$_table . '.name ' .
+            'ELSE "' . __('Not In Segment') . '" END %s as segment_name',
+            ($group_by_subscribers) ? 'MAX(' : '',
+            ($group_by_subscribers) ? ')' : ''
+          )
         )
         ->whereRaw(
           SubscriberSegment::$_table . '.segment_id IN (' .
@@ -207,14 +217,19 @@ class Export {
           $this->segments
         );
     } else {
-      // use an aggregate function to prevent non-deterministic GROUP BY issue
-      // in MySQL 5.7+
       $subscribers = $subscribers
-        ->selectExpr('CONCAT('.Segment::$_table . '.name) as segment_name')
+        ->selectExpr(
+        // use an aggregate function when grouping by segments
+          sprintf(
+            '%s '.Segment::$_table . '.name %s as segment_name',
+            ($group_by_subscribers) ? 'MAX(' : '',
+            ($group_by_subscribers) ? ')' : ''
+          )
+        )
         ->whereIn(SubscriberSegment::$_table . '.segment_id', $this->segments);
     }
-    if(!$this->group_by_segment_option) {
+    if($group_by_subscribers) {
-      // if grouping by segments, use a GROUP BY clause on subscriber id
+      // if not grouping by segments, use a GROUP BY clause on subscriber id
       $subscribers =
         $subscribers->groupBy(Subscriber::$_table . '.id');
     }
@@ -223,11 +238,15 @@ class Export {
       $subscribers =
         $subscribers->where(Subscriber::$_table . '.status', 'subscribed');
     }
-    $subscribers = $subscribers
+    try {
-      ->whereNull(Subscriber::$_table . '.deleted_at')
+      $subscribers = $subscribers
-      ->offset($offset)
+        ->whereNull(Subscriber::$_table . '.deleted_at')
-      ->limit($limit)
+        ->offset($offset)
-      ->findArray();
+        ->limit($limit)
+        ->findArray();
+    } catch(\PDOException $e) {
+      !ddd(\ORM::get_last_statement());
+    }
     return $subscribers;
   }