postgresql issue with group by clauses in argument processing

I have run into the same issue. Will patch and report back.

--
Sammy Spets
Drupal core maintainer (PostgreSQL)
Synerger Pty Ltd
http://synerger.com

ok. I've tested out the patch and it appears to work. Nice work on fixing the issue cwoodruf. The patch appears to deal with two problems so we'll have to split this out into more manageable issues.

1. (as discussed in this issue) PostgreSQL requires the non-aggregated fields to be specified in a GROUP BY clause when there is an aggregate function specified in the select list. The patch above adds support for aggregate function detection and subsequent addition of a GROUP BY containing non-aggregate fields to views_query::query(). Unfortunately, views_handler_argument::summary_basics() does not correctly setup the views_query::fields entry so that this can happen. This is the second issue.
2. Code in views_handler_argument::summary_basics() adds DISTINCT() and COUNT() to the field manually preventing views_query::query() to know the fields are DISTINCT/COUNT. Function views_query::query() requires setting of the distinct/count members of the views_query::fields elements having those properties. This is a separate issue and requires a separate issue post.

I'll add an issue to the queue for the second one and follow-up with a link to that. This issue is dependent on that one.

Ok the issue for 2) above is http://drupal.org/node/291292. Setting issue to critical priority as views does not function on postgresql. Also setting it to CNW because patches are in ed format, doesn't use drupal code style and uses an 'and' instead of an 'or' for $isaggregate and $this->groupby.

Assigning to me.

Rerolled the ed format patch and drupalized the code (true to TRUE etc). Tested it on PostgreSQL 8.2 and it works. No difference between 8.2 and 7.4 in this department so assuming it works for 7.4. Needs review on MySQL before we can set it to RTBC.

Always too quick to hit the submit button. Here's the patch. This requires the patch in http://drupal.org/node/291292 before it'll work.

Ok, so I don't have pgsql available to test on, but you are the recognized expert. The patch looks ok on a visual inspection, so I have committed this. Let's see who notices!

Hmm. I just had to roll this back; it broke MySQL.

# user warning: Can't group on 'num_records' query: SELECT COUNT(*) FROM (SELECT SUBSTR(node.title, 1, 1) AS title_truncated, COUNT(node.nid) AS num_records FROM node node GROUP BY title_truncated, title_truncated, num_records ORDER BY title_truncated ASC ) AS count_alias in /var/www/views2/sites/all/modules/views/includes/view.inc on line 652.
# user warning: Can't group on 'num_records' query: SELECT SUBSTR(node.title, 1, 1) AS title_truncated, COUNT(node.nid) AS num_records FROM node node GROUP BY title_truncated, title_truncated, num_records ORDER BY title_truncated ASC LIMIT 0, 36 in /var/www/views2/sites/all/modules/views/includes/view.inc on line 677.

Thanks for having a go at it merlinofchaos!

Yeah the patch in this issue requires the patch in http://drupal.org/node/291292 to work. Definitely won't work otherwise.

Also, the queries you pasted have group by fields duplicated. I've rerolled the patch to have a call to array_unique() around the array_merge(). Now to get that other patch thru. :)

Marking as CNR again since it wasn't reviewed with the dependency in place.

I too ran into this after trying out the Views 2 documentation 'Getting Started' examples. I use PostgreSQL for 90% of my tinkering and try to help where I can. I've generated updated patches to this issue and to #291292: Count and distinct field properties not set for summaries. I certainly don't know enough of the Views 2 internals to call this a true patch review but I'll settle for submitting a re-roll in hopes that some progress can be made here.

After applying the 2 re-rolled patches I don't run into the problems with the GROUP BY in PostgreSQL. I exported the view to a MySQL based d6 install running on the same drupal codebase and the the view runs without error.

Tested on PostgreSQL 8.3

Hopefully this time all will work!

Hi,
new postgreSQL functionality breaks grouping with mysql.
can we clear $non_aggregates if Database is mysql?

Any changes in this topic?

Adding tag

I got that problem for a long time too. Still happening in 6.x-3.x-dev...

query: SELECT DISTINCT(node.nid) AS nid_1, FIRST(node.title) AS node_title, FIRST(node.nid) AS nid,
 FIRST(GREATEST(node.changed, node_comment_statistics.last_comment_timestamp)) AS
 node_comment_statistics_last_updated, FIRST(node_revisions.teaser) AS node_revisions_teaser,
 FIRST(node_revisions.format) AS node_revisions_format FROM node node INNER JOIN term_node
 term_node ON node.vid = term_node.vid INNER JOIN node_comment_statistics
 node_comment_statistics ON node.nid = node_comment_statistics.nid LEFT JOIN node_revisions
 node_revisions ON node.vid = node_revisions.vid INNER JOIN node_access na ON na.nid = node.nid
 WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all')
 OR (na.gid = 1 AND na.realm = 'node_privacy_byrole_role') OR (na.gid = 0
 AND na.realm = 'node_privacy_byrole_user'))) AND ( (term_node.tid = 265)
 AND (node.type in ('product')) AND (node.status = 1) )ORDER BY node_title
 ASC LIMIT 30 OFFSET 0 in .../sites/all/modules/views/plugins/views_plugin_query_default.inc on line 1146.

