Error in SELECT statement under Postgresql [#175681]

Installed acidfree on Drupal 5.2 with Postgresql, created an album with default ordering options, and encountered an error indicating that fields in the ORDER BY clause must also be present in the SELECT field list. The original code includes a RAND() in the ORDER BY clause but not in the SELECT field list. I assume that MySQL doesn't barf in this case but Postgresql does. My patch attempts to iterate through the fields present in the ORDER BY clause and add each to the field list if it is not present. It would be simpler to merely add RAND() to the field list but I wanted to be more robust. Here's the patch:

--- orig/acidfree/acidfree.module       2007-08-14 14:02:38.000000000 -0400
+++ patched/acidfree/acidfree.module    2007-09-14 12:22:09.000000000 -0400
@@ -1915,7 +1915,21 @@
     }
     // Create count query. Drupal's pager function does not build the query properly
     $clauses = "FROM {node} n INNER JOIN {term_node} tn ON n.nid = tn.nid WHERE $include_albums tn.tid = %d AND n.status = 1";
-    $query = db_rewrite_sql("SELECT DISTINCT(n.nid), n.title, n.created " . $clauses.' ORDER BY '. $order);
+
+    $fields = 'DISTINCT(n.nid), n.title, n.created';
+
+    // Postgres requires that fields specified in ORDER BY clauses must also appear in the SELECT list.
+    if ($GLOBALS['db_type'] == 'pgsql') {
+      foreach(explode(',',$order) as $orderclause) {
+        $orderclause = explode(' ',trim($orderclause));
+        $orderfield = $orderclause[0];
+        if (!preg_match('/\b' . $orderfield . '\b/',$fields)) {
+          $fields .= ", $orderfield";
+        }
+      }
+    }
+
+    $query = db_rewrite_sql("SELECT $fields $clauses ORDER BY $order");
     $count_query = db_rewrite_sql("SELECT COUNT(DISTINCT(n.nid)) " . $clauses);
     if ($limit == -1) {
         $kids = db_query($query, $tid);

Comment	File	Size	Author
#3	safe_rand_ordering.patch	1.62 KB	vhmauery
#2	acidfree.module_5.patch	2.59 KB	pillarsdotnet
	acidfree.module_2.patch	1.18 KB	pillarsdotnet

Comments

Comment #1

pillarsdotnet commented 15 September 2007 at 16:47

Version:

5.x-1.x-dev

» master

Bah! Same problem occurs in two places. I'm copying my code rather than adding a helper function, since this is a quick-fix workaround. The right thing (tm) is probably to do the check in db_rewrite_sql().

Bumping version to HEAD. The following patch should apply with some fuzz.

--- orig/acidfree/acidfree.module       2007-08-14 14:02:38.000000000 -0400
+++ patched/acidfree/acidfree.module    2007-09-15 12:41:51.000000000 -0400
@@ -1915,7 +1908,20 @@
     }
     // Create count query. Drupal's pager function does not build the query properly
     $clauses = "FROM {node} n INNER JOIN {term_node} tn ON n.nid = tn.nid WHERE $include_albums tn.tid = %d AND n.status = 1";