The actual PostgreSQL error:

Query failed: ERROR: column "node.nid" must appear in the GROUP BY clause or be used in an aggregate function LINE 1: SELECT DISTINCT(node.nid) AS nid_1

Thank you.
Alexis Wilke

This distinct is added by node.module or?

No idea how this can be fixed out of views.

@AlexisWilke

By just looking at your message, there is no groupby involved here.
In general can you try to help to reproduce this problem? For example post an export with just default fields.

Okay, good point. The GRANT system adds the DISTINCT because of the Node privacy byrole module I'm using...

The DISTINCT is added with this function:

function db_distinct_field($table, $field, $query) {
  $matches = array();
  if (!preg_match('/^SELECT\s*DISTINCT/i', $query, $matches)) {
    // Only add distinct to the outer SELECT to avoid messing up subqueries.
    $query = preg_replace('/^SELECT/i', 'SELECT DISTINCT', $query);
  }

  return $query;
}

and as you can see, they do not do anything else than add the DISTINCT (i.e. no consideration for a GROUP BY.)

Someone, somewhere, calls db_rewrite_sql(), which calls the _db_rewrite_sql() internal function. That function calls all the module-name_db_rewrite_sql() functions. The node implementation looks like this:

function node_db_rewrite_sql($query, $primary_table, $primary_field) {
  if ($primary_field == 'nid' && !node_access_view_all_nodes()) {
    $return['join'] = _node_access_join_sql($primary_table);
    $return['where'] = _node_access_where_sql();
    $return['distinct'] = 1;
    return $return;
  }
}

and as we can see the $return array includes the 'distinct' flag set to 1.

On return in the db_rewrite_sql() function, this is tested first and the distinct function is called:

function db_rewrite_sql($query, $primary_table = 'n', $primary_field = 'nid',  $args = array()) {
  list($join, $where, $distinct) = _db_rewrite_sql($query, $primary_table, $primary_field, $args);

  if ($distinct) {
    $query = db_distinct_field($primary_table, $primary_field, $query);
  }

  [...]

What looks surprising is that in the error I get, the node.nid is between parenthesis. This may be because someone adds them later? (views? to and the "AS nid_1"?)

The GRANT system is something from Core so it should be fully supported by views, I think.

I would suggest a hack, something like this, forcing distinct because the GRANT system adds it and thus whether the user select that flag is not significant anymore, it is required. But I'm not too sure where it would need to be checked or whether it would be too late anyway (i.e. the db_rewrite_sql() is called after you generate the SQL order... in which case you'd have to re-generate the SQL with distinct turn on, but only if the DB is psql.)

  // test from the db_distinct_field() function
  $this->distinct = preg_match('/^SELECT\s*DISTINCT/i', $query, $matches);

That reminds me I have a few fixes (i.e. added a few GROUP BY) in Core so it works properly with PostgreSQL... So Core is really doing it wrong.

Below is my view export. Really nothing extraordinary...

Thank you.
Alexis

$view = new view;
$view->name = 'drupal_modules';
$view->description = 'Shows a list of all the Drupal modules.';
$view->tag = 'Drupal';
$view->base_table = 'node';
$view->human_name = '';
$view->core = 0;
$view->api_version = '3.0-alpha1';
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */

/* Display: Defaults */
$handler = $view->new_display('default', 'Defaults', 'default');
$handler->display->display_options['access']['type'] = 'none';
$handler->display->display_options['cache']['type'] = 'none';
$handler->display->display_options['query']['type'] = 'views_query';
$handler->display->display_options['query']['options']['distinct'] = TRUE;
$handler->display->display_options['exposed_form']['type'] = 'basic';
$handler->display->display_options['pager']['type'] = 'full';
$handler->display->display_options['pager']['options']['items_per_page'] = 30;
$handler->display->display_options['style_plugin'] = 'list';
$handler->display->display_options['row_plugin'] = 'fields';
/* Field: Node: Title */
$handler->display->display_options['fields']['title']['id'] = 'title';
$handler->display->display_options['fields']['title']['table'] = 'node';
$handler->display->display_options['fields']['title']['field'] = 'title';
$handler->display->display_options['fields']['title']['label'] = '';
$handler->display->display_options['fields']['title']['alter']['alter_text'] = 0;
$handler->display->display_options['fields']['title']['alter']['make_link'] = 0;
$handler->display->display_options['fields']['title']['alter']['word_boundary'] = 1;
$handler->display->display_options['fields']['title']['alter']['ellipsis'] = 1;
$handler->display->display_options['fields']['title']['alter']['strip_tags'] = 0;
$handler->display->display_options['fields']['title']['alter']['trim'] = 0;
$handler->display->display_options['fields']['title']['alter']['html'] = 0;
$handler->display->display_options['fields']['title']['link_to_node'] = 1;
/* Field: Node: Updated/commented date */
$handler->display->display_options['fields']['last_updated']['id'] = 'last_updated';
$handler->display->display_options['fields']['last_updated']['table'] = 'node_comment_statistics';
$handler->display->display_options['fields']['last_updated']['field'] = 'last_updated';
$handler->display->display_options['fields']['last_updated']['label'] = 'Last Updated on';
$handler->display->display_options['fields']['last_updated']['alter']['alter_text'] = 0;
$handler->display->display_options['fields']['last_updated']['alter']['make_link'] = 0;
$handler->display->display_options['fields']['last_updated']['alter']['word_boundary'] = 1;
$handler->display->display_options['fields']['last_updated']['alter']['ellipsis'] = 1;
$handler->display->display_options['fields']['last_updated']['alter']['strip_tags'] = 0;
$handler->display->display_options['fields']['last_updated']['alter']['trim'] = 0;
$handler->display->display_options['fields']['last_updated']['alter']['html'] = 0;
/* Field: Node: Teaser */
$handler->display->display_options['fields']['teaser']['id'] = 'teaser';
$handler->display->display_options['fields']['teaser']['table'] = 'node_revisions';
$handler->display->display_options['fields']['teaser']['field'] = 'teaser';
$handler->display->display_options['fields']['teaser']['label'] = '';
$handler->display->display_options['fields']['teaser']['alter']['alter_text'] = 0;
$handler->display->display_options['fields']['teaser']['alter']['make_link'] = 0;
$handler->display->display_options['fields']['teaser']['alter']['word_boundary'] = 1;
$handler->display->display_options['fields']['teaser']['alter']['ellipsis'] = 1;
$handler->display->display_options['fields']['teaser']['alter']['strip_tags'] = 0;
$handler->display->display_options['fields']['teaser']['alter']['trim'] = 0;
$handler->display->display_options['fields']['teaser']['alter']['html'] = 0;
/* Sort criterion: Node: Title */
$handler->display->display_options['sorts']['title']['id'] = 'title';
$handler->display->display_options['sorts']['title']['table'] = 'node';
$handler->display->display_options['sorts']['title']['field'] = 'title';
/* Filter: Taxonomy: Term */
$handler->display->display_options['filters']['tid']['id'] = 'tid';
$handler->display->display_options['filters']['tid']['table'] = 'term_node';
$handler->display->display_options['filters']['tid']['field'] = 'tid';
$handler->display->display_options['filters']['tid']['value'] = array(
  265 => '265',
);
$handler->display->display_options['filters']['tid']['expose']['operator'] = FALSE;
$handler->display->display_options['filters']['tid']['reduce_duplicates'] = 0;
$handler->display->display_options['filters']['tid']['type'] = 'select';
$handler->display->display_options['filters']['tid']['vid'] = '5';
$handler->display->display_options['filters']['tid']['hierarchy'] = 1;
/* Filter: Node: Type */
$handler->display->display_options['filters']['type']['id'] = 'type';
$handler->display->display_options['filters']['type']['table'] = 'node';
$handler->display->display_options['filters']['type']['field'] = 'type';
$handler->display->display_options['filters']['type']['value'] = array(
  'product' => 'product',
);
/* Filter: Node: Published */
$handler->display->display_options['filters']['status']['id'] = 'status';
$handler->display->display_options['filters']['status']['table'] = 'node';
$handler->display->display_options['filters']['status']['field'] = 'status';
$handler->display->display_options['filters']['status']['value'] = '1';
$translatables['drupal_modules'] = array(
  t('Defaults'),
  t('more'),
  t('Apply'),
  t('Reset'),
  t('Sort by'),
  t('Asc'),
  t('Desc'),
  t('Items per page'),
  t('- All -'),
  t('Offset'),
  t('Last Updated on'),
);

Nevermind...

(1) my view uses Distinct;

(2) I tried with the Disable SQL rewriting flag and it still doesn't work (no group by.)

Therefore, this is 100% a view bug!

The following is the SQL generated by Views when I edit the view with the problem (notice the lack of node protected by role references):

SELECT DISTINCT(node.nid) AS nid_1,
FIRST(node.title) AS node_title,
FIRST(node.nid) AS nid,
FIRST(GREATEST(node.changed, node_comment_statistics.last_comment_timestamp)) AS node_comment_statistics_last_updated,
FIRST(node_revisions.teaser) AS node_revisions_teaser,
FIRST(node_revisions.format) AS node_revisions_format
 FROM node node 
 INNER JOIN term_node term_node ON node.vid = term_node.vid
 INNER JOIN node_comment_statistics node_comment_statistics ON node.nid = node_comment_statistics.nid
 LEFT JOIN node_revisions node_revisions ON node.vid = node_revisions.vid
 WHERE (term_node.tid = 265) AND (node.type in ('product')) AND (node.status = 1)
   ORDER BY node_title ASC

Since right now I do not need the DISTINCT I removed that flag so at least that view works. But that means the Distinct flag doesn't work and since it is set by default, that's a problem...

Thank you.
Alexis

Tagging

The Drupal 6 branch is no longer supported, please check with the D6LTS project if you need further support. For more information as to why this issue was closed, please see issue #3030347: Plan to clean process issue queue