-    $query = db_rewrite_sql("SELECT DISTINCT(n.nid), n.title, n.created " . $clauses.' ORDER BY '. $order);
+    $fields = 'DISTINCT(n.nid), n.title, n.created';
+
+    // Postgres requires that fields specified in ORDER BY clauses must also appear in the SELECT list.
+    if ($GLOBALS['db_type'] == 'pgsql') {
+      foreach(explode(',',$order) as $orderclause) {
+        $orderclause = explode(' ',trim($orderclause));
+        $orderfield = $orderclause[0];
+        if (!preg_match('/\b' . $orderfield . '\b/',$fields)) {
+          $fields .= ", $orderfield";
+        }
+      }
+    }
+
+    $query = db_rewrite_sql("SELECT $fields $clauses ORDER BY $order");
     $count_query = db_rewrite_sql("SELECT COUNT(DISTINCT(n.nid)) " . $clauses);
     if ($limit == -1) {
         $kids = db_query($query, $tid);
@@ -2554,7 +2564,20 @@
     if (! $nid) {
         // Create count query. Drupal's pager function does not build the query properly
         $clauses = 'SELECT DISTINCT(n.nid) FROM {node} n INNER JOIN {term_node} tn ON n.nid = tn.nid WHERE n.type <> \'acidfree\' AND tn.tid = %d AND n.status = 1 ORDER BY '. $pager_sort . " LIMIT 1 OFFSET %d";
-        $query = db_rewrite_sql($clauses);
+        $fields = 'DISTINCT(n.nid), n.title, n.created';
+
+        // Postgres requires that fields specified in ORDER BY clauses must also appear in the SELECT list.
+        if ($GLOBALS['db_type'] == 'pgsql') {
+          foreach(explode(',',$order) as $orderclause) {
+            $orderclause = explode(' ',trim($orderclause));
+            $orderfield = $orderclause[0];
+            if (!preg_match('/\b' . $orderfield . '\b/',$fields)) {
+              $fields .= ", $orderfield";
+            }
+          }
+        }
+
+        $query = db_rewrite_sql("SELECT $fields $clauses ORDER BY $order");
         $nid = db_result(db_query($query, $acidfree_pid, $offset));
         $offset_lookup[$acidfree_pid][$offset] = $nid;
         $parents = taxonomy_node_get_terms($nid);
a

Comment #2

pillarsdotnet commented 15 September 2007 at 17:57

Status	File	Size
new	acidfree.module_5.patch	2.59 KB

Okay, I flubbed that one; here's a corrected patch:

--- orig/acidfree/acidfree.module       2007-08-14 14:02:38.000000000 -0400
+++ patched/acidfree/acidfree.module    2007-09-15 13:52:28.000000000 -0400
@@ -1915,7 +1908,20 @@
     }
     // Create count query. Drupal's pager function does not build the query properly
     $clauses = "FROM {node} n INNER JOIN {term_node} tn ON n.nid = tn.nid WHERE $include_albums tn.tid = %d AND n.status = 1";
-    $query = db_rewrite_sql("SELECT DISTINCT(n.nid), n.title, n.created " . $clauses.' ORDER BY '. $order);
+    $fields = 'DISTINCT(n.nid), n.title, n.created';
+
+    // Postgres requires that fields specified in ORDER BY clauses must also appear in the SELECT list.
+    if ($GLOBALS['db_type'] == 'pgsql') {
+      foreach(explode(',',$order) as $orderclause) {
+        $orderclause = explode(' ',trim($orderclause));
+        $orderfield = $orderclause[0];
+        if (!preg_match('/\b' . $orderfield . '\b/',$fields)) {
+          $fields .= ", $orderfield";
+        }
+      }
+    }
+
+    $query = db_rewrite_sql("SELECT $fields $clauses ORDER BY $order");
     $count_query = db_rewrite_sql("SELECT COUNT(DISTINCT(n.nid)) " . $clauses);
     if ($limit == -1) {
         $kids = db_query($query, $tid);
@@ -2553,8 +2563,23 @@
     $nid = $offset_lookup[$acidfree_pid][$offset];
     if (! $nid) {
         // Create count query. Drupal's pager function does not build the query properly
-        $clauses = 'SELECT DISTINCT(n.nid) FROM {node} n INNER JOIN {term_node} tn ON n.nid = tn.nid WHERE n.type <> \'acidfree\' AND tn.tid = %d AND n.status = 1 ORDER BY '. $pager_sort . " LIMIT 1 OFFSET %d";
-        $query = db_rewrite_sql($clauses);
+        $clauses = 'FROM {node} n INNER JOIN {term_node} tn ON n.nid = tn.nid WHERE n.type <> \'acidfree\' AND tn.tid = %d AND n.status = 1';
+        $order = $pager_sort;
+        $limit = 'LIMIT 1 OFFSET %d';
+        $fields = 'DISTINCT(n.nid), n.title, n.created';
+
+        // Postgres requires that fields specified in ORDER BY clauses must also appear in the SELECT list.
+        if ($GLOBALS['db_type'] == 'pgsql') {
+          foreach(explode(',',$order) as $orderclause) {
+            $orderclause = explode(' ',trim($orderclause));
+            $orderfield = $orderclause[0];
+            if (!preg_match('/\b' . $orderfield . '\b/',$fields)) {
+              $fields .= ", $orderfield";
+            }
+          }
+        }
+
+        $query = db_rewrite_sql("SELECT $fields $clauses ORDER BY $order $limit");
         $nid = db_result(db_query($query, $acidfree_pid, $offset));
         $offset_lookup[$acidfree_pid][$offset] = $nid;
         $parents = taxonomy_node_get_terms($nid);

Comment #3

vhmauery commented 18 February 2008 at 05:46

Status	File	Size
new	safe_rand_ordering.patch	1.62 KB

Here is a much cleaner implementation. Can you try this out? Make sure you test the random items block too.

Index: acidfree.module
===================================================================
--- acidfree.module     (revision 536)
+++ acidfree.module     (working copy)
@@ -974,11 +974,14 @@
         $order = 'votes DESC';
         $join = 'INNER JOIN {node_counter} c on n.nid=c.nid ';
         $select = ', ABS(c.daycount*10 + c.totalcount) AS votes';
+    } else if ($order == 'RAND()') {
+        $order = 'rand DESC';
+        $select = ', RAND() as rand';
     }
     // FIXME: we should make this query use $acidfree_types
     if (count($albums) > 0) {
         $terms = implode(',', $albums);
-        $query = "SELECT n.nid FROM {node} n ".
+        $query = "SELECT n.nid $select FROM {node} n ".
             "INNER JOIN {term_node} t on n.nid=t.nid ".
             "WHERE n.type IN ('image', 'video') AND ".
             "t.tid IN ($terms) AND n.status=1 ".
@@ -1977,9 +1980,13 @@
     } else {
         $include_albums = "n.type <> 'acidfree' AND";
     }
+    if ($order == 'RAND()') {
+        $order = 'rand DESC';
+        $select = ', RAND() as rand';
+    }
     // Create count query. Drupal's pager function does not build the query properly
     $clauses = "FROM {node} n INNER JOIN {term_node} tn ON n.nid = tn.nid WHERE $include_albums tn.tid = %d AND n.status = 1";
-    $query = db_rewrite_sql("SELECT DISTINCT(n.nid), n.title, n.created " . $clauses.' ORDER BY '. $order);
+    $query = db_rewrite_sql("SELECT DISTINCT(n.nid), n.title, n.created $select " . $clauses.' ORDER BY '. $order);
     $count_query = db_rewrite_sql("SELECT COUNT(DISTINCT(n.nid)) " . $clauses);
     if ($limit == -1) {
         $kids = db_query($query, $tid);

Comment #4

vhmauery commented 4 March 2008 at 06:08

Status:

Needs review

» Fixed

Fixed in CVS.

Comment #5

jaydub commented 13 March 2008 at 10:32

Status:

Fixed

» Active

I am running off of -dev so I should have this fix (file is dated March 5) but there's another PostgreSQL error...

You should add the 'n.sticky' that is included in the default parameter for $order in the
_acidfree_get_children() function

function &_acidfree_get_children($tid, $limit=-1, $pagerid=0, $include_albums=false, $order='n.sticky DESC, n.nid DESC')

.. snip ...

here:

$query = db_rewrite_sql("SELECT DISTINCT(n.nid), n.title, n.created, n.sticky $select " . $clauses.' ORDER BY '. $order);

adding the 'n.sticky' to the SELECT clause gets the error to disappear.

Comment #6

vhmauery commented 14 March 2008 at 13:46

Status:

Active

» Fixed

fixed in CVS.

Comment #7

Anonymous (not verified) commented 28 March 2008 at 13:52

Status:

Fixed

» Closed (fixed)

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

Error in SELECT statement under Postgresql

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

News items

Our community

Documentation

Drupal code base

Governance of community