Using brand new install of drupal 6.3.

I am creating a list of members using some custom cck fields I set up in the content profile module.

Export of View:

$view = new view;
$view->name = 'Members_List';
$view->description = '';
$view->tag = 'members';
$view->view_php = '';
$view->base_table = 'node';
$view->is_cacheable = FALSE;
$view->api_version = 2;
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */
$handler = $view->new_display('default', 'Members', 'default');
$handler->override_option('fields', array(
  'name' => array(
    'id' => 'name',
    'table' => 'users',
    'field' => 'name',
  ),
  'picture' => array(
    'id' => 'picture',
    'table' => 'users',
    'field' => 'picture',
  ),
  'created' => array(
    'label' => 'Joined',
    'date_format' => 'time ago',
    'custom_date_format' => '',
    'exclude' => 0,
    'id' => 'created',
    'table' => 'users',
    'field' => 'created',
    'relationship' => 'none',
  ),
  'field_birthdate_value' => array(
    'label' => '',
    'link_to_node' => 0,
    'label_type' => 'widget',
    'format' => 'default',
    'multiple' => array(
      'group' => TRUE,
      'multiple_number' => '',
      'multiple_from' => '',
      'multiple_reversed' => FALSE,
    ),
    'exclude' => 0,
    'id' => 'field_birthdate_value',
    'table' => 'node_data_field_birthdate',
    'field' => 'field_birthdate_value',
    'relationship' => 'none',
  ),
  'field_orientation_value' => array(
    'label' => '',
    'link_to_node' => 0,
    'label_type' => 'widget',
    'format' => 'default',
    'multiple' => array(
      'group' => TRUE,
      'multiple_number' => '',
      'multiple_from' => '',
      'multiple_reversed' => FALSE,
    ),
    'exclude' => 0,
    'id' => 'field_orientation_value',
    'table' => 'node_data_field_orientation',
    'field' => 'field_orientation_value',
    'relationship' => 'none',
  ),
  'field_relationshipstatus_value' => array(
    'id' => 'field_relationshipstatus_value',
    'table' => 'node_data_field_relationshipstatus',
    'field' => 'field_relationshipstatus_value',
  ),
  'field_city_value' => array(
    'id' => 'field_city_value',
    'table' => 'node_data_field_city',
    'field' => 'field_city_value',
  ),
  'field_province_value' => array(
    'id' => 'field_province_value',
    'table' => 'node_data_field_province',
    'field' => 'field_province_value',
  ),
));
$handler->override_option('sorts', array(
  'name' => array(
    'order' => 'ASC',
    'id' => 'name',
    'table' => 'users',
    'field' => 'name',
    'relationship' => 'none',
  ),
));
$handler->override_option('access', array(
  'type' => 'none',
  'role' => array(),
  'perm' => '',
));
$handler->override_option('title', 'Members List');
$handler->override_option('header', 'Members List');
$handler->override_option('header_format', '1');
$handler->override_option('header_empty', 1);
$handler->override_option('use_ajax', TRUE);
$handler->override_option('use_pager', '1');
$handler->override_option('distinct', 1);
$handler->override_option('style_plugin', 'table');
$handler->override_option('style_options', array(
  'grouping' => '',
  'override' => 1,
  'sticky' => 0,
  'order' => 'asc',
  'columns' => array(
    'name' => 'name',
    'picture' => 'picture',
    'created' => 'created',
    'field_birthdate_value' => 'field_birthdate_value',
    'field_orientation_value' => 'field_orientation_value',
    'field_relationshipstatus_value' => 'field_relationshipstatus_value',
    'field_city_value' => 'field_city_value',
    'field_province_value' => 'field_province_value',
  ),
  'info' => array(
    'name' => array(
      'sortable' => 0,
      'separator' => '',
    ),
    'picture' => array(
      'sortable' => 0,
      'separator' => '',
    ),
    'created' => array(
      'sortable' => 1,
      'separator' => '',
    ),
    'field_birthdate_value' => array(
      'sortable' => 1,
      'separator' => '',
    ),
    'field_orientation_value' => array(
      'sortable' => 1,
      'separator' => '',
    ),
    'field_relationshipstatus_value' => array(
      'sortable' => 1,
      'separator' => '',
    ),
    'field_city_value' => array(
      'sortable' => 1,
      'separator' => '',
    ),
    'field_province_value' => array(
      'sortable' => 1,
      'separator' => '',
    ),
  ),
  'default' => 'name',
));
$handler = $view->new_display('page', 'Page', 'page_1');
$handler->override_option('path', 'members');
$handler->override_option('menu', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));
$handler->override_option('tab_options', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));
$handler = $view->new_display('page', 'Page', 'page_2');
$handler->override_option('path', 'members');
$handler->override_option('menu', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));
$handler->override_option('tab_options', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));
$handler = $view->new_display('page', 'Page', 'page_3');
$handler->override_option('path', 'members');
$handler->override_option('menu', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));
$handler->override_option('tab_options', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));

Export of Query:

SELECT DISTINCT(node.nid) AS nid,
   users.name AS users_name,
   users.uid AS users_uid,
   users.picture AS users_picture,
   users.created AS users_created,
   node.type AS node_type,
   node_data_field_birthdate.field_birthdate_value AS node_data_field_birthdate_field_birthdate_value,
   node_data_field_birthdate.field_birthdate_value2 AS node_data_field_birthdate_field_birthdate_value2,
   node_data_field_birthdate.nid AS node_data_field_birthdate_nid,
   node_data_field_birthdate.field_orientation_value AS node_data_field_birthdate_field_orientation_value,
   node_data_field_birthdate.field_relationshipstatus_value AS node_data_field_birthdate_field_relationshipstatus_value,
   node_data_field_birthdate.field_city_value AS node_data_field_birthdate_field_city_value,
   node_data_field_birthdate.field_province_value AS node_data_field_birthdate_field_province_value
 FROM node node 
 INNER JOIN users users ON node.uid = users.uid
 LEFT JOIN content_type_profile node_data_field_birthdate ON node.vid = node_data_field_birthdate.vid
    ORDER BY users_name ASC

Copy of error:


    * user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DISTINCT(node.nid), users.name AS users_name, users.uid AS users_uid, u' at line 1 query: SELECT COUNT(*) FROM (SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), users.name AS users_name, users.uid AS users_uid, users.picture AS users_picture, users.created AS users_created, node.type AS node_type, node_data_field_birthdate.field_birthdate_value AS node_data_field_birthdate_field_birthdate_value, node_data_field_birthdate.field_birthdate_value2 AS node_data_field_birthdate_field_birthdate_value2, node_data_field_birthdate.nid AS node_data_field_birthdate_nid, node_data_field_birthdate.field_orientation_value AS node_data_field_birthdate_field_orientation_value, node_data_field_birthdate.field_relationshipstatus_value AS node_data_field_birthdate_field_relationshipstatus_value, node_data_field_birthdate.field_city_value AS node_data_field_birthdate_field_city_value, node_data_field_birthdate.field_province_value AS node_data_field_birthdate_field_province_value FROM node node INNER JOIN users users ON node.uid = users.uid LEFT JOIN content_type_profile node_data_field_birthdate ON node.vid = node_data_field_birthdate.vid ORDER BY users_name ASC ) AS count_alias in /home/qwo/public_dev/qwo02.0/sites/default/modules/views/includes/view.inc on line 652.
    * user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DISTINCT(node.nid), users.name AS users_name, users.uid AS users_uid, u' at line 1 query: SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), users.name AS users_name, users.uid AS users_uid, users.picture AS users_picture, users.created AS users_created, node.type AS node_type, node_data_field_birthdate.field_birthdate_value AS node_data_field_birthdate_field_birthdate_value, node_data_field_birthdate.field_birthdate_value2 AS node_data_field_birthdate_field_birthdate_value2, node_data_field_birthdate.nid AS node_data_field_birthdate_nid, node_data_field_birthdate.field_orientation_value AS node_data_field_birthdate_field_orientation_value, node_data_field_birthdate.field_relationshipstatus_value AS node_data_field_birthdate_field_relationshipstatus_value, node_data_field_birthdate.field_city_value AS node_data_field_birthdate_field_city_value, node_data_field_birthdate.field_province_value AS node_data_field_birthdate_field_province_value FROM node node INNER JOIN users users ON node.uid = users.uid LEFT JOIN content_type_profile node_data_field_birthdate ON node.vid = node_data_field_birthdate.vid ORDER BY users_name ASC LIMIT 0, 10 in /home/qwo/public_dev/qwo02.0/sites/default/modules/views/includes/view.inc on line 677.
CommentFileSizeAuthor
#452 284392-fix-typo-D6.patch893 bytesbrianV
#452 284392-simplify-with-tests-db_distinct_fields-D5.patch5.54 KBbrianV
#445 284392-simplify-with-tests-db_distinct_fields-rev4.D6.fixed_.patch5.79 KBGábor Hojtsy
#397 284392-simplify-with-tests-db_distinct_fields-rev4.D6.patch11.63 KBbenoitg
#388 284392-simplify-db_distinct_fields-rev4.D6.patch6.52 KBbrianV
#381 views_missing_pager_modified_queries_report1.txt9.81 KBjannalexx
#377 284392-simplify-db_distinct_fields-rev3.D6.patch5.88 KBbrianV
#365 284392-simplify-db_distinct_fields-rev2.D6.patch5.58 KBbrianV
#362 284392-simplify-db_distinct_fields.D6.patch4.78 KBbrianV
#361 rewrite-view-export.txt11.34 KBrealityloop
#361 rewrite-sql.txt1.47 KBrealityloop
#361 rewrite-output.png27.92 KBrealityloop
#361 rewrite-vocabs.png24.8 KBrealityloop
#354 Before Patch.png103.44 KBjdwfly
#354 After Patch.png90.84 KBjdwfly
#353 views26.txt44.9 KBbenoitg
#353 views28.txt45.07 KBbenoitg
#351 views_2.6_2.8_queries.distinct.comparison.txt89.33 KBjannalexx
#349 temp_debug.patch1.45 KBbenoitg
#340 284392.2.patch14.11 KBmathieu
#339 284392.db_rewrite.patch11.38 KBmathieu
#338 284392.patch5.72 KBmathieu
#337 db_rewrite_and_tests-D6.6.patch15.79 KBbenoitg
#336 db_rewrite_and_tests-D6.5.patch6.65 KBbenoitg
#330 db_rewrite_and_tests-D6.4.patch5.69 KBbenoitg
#330 drupal_distinct_sql_misuse.sql_.txt3.16 KBbenoitg
#308 database.mysql_.inc_.patch1.23 KBopteronmx
#298 db_rewrite_and_tests-D6.3.patch7.48 KBevoltech
#257 db_rewrite_and_tests-D6.patch5.18 KBmathieu
#233 database-distinct-drupal-6.14-2_1.patch1.13 KBtyr
#230 database-distinct-drupal-6.14-3_1.patch1.75 KBJuliaKM
#227 database-distinct-drupal-6.14-3.patch1.27 KBtyr
#206 database.test3.69 KBSteven Jones
#204 database-distinct-drupal-6.14-2.patch1.13 KBtyr
#195 284392-database-distinct-drupal-6.14.patch1.1 KBSteven Jones
#189 284392-database-distinct-drupal-6.14.patch1.11 KBSteven Jones
#187 database.test2.85 KBSteven Jones
#175 debug-steps.txt732 bytesagentrickard
#175 view.txt3.12 KBagentrickard
#166 284392-database-distinct-drupal-6.14.patch1.09 KBken54671
#122 view.txt2.06 KBagentrickard
#122 284392-database-distinct.patch1.96 KBagentrickard
#93 database-D6.patch1.96 KBtassoman
#73 calendarview.txt16.03 KBsrjosh
#62 node.test5.18 KBAlice Heaton
#24 database.patch1.96 KBAlice Heaton
#24 test_patch.php_.txt1.37 KBAlice Heaton
#19 database.patch2.15 KBAlice Heaton
#18 database.patch2.01 KBAlice Heaton
#11 birthdate.JPG26.02 KBPassionate_Lass
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

Project: Views (for Drupal 7) » Drupal core
Version: 6.x-2.x-dev » 6.x-dev
Component: Views Data » node.module
Category: support » bug

Wow, and here I thought Drupal 6 would finally have fixed various db_rewrite_sql bugs.

First, thank you for this extremely completely bug report. It is nice to see these.

To summarize: db_rewrite_sql is rewriting this, which is the last step that happens; the query that Views reports to you is the unaltered version. It happens just prior to rewrite (and prior to a query substitution phase but that only replaces phrases like ***CURRENT_USER*** with the uid of the current user).

Views' version of the query does this:

SELECT DISTINCT(node.nid) AS nid,

But the query that's actually run does this:

SELECT DISTINCT(node.nid) AS DISTINCT(node.nid)

That 'nid' is being rewritten into DISTINCT(node.nid) incorrectly.

Note that it is possible this might not be entirely core's fault. Be sure to check any of your modules that might be using db_rewrite_sql and doing their own rewriting.

Passionate_Lass’s picture

You're very welcome. I just followed the instructions I got when I went to submit a report.

How would I know they are using db_rewrite_sql? I know enough to figure things out but nothing in depth code wise.

Just so I know, I'm trying to pull up all users but when I didn't turn on the distinct function it pulled up multiple versions of the same user for some reason. If I shut off distinct I get:


Members List

Members List
Name Picture Joined Birthdays Orientation Relationship Status City Province
Kristina 20 hours 41 min ago 08/13/1983 Lesbian Seeing Someone Hull Quebec
Kristina 20 hours 41 min ago
Kristina 20 hours 41 min ago
Kristina 20 hours 41 min ago
Kristina 20 hours 41 min ago
Kristina 20 hours 41 min ago

merlinofchaos’s picture

passionate_lass: You're trying to list users but your view type is set to 'node'. So what you're really listing is content attached to users. You should create a new view, whose type is set to 'users' (on the initial creation page) and you'll get much better results. That will avoid this DISTINCT stickiness, at least.

As for db_rewrite_sql, that's just something that requires familiarity with Drupal, I think; all queries will go through rewrite, unless run by user ID #1, but not all rewrites will do anything.

Passionate_Lass’s picture

Ah.

I tried using type "users" but it wouldn't let me access the fields I set up in CCK linked to the content profile node type. Umm... Does that make sense? :)

merlinofchaos’s picture

Ahh. If you have a content profile node type...there should be some way of filtering the view to just that node type, then. What module provides this? I haven't any experience with it.

Passionate_Lass’s picture

Yeah I think that is why it is glitching up. If I try the user view type it doesn't let me access node information, while if I try the node view type it lets me access user information in turn permitting me to create the listing I want.

http://drupal.org/project/content_profile

merlinofchaos’s picture

Try removing the distinct and add a Node: Type filter and select the node type you're using for your content profile. That will limit your view to just those nodes.

That module really needs some views integration so that it can create a relationship.

Passionate_Lass’s picture

I think they are trying to make the module as simple as possible. *rolls eyes* In turn cutting out views integration.

However, you are a genius! That worked perfectly! Yay!

This is totally unrelated to this issue but a question I'd like to ask since it pertains to the same view / listing. Is there a way to take the birthdate field I created and turn it into age in the view... or is that a cck dohicky? XD

Passionate_Lass’s picture

Tried using "time ago" format but it gives "24 years 49 weeks ago ago" for my age. Is there a way to tell it just to display years?

merlinofchaos’s picture

Enter '1' in the format field. Unfortunately I think that will still be '24 years ago' rather than just '24'.

Passionate_Lass’s picture

FileSize
26.02 KB

Hm.

I don't see an option for that. I uploaded an attachment for what I see when I look at the configure field window.

merlinofchaos’s picture

Oh right, that's a date.module field. You'll have to ask in the date.module queue for that one, then. It would appear that Karen didn't implement the format field that the core Views dates have.

merlinofchaos’s picture

Title: View that provides sql error » db_rewrite_sql causing issues with DISTINCT

To summarize:

SELECT DISTINCT(node.nid) AS nid ...

is being rewritten as:

SELECT DISTINCT(node.nid) AS DISTINCT(node.nid) ...
capellic’s picture

This started to happen to all my views immediately after I installed nodeaccess. Should I be reporting this over there? I did a search for db_rewrite_sql through my code base, but didn't find anything.

I even removed the unique setting in the view and it still gives me this error:

user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DISTINCT(node.nid), node_data_field_date_time_select.field_date_time_select_v' at line 1 query: SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), node_data_field_date_time_select.field_date_time_select_value2 AS node_data_field_date_time_select_field_date_time_select_value2, node.title AS node_title, node.type AS node_type, node_data_field_date_time_select.field_date_time_select_value AS node_data_field_date_time_select_field_date_time_select_value, node_data_field_date_time_select.nid AS node_data_field_date_time_select_nid FROM node node LEFT JOIN content_type_event node_data_field_date_time_select ON node.vid = node_data_field_date_time_select.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 = 'nodeaccess_rid') OR (na.gid = 0 AND na.realm = 'nodeaccess_uid') OR (na.gid = 0 AND na.realm = 'nodeaccess_author'))) AND ( (node.type in ('event')) AND (DATE_FORMAT(STR_TO_DATE(node_data_field_date_time_select.field_date_time_select_value2, '%Y-%m-%dT%T'), '%Y-%m-%d') >= '2008-08-22') )ORDER BY node_data_field_date_time_select_field_date_time_select_value ASC LIMIT 0, 5 in /Users/stephenm/Sites/bedc.dev/htdocs/sites/all/modules/views/includes/view.inc on line 681.

When I look at the SQL that Views is creating, it looks fine:

SELECT node.nid AS nid,
   node.title AS node_title,
   users.name AS users_name,
   users.uid AS users_uid,
   node_revisions.teaser AS node_revisions_teaser,
   node_revisions.format AS node_revisions_format,
   node.changed AS node_changed,
   node.created AS node_created
 FROM node node 
 INNER JOIN users users ON node.uid = users.uid
 LEFT JOIN node_revisions node_revisions ON node.vid = node_revisions.vid
 WHERE (node.type in ('resource')) AND (node.status <> 0)
   ORDER BY node_created DESC

And here are the gory details - an export of my view:

$view = new view;
$view->name = 'resources';
$view->description = 'Lists all of the resources in order of most recent first';
$view->tag = 'resources';
$view->view_php = '';
$view->base_table = 'node';
$view->is_cacheable = FALSE;
$view->api_version = 2;
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */
$handler = $view->new_display('default', 'Defaults', 'default');
$handler->override_option('fields', array(
  'title_1' => array(
    'label' => '',
    'link_to_node' => 1,
    'exclude' => 0,
    'id' => 'title_1',
    'table' => 'node',
    'field' => 'title',
    'relationship' => 'none',
  ),
  'name' => array(
    'label' => 'Posted by',
    'link_to_user' => 1,
    'exclude' => 0,
    'id' => 'name',
    'table' => 'users',
    'field' => 'name',
    'relationship' => 'none',
  ),
  'teaser' => array(
    'label' => '',
    'exclude' => 0,
    'id' => 'teaser',
    'table' => 'node_revisions',
    'field' => 'teaser',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
  'view_node' => array(
    'label' => '',
    'text' => 'read more >',
    'exclude' => 0,
    'id' => 'view_node',
    'table' => 'node',
    'field' => 'view_node',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
  'changed' => array(
    'label' => 'Updated date',
    'date_format' => 'small',
    'custom_date_format' => '',
    'exclude' => 0,
    'id' => 'changed',
    'table' => 'node',
    'field' => 'changed',
    'override' => array(
      'button' => 'Override',
    ),
    'relationship' => 'none',
  ),
));
$handler->override_option('sorts', array(
  'created' => array(
    'order' => 'DESC',
    'granularity' => 'second',
    'id' => 'created',
    'table' => 'node',
    'field' => 'created',
    'relationship' => 'none',
  ),
));
$handler->override_option('arguments', array(
  'name' => array(
    'default_action' => 'ignore',
    'style_plugin' => 'default_summary',
    'style_options' => array(),
    'wildcard' => 'all',
    'wildcard_substitution' => 'All',
    'title' => '%1',
    'default_argument_type' => 'fixed',
    'default_argument' => '',
    'validate_type' => 'none',
    'validate_fail' => 'not found',
    'glossary' => 0,
    'limit' => '0',
    'case' => 'ucwords',
    'path_case' => 'lower',
    'transform_dash' => 0,
    'add_table' => 0,
    'require_value' => 0,
    'id' => 'name',
    'table' => 'term_data',
    'field' => 'name',
    'relationship' => 'none',
    'default_options_div_prefix' => '',
    'default_argument_user' => 0,
    'default_argument_fixed' => '',
    'default_argument_php' => '',
    'validate_argument_node_type' => array(
      'forum' => 0,
      'event' => 0,
      'page' => 0,
      'resource' => 0,
      'resource_guide_page' => 0,
      'story' => 0,
      'vendor' => 0,
      'vendor_page' => 0,
    ),
    'validate_argument_node_access' => 0,
    'validate_argument_nid_type' => 'nid',
    'validate_argument_vocabulary' => array(
      '5' => 0,
      '1' => 0,
      '6' => 0,
      '3' => 0,
      '2' => 0,
    ),
    'validate_argument_type' => 'tid',
    'validate_argument_php' => '',
    'override' => array(
      'button' => 'Override',
    ),
  ),
));
$handler->override_option('filters', array(
  'type' => array(
    'operator' => 'in',
    'value' => array(
      'resource' => 'resource',
    ),
    'group' => '0',
    'exposed' => FALSE,
    'expose' => array(
      'operator' => FALSE,
      'label' => '',
    ),
    'id' => 'type',
    'table' => 'node',
    'field' => 'type',
    'relationship' => 'none',
  ),
  'status' => array(
    'operator' => '=',
    'value' => 1,
    'group' => '0',
    'exposed' => FALSE,
    'expose' => array(
      'operator' => FALSE,
      'label' => '',
    ),
    'id' => 'status',
    'table' => 'node',
    'field' => 'status',
    'relationship' => 'none',
  ),
));
$handler->override_option('access', array(
  'type' => 'role',
  'role' => array(
    '2' => 2,
  ),
  'perm' => 'access administration menu',
));
$handler->override_option('title', 'Resource Guide');
$handler->override_option('empty', 'There are no resources to meet your request in this category.');
$handler->override_option('empty_format', '3');
$handler->override_option('items_per_page', 20);
$handler->override_option('distinct', 0);
$handler = $view->new_display('page', 'Page', 'page_1');
$handler->override_option('path', 'resource-guide');
$handler->override_option('menu', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));
$handler->override_option('tab_options', array(
  'type' => 'none',
  'title' => '',
  'weight' => 0,
));
gausarts’s picture

Hi, all

I confirm comment #14. Although I have no idea if this is related to access things. This distinct error _in my case_ only appear whenever I installed modules dealing with access. This happened to me either when I install User_Relationship_Node_Access or later Friendlist_Access. The easiest solution as suggested by merlin (http://drupal.org/node/284753) is to disable distinct from views for now until this issue here is resolved. Thanks

cpelham’s picture

Is there a patch/hack for this?
As far as I can tell there have been a number of issues related to db_rewrite_sql (don't laugh if this is stating the obvious!), and it's hard for me to tell which posts/patches might apply to this particular problem (just started php class at NYU, hoping to get better at solving these things myself!).

Alice Heaton’s picture

I can confirm I have the same issue, when enabling the ACL module. I can also confirm removing 'Distinct' in my views stops the problems.

@cpelham : I tested with Drupal 6.4 and Views 2.0 rc4 ; so this issue is not solved

Alice Heaton’s picture

Component: node.module » mysql database
Status: Active » Needs work
FileSize
2.01 KB

Ok, I've pinpointed the problem. The function db_distinct_field in includes/database.mysql and includes/database.mysqli is very broken !

It will replace any occurence of (say) 'nid' found between the original 'SELECT' and a 'FROM' with 'DISTINCT(node.nid)'.

  • 'DISTINCT(node.nid) AS nid' will become 'DISTINCT(node.nid) AS DISTINCT(node.nid)
  • 'SELECT foo_node.nid FROM ...' becomes 'SELECT foo_ DISTINCT(node.nid) FROM ...'
  • 'SELECT ... FROM ... WHERE x IN (SELECT node.nid FROM ...)' becomes 'SELECT ... FROM ... WHERE x IN (SELECT DISTINCT(node.nid) FROM ...)'

I've seen all the cases above happen on my test sites. This is not a node.module issue, but a mysql database issue ; so I'm moving it there (not sure if it should go to 'database system' or to 'mysql database').

Attached is a patch (against Drupal 6.4) that fixes the problem on my test site. Anyone can test it to confirm ? It's not as terse as the original, and will most likely run slower, but at least it works ;)

Alice Heaton’s picture

FileSize
2.15 KB

Sorry, same patch as above, but ran from the Drupal root rather than from the includes directory.

merlinofchaos’s picture

Shouldn't you have marked this needs review, not needs work?

aangel’s picture

I can confirm on 6.4 with views 6.x-2.0-rc4 and cck 6.x-2.0-rc8 that this cleared up the distinct(nid) problem that showed up only when I installed tac_lite.

(I haven't reviewed the code, though -- not quite sure what I would need to be on the lookout for.)

Alice Heaton’s picture

@merlinofchaos : I've put it on "needs work" because I think it can be optimised. The original code is just one preg_replace ; my version has 2 ifs, 2 preg_match, 1 preg_replace and 1 str_replace...

It's more of a proof of concept (ie. to show that this is indeed where the bug is).

Of course it's better to have slow working code than fast buggy code, but I'm sure my version can be optimised with a bit of thinking :)

merlinofchaos’s picture

Ok, just bear in mind that patches marked CNW will get less attention than CNR.

Alice Heaton’s picture

Status: Needs work » Needs review
FileSize
1.37 KB
1.96 KB

Ok, I've put my thinking cap on and wrote a more optimised version. I don't think this can be written with only one preg_replace as per the original (because you need to cater for the possibility of having sub-queries) ; this version has one if, one preg_match and one preg_replace.

It works on my site, and I've written a test script to check for various potential problems. Both are attached ; the latest patch as 'database.patch' and the test as 'test_patch.php' (this is just my own test - I'm yet to look up how Drupal tests work...)

Now set to 'code needs review' !

Paulo Rodrigues’s picture

Applied the patch and all content was gone. Unapplied the patch, and all content was still gone. Rebuilt permissions and content was fine, views included. No more errors. Odd...

Alice Heaton’s picture

@Paulo Rodrigues : From what you say, it sounds like the patch affected the queries used by some access modules to evaluate grants. Which access module are you using ? The patch is stricter than the original code - it won't rewrite some queries that the original would (wrongly) have ; so maybe there's a problem with the access module expecting the old (wrong) behaviour ?

Or maybe there's a problem with the patch ! Would be great to be able to see the generated SQL to see what the problem is - did you get any mysql errors ? Can you try logging queries with the Developer module and looking at those ?

sars’s picture

Interesting, when the official solution of this problem can be expected?

netbear’s picture

Just to subscribe and to say that have absolutely the same problem: views worked fine untill I've installed Content access module. After that the problem with DISTINCT(node.nid) appeared.

Alice Heaton’s picture

@netbear : Did you try the patch in #24 ? Would be great to have some more feedback on that one.

Robin Monks’s picture

subscribing

netbear’s picture

@Anselm Heaton
No, I haven't yet, but I solved my problem with setting my view property DISTINCT to "Off".

quioxte’s picture

how do you specificly set the view property to "off"

keesee’s picture

I ran into the same problem with Domain Access. Subscribing. Thx merlin for pointing me this way.
--
J.

agentrickard’s picture

Not to pile on too much, but does the patch also address the DISTINCT issue with pager queries, which is related?

#264092: pager count(*) error

ichsan’s picture

Thanks man. It works!!! I use distinct for views and acces_content module. The errors are now gone!

Anonymous’s picture

Netbear at #31 said:

No, I haven't yet, but I solved my problem with setting my view property DISTINCT to "Off".

Ditto here. Thanks.

bkoether’s picture

I had the same problem with Drupal 6.9 and Views 6.x-2.2 and the patch in #24 works great.

Ben

gengel’s picture

subscribing

Psikik’s picture

Subscribing.

Timo.Kissing’s picture

Subscribe.

jannalexx’s picture

patch #24 as workaround in views (pager issues with access modules and db_rewrite_sql - distinct error messages)
ACL http://drupal.org/node/367761
VIEWS http://drupal.org/node/366419
works ok for now, thanks

smithn.nc’s picture

Subscribing.
Edit:
I also just applied the patch to a test copy of the site I'm working on. All seems to be well now, no more problems with DISTINCT.

KarenS’s picture

Subscribing, this function has interfered with Views for as long as I have been using it (2-3 years). It would be *really* nice to get this fixed in D6 (and I assume the problem still exists in D7). It is a horrible problem that needlessly breaks perfectly good Views SQL.

tiantian20007’s picture

I applied the patch to my site. Yes, the old problem is gone, however, a new conflict came up. One of my block built by views goes wrong. The content in this block has been showed twice. I don't think it's a good idea to change sth in the database.mysql.inc for a module. It do cause other problems!

KarenS’s picture

This is a patch that is badly needed in many situations. If you're going to report that it creates other problems it would be better to provide complete details about how to reproduce any problems it caused. It's hard to imagine any way that this patch would cause a block to appear twice.

Alice Heaton’s picture

@tiantian20007 : Thanks for this. As KarenS says, this is an important patch, so if you could help us test it and fix potential problems with it, that would be very helpful. I can see two things happening :

1. You had not selected 'DISTINCT' in the view for your block, and it was set (wrongly) by the old code. The new code does not set it, so you have to specify manually in your view. Can you edit the view displayed in your block, and in "basic settings" set "Distinct" to "Yes".

Does that fix your problem ? If so the problem is not with the patch :)

2. If the above does not fix your problem, then the problem might reside with the patch not applying "DISTINCT" when it should. In that case could you install the Devel module, and enable query logging and displaying. Then view the same page, once with the patch and once without.

In both cases, grab the generated queries (will be displayed below your page normally) and send both versions here. If you have the skills to identify which query generates the block, please just send just both versions of that one, otherwise send it all and I'll try and work it out.

I know this is a bit of work, but if you have the time it would benefit the whole of the Drupal comunity :)

dkane’s picture

I have never gotten the error listed above, just when I clicked on distinct it wouldn't provide distinct results (it didn't seem to do anything really). I applied the patch in #24 successfully to root/includes/database.mysql.inc and root/includes/database.mysqli.inc but I am still not returning distinct results.

Any Ideas?

Alice Heaton’s picture

@dkane : You seem to be having a different issue. The bug mentioned here actually generates incorrect SQL (can happen with a combination of Views and access control modules). So the symptoms would be SQL errors, either visible on your screen or on your logs. If you do not have those symptoms, I suggest you open a different ticket for you issue (I would assume on the Views module).

cpelham’s picture

As I recall (please correct me if I recall wrongly) Earl has written that the failures of the Distinct setting in the Views UI (such as displaying no nodes at all) is caused by a bug in the db_rewrie_sql. This particular thread is titled db_rewrite_sql causing issues [plural] with DISTINCT. This is an issue with distinct so I think it is correct to reference here in this thread and to await a solution via the patch that will eventually come from this thread.

Anonymous’s picture

Hey Everyone. I think this issue has to do with my bug. All of my views-lists are showing multiple items of the same node - in this case, blog entries. I believe it has to do with the caching associated with VotingAPI since the lists are sorted by votes - and the multiple entries will display because the exact same entries will have different vote numbers. I am using the Plus1 module to tally votes. Basically, I want to be able to select "Distinct" within my view to solve this problem. But whenever I select "Distinct" - I am given a SQL Error, which I copied below. The query and an export of the view are also copied below.

I believe the Patch should be a fix for this problem. Is that correct? If so, could someone just provide me with some more detailed help on how to install the patch. I'm a bit of a Drupal newbie here and learning as I go. Is it the .patch file I should be using or the .txt file? And am I copying and pasting that as a text file into my root directory or a different directory? Or am I appending that code as text to an existing file? Just a little unclear on what I'm supposed to be doing.

Thanks so much for your help. I really hope this solves my problem and I will definitely report back!!

Best,
Michael

Further Information on My Bug:

When I try to select Distinct on all these lists, I am given the following error:

user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DISTINCT(node.nid), users.name AS users_name, users.uid AS users_uid, n' at line 1 query: SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), users.name AS users_name, users.uid AS users_uid, node.title AS node_title, votingapi_cache_node_points_vote.value AS votingapi_cache_node_points_vote_value, node.created AS node_created FROM node node LEFT JOIN votingapi_cache votingapi_cache_node_points_vote ON node.nid = votingapi_cache_node_points_vote.content_id AND (votingapi_cache_node_points_vote.content_type = 'node' AND votingapi_cache_node_points_vote.value_type = 'points' AND votingapi_cache_node_points_vote.tag = 'vote') INNER JOIN term_node term_node ON node.vid = term_node.vid INNER JOIN users users ON node.uid = users.uid WHERE (term_node.tid = 28) AND (node.type in ('blog')) ORDER BY votingapi_cache_node_points_vote_value DESC LIMIT 0, 10 in /home/forens7/public_html/modules/views/includes/view.inc on line 731.

Here's the Query from the live preview:

SELECT node.nid AS nid, users.name AS users_name, users.uid AS users_uid, node.title AS node_title, votingapi_cache_node_points_vote.value AS votingapi_cache_node_points_vote_value, node.created AS node_created FROM node node LEFT JOIN votingapi_cache votingapi_cache_node_points_vote ON node.nid = votingapi_cache_node_points_vote.content_id AND (votingapi_cache_node_points_vote.content_type = 'node' AND votingapi_cache_node_points_vote.value_type = 'points' AND votingapi_cache_node_points_vote.tag = 'vote') INNER JOIN term_node term_node ON node.vid = term_node.vid INNER JOIN users users ON node.uid = users.uid WHERE (term_node.tid = 28) AND (node.type in ('blog')) ORDER BY votingapi_cache_node_points_vote_value DESC

Here's an export of the view:


$view = new view;
$view->name = 'Best_LD_Blogs';
$view->description = 'The most recommended blogs within the L-D category. ';
$view->tag = '';
$view->view_php = '';
$view->base_table = 'node';
$view->is_cacheable = FALSE;
$view->api_version = 2;
$view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */
$handler = $view->new_display('default', 'Defaults', 'default');
$handler->override_option('relationships', array(
  'votingapi_cache' => array(
    'label' => 'Voting results',
    'required' => 0,
    'votingapi' => array(
      'value_type' => 'points',
      'tag' => 'vote',
      'function' => '',
    ),
    'id' => 'votingapi_cache',
    'table' => 'node',
    'field' => 'votingapi_cache',
    'relationship' => 'none',
  ),
));
$handler->override_option('fields', array(
  'name' => array(
    'label' => 'Name',
    'link_to_user' => 1,
    'exclude' => 0,
    'id' => 'name',
    'table' => 'users',
    'field' => 'name',
    'relationship' => 'none',
  ),
  'title' => array(
    'label' => 'Title',
    'link_to_node' => 1,
    'exclude' => 0,
    'id' => 'title',
    'table' => 'node',
    'field' => 'title',
    'relationship' => 'none',
  ),
  'value' => array(
    'label' => 'Votes',
    'set_precision' => FALSE,
    'precision' => 0,
    'decimal' => '.',
    'separator' => ',',
    'prefix' => '',
    'suffix' => '',
    'appearance' => '',
    'exclude' => 0,
    'id' => 'value',
    'table' => 'votingapi_cache',
    'field' => 'value',
    'relationship' => 'votingapi_cache',
  ),
  'created' => array(
    'label' => 'Post date',
    'date_format' => 'small',
    'custom_date_format' => '',
    'exclude' => 0,
    'id' => 'created',
    'table' => 'node',
    'field' => 'created',
    'relationship' => 'none',
  ),
));
$handler->override_option('filters', array(
  'tid' => array(
    'operator' => 'or',
    'value' => array(
      '28' => '28',
    ),
    'group' => '0',
    'exposed' => FALSE,
    'expose' => array(
      'operator' => FALSE,
      'label' => '',
    ),
    'type' => 'select',
    'vid' => '2',
    'id' => 'tid',
    'table' => 'term_node',
    'field' => 'tid',
    'hierarchy' => 0,
    'relationship' => 'none',
    'reduce_duplicates' => 0,
  ),
  'type' => array(
    'operator' => 'in',
    'value' => array(
      'blog' => 'blog',
    ),
    'group' => '0',
    'exposed' => FALSE,
    'expose' => array(
      'operator' => FALSE,
      'label' => '',
    ),
    'id' => 'type',
    'table' => 'node',
    'field' => 'type',
    'relationship' => 'none',
  ),
));
$handler->override_option('access', array(
  'type' => 'none',
));
$handler->override_option('title', 'Lincoln Douglas Debate');
$handler->override_option('header', '<br> 

<h2><table> <tr> <td> <strong> Best L-D Blogs </strong></td> <td> <a href="http://www.forensicscommunity.com/forum/2"> Discuss L-D Debate </a></td>  </table> </h2> <br> 

');
$handler->override_option('header_format', '2');
$handler->override_option('header_empty', 1);
$handler->override_option('footer', '<br> <br> 

<br> <br> <center> <h2> <b> Lincoln-Douglas Debate Videos </b> </h2> <br> </center> 

<center>
<?php
   $params[\'width\'] = 652;
   $params[\'height\'] = 432;
   $params[\'playlist\'] = \'ld_videos\';
   print dashplayer_get_player($params);
 </center>
');
$handler->override_option('footer_format', '3');
$handler->override_option('footer_empty', 1);
$handler->override_option('distinct', 0);
$handler->override_option('style_plugin', 'table');
$handler->override_option('style_options', array(
  'grouping' => '',
  'override' => 1,
  'sticky' => 0,
  'order' => 'desc',
  'columns' => array(
    'name' => 'name',
    'title' => 'title',
    'value' => 'value',
    'created' => 'created',
  ),
  'info' => array(
    'name' => array(
      'sortable' => 0,
      'separator' => '',
    ),
    'title' => array(
      'sortable' => 0,
      'separator' => '',
    ),
    'value' => array(
      'sortable' => 0,
      'separator' => '',
    ),
    'created' => array(
      'sortable' => 0,
      'separator' => '',
    ),
  ),
  'default' => 'value',
));
$handler = $view->new_display('block', 'Block', 'block_1');
$handler->override_option('block_description', '');
$handler->override_option('block_caching', -1);
$handler = $view->new_display('page', 'Page', 'page_1');
$handler->override_option('path', 'lincolndouglas');
$handler->override_option('menu', array(
  'type' => 'none',
  'title' => '',
  'description' => '',
  'weight' => 0,
  'name' => 'navigation',
));
$handler->override_option('tab_options', array(
  'type' => 'none',
  'title' => '',
  'description' => '',
  'weight' => 0,
));
Anonymous’s picture

Okay, nevermind my last post.

I posted the issue as a bug earlier than this comment, and I found out that it was a very simple easy fix that doesn't need a patch.

All I was forgetting to do was to set my Voting API Relationship to something other than "no filtering" within the View. Silly me. I'm happily relieved it was a simple issue. Ignore my last comment. Thanks anyway!!

KarenS’s picture

This issue is getting terribly polluted with unrelated issues. I think we have to ignore the comment in #44 since it provided no information and there was not follow up to produce any evidence that this patch caused any problems.

The current patch is the one in #24. @Anselm Heaton, can you re-roll and add a simpletest to your patch using your examples? Then we would not only fix this problem but have a method to keep it from popping up again.

KarenS’s picture

Version: 6.x-dev » 7.x-dev

And we need to fix it in HEAD first, then backport it.

Status: Needs review » Needs work

The last submitted patch failed testing.

Dave Reid’s picture

Version: 7.x-dev » 6.x-dev

db_rewrite_sql() is not going to be in 7.x anymore (thank goodness... no more SQL string regex!). This is only going to affect 6.x.

KarenS’s picture

Status: Needs work » Needs review

Yay! I didn't realize that db_rewrite_sql() is gone. Then we don't need tests (wouldn't hurt either, but not required). Resetting status because it failed testing because it isn't a D7 patch.

voxpelli’s picture

I applied it to a site and it removed the errors while seemingly not breaking any of the access protection.
Hasn't thoroughly tested it though - but I think it's a very important/urgent patch.

merlinofchaos’s picture

Priority: Normal » Critical

I agree, the breakage here is actually critical.

bigheadfish’s picture

Before I upgraded from 6.9 to 6.10, my Views worked properly. After upgrading, I got this error in some of my views.

http://drupal.org/node/404482

KarenS’s picture

@bigheadfish, your report is muddying the waters here. We need to figure out if this patch is working. You didn't apply the patch, you're just re-reporting the original problem. If you want to help fix it, apply the patch and see if it fixes your problem.

bigheadfish’s picture

I did not try the patch.

I use str_replace (safer than preg_replace) right before line 731 in view.inc. It is definitely a very bad practice but it works for now. I will wait till the official bug fix is released.

Alice Heaton’s picture

FileSize
5.18 KB

Here is SimpleTest test file for this problem. For the moment it contains 12 unit tests, all testing the db_distinct_field function. Feel free to add more ;)

I couldn't get SimpleTest to pick it up from the includes/ folder (in Drupal 6), so I've added (randomly) to the node module, and the group 'Core'. Sorry this is the first time I try SimpleTest, let me know if you know what I did wrong ;)

Without the patch :
5 passes
7 failures

With the patch in #24 :
12 passes

radj’s picture

It fixes the view problem but two other problems popped out:

1. some of my primary link menu items didn't appear. it caused an SQL error:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near &#039;FROM node n WHERE n.status = 1 AND n.nid IN (7, 18, 282)&#039; at line 1 query: SELECT FROM node n WHERE n.status = 1 AND n.nid IN (7, 18, 282) in /home/test/public_html/includes/menu.inc on line 991.

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near &#039;FROM node n WHERE n.status = 1 AND n.nid IN (183)&#039; at line 1 query: SELECT FROM node n WHERE n.status = 1 AND n.nid IN (183) in /home/test/public_html/includes/menu.inc on line 991.

If you notice, no SELECT field is there.

2. In the forums, no posts are listed under a certain forum even though there are supposed to be items underneath and when looking in site/forum, no latest posts are recognized. SQL error:
You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near &#039; r.tid, n.title, n.type, n.sticky, u.name, u.uid, n.created AS timestamp, n.comm&#039; at line 1 query: SELECT , r.tid, n.title, n.type, n.sticky, u.name, u.uid, n.created AS timestamp, n.comment AS comment_mode, l.last_comment_timestamp, IF(l.last_comment_uid != 0, cu.name, l.last_comment_name) AS last_comment_name, l.last_comment_uid, l.comment_count AS num_comments, f.tid AS forum_tid FROM node_comment_statistics l INNER JOIN node n ON n.nid = l.nid INNER JOIN users cu ON l.last_comment_uid = cu.uid INNER JOIN term_node r ON n.vid = r.vid INNER JOIN users u ON n.uid = u.uid INNER JOIN forum f ON n.vid = f.vid WHERE n.status = 1 AND r.tid = 117 ORDER BY n.sticky DESC, l.last_comment_timestamp DESC, n.created DESC LIMIT 0, 25 in /home/norkisgroup/public_html/modules/forum/forum.module on line 588

Same problem as 1st, there's a problem in the select fields.

Harry Slaughter’s picture

Component: mysql database » database system

This critical bug is not to hard to tickle if you're using views and node access. I've been bitten a few times by this.

The the problems easily waterfalls because if you can't use a module such as workflow_access, you're not really able to establish sane read/write permissions based on workflow states of a given node. You must either give 'administer nodes' permission to editor/moderator type roles or set node.status to TRUE prematurely to allow these people to view nodes (which they can already edit if they have 'edit any XXX content' permission, and this of course means that anyone can view nodes that may not be considered public in terms of workflow state. eeep.

This problem is pretty easy to reproduce:

* enable views, workflow and workflow access
* apply a workflow to nodetype XXXX
* create a node view for XXXX nodes that uses the 'distinct' filter.
* access the created view

ntt’s picture

subscribing

ntt’s picture

I applied the patch in #24 and it appears to fix the DISTINCT issues I was having, but it appears to be creating a new problem.

The following error looks like it is coming from the simplenews module:

user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near ' n.title, sn.s_status FROM node n INNER JOIN term_node t ON n.vid = t.vid INNER ' at line 1 query: SELECT , n.title, sn.s_status FROM node n INNER JOIN term_node t ON n.vid = t.vid INNER JOIN simplenews_newsletters sn ON n.nid = sn.nid INNER JOIN node_access na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 2 AND na.realm = 'workflow_access') OR (na.gid = 3 AND na.realm = 'workflow_access') OR (na.gid = 703 AND na.realm = 'workflow_access_owner'))) AND ((n.moderate != 1 OR n.uid = 703)) AND ( (t.tid = 25 AND n.status = 1) )ORDER BY n.created DESC LIMIT 0, 5 in [DRUPAL_ROOT]/sites/all/modules/simplenews/simplenews.module on line 1234.
mbria’s picture

subscribing

Alice Heaton’s picture

@radj, @ntt

You seem to be having the same issue. It's not easy to reproduce (my forums work well) - do you think you could post the SQL statement WITHOUT the patch ? Thanks :)

In the mean time I'll see if I can work out in code cases where it would get this wrong.

Alice Heaton’s picture

@radj, @ntt :

I've tried the queries generated by menu.inc and simplenews.inc, and on my site they get rewritten properly. So we're having a clash with another module here; it's not really possible to know which. db_distinct_field is called after hook_db_rewrite_sql, so I assume one module is rewriting the query in a way that confuses the patched db_distinct_field.

Though I can't see how that happens, so I'll really need to look at one of the queries in question. Could you try to work out which query is failing ? If you have a debugger, you could set a watchpoint so see when the query gets rewritten badly. Otherwise, you could try the following old style debugging :

- edit includes/database.inc ;
- find "function db_rewrite_sql" and locate

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

- Replace that with :

  if ($distinct) {
    $old_query = $query;
    $query = db_distinct_field($primary_table, $primary_field, $query);
    if (preg_match('/SELECT\s+(FROM|,)/', $query) {
      echo "<h1>Rewrite error!</h1>Query '$old_query' became '$query' for '$primary_table . $primary_field' .<br/>";
    } 
  }

This should output some stuff - can you copy it here (or send it to me directly).

Thanks :)

fpedraza’s picture

Hi, I was having a serious issue with Drupal 6.10, Views 6.x-2.5 and Taxonomy Access Control 6.x-1.x-dev

When TAC was enabled, views did not work at all (no pager, no view, nothing), except one view of users, because of the famous query rewrite issue. With patch in #24 everything works again and I haven't had any collateral damages.

Thanks Anselm!

yasir farooqui’s picture

Patch #24 worked for me! Initially it did not work and I thought there is some problem in the patch, but applying the patch and then clearing the cache solved my issue.

Thanks

srjosh’s picture

Patch #24 also worked for me. I am running:
D6.10
ACL 6.x-1.0-beta4
Forum Access 6.x-1.0-beta3
Calendar 6.x-2.1
Date 6.x-2.x-dev (Patched as in #409476: date_timezone_set() & date_format() error when editing CCK date repeating fields.)

srjosh’s picture

FileSize
16.03 KB

Addition: couldn't print view to page; problematic view attached as text file. Hope that's ok. It's a pretty standard calendar view.

godo’s picture

Just to subscribe

exprairiot’s picture

The patch works for me in both 6.9 and 6.10.

Thanks for the great work on this.

drupal999’s picture

subscribe

squarecandy’s picture

scrubscribulating....

KarenS’s picture

The patch in #24 worked great for me to fix the problems I found using Organic Groups access control in #384650: Node distinct broken when using node access control. I haven't run into any other problems so far.

#66 seems to be coming from 'workflow_access'. I don't know anything about that module.

KarenS’s picture

Status: Needs review » Reviewed & tested by the community

Also, it is almost certain that #66 is reporting a problem that already exists, not a new problem caused by this patch.

This is a huge problem for many. This patch appears to fix many critical problems and I'd vote to commit it even if it doesn't find and fix every problem. Then there could be follow-on patches for additional problems if someone finds a solution for them.

If we get new information that #66 is not also broken in the current, unpatched, code, then the code needs work, otherwise I would venture to mark it ready to commit.

KarenS’s picture

Status: Reviewed & tested by the community » Needs work

Bah, reading more carefully, #66 says it is a new problem. We need more information about that I guess. Looks like is some combination of Workflow access and Simplenews, but there is not enough information in the report for anyone to be able to test it. In particular, a copy of the view so someone is able to try to reproduce it.

I'm not using any of those modules, so I can't do it.

KarenS’s picture

There is an existing issue in the workflow issue queue that says workflow access is broken now in the same way all the other modules are broken, which should imply that it needs the same fix. I've posted on that issue #354745: Workflow Access breaks Views that utilize the "distinct" basic setting to try to get some users of that module to come and test this patch.

bnice5000’s picture

subscribe

agerson’s picture

subscribe

Taxoman’s picture

Subscribing.

suit4’s picture

So this patch is worth trying. Works for me!

chawl’s picture

#24 worked for 6.12 and saved my life.

This should be, has to be, must be committed to the next release.

Tx.

swentel’s picture

Subscribe, need to follow up when core is updated.

Stalski’s picture

Subscribing

Albert Volkman’s picture

subscribin'

neorg’s picture

I had a the following problem: "Module "content_access" in combination with view "date_browser" give MYSQL error" (see: http://drupal.org/node/465146 )

Patch from #24 solved the probeln for me.

  • Drupal 6.12
  • Views 6.x-2.5
  • Content Access 6.x-1.1
  • jim0203’s picture

    Patch @ #24 worked for me too. Was having problems with Views Calc.

    bohz’s picture

    Patch @#24 worked for my case too.

    I have a simple view listing multiple imagefield fields from different nodes using user:User ID from URL as argument.
    Before patching the pager wouldn't show up in the view. Now it works properly and no other issues arose, so far.
    For the record: I was not using distinct setting in my view (distinct:no)
    Thanks a lot!

    tassoman’s picture

    FileSize
    1.96 KB

    Let's try to pass the automated test....

    kentr’s picture

    subscribe

    Aren Cambre’s picture

    Patch in #24 fixed it for me.

    Agileware’s picture

    The patch in #24/#93 (same patch) fixed it for me too. I haven't seen any adverse affects yet.

    Harry Slaughter’s picture

    Issue tags: +select distinct as distinct bug

    I've applied #24 patch and it seems to work. haven't done extensive testing.

    I hope this critical problem catches the attention of the core team. It's a pretty significant bug.

    joelstein’s picture

    subscribing

    KarenS’s picture

    Status: Needs work » Needs review

    OK, we have lots and lots of reports that #24 works to fix the problem. There are, unfortunately, a couple reports saying it didn't. But no one who reported that it didn't work provided enough information for anyone to do anything with, and none of them have come back to provide more clarification. So effectively, they derailed this patch without doing anything to help fix the problem.

    I'm going to try marking this back to 'needs review' and see where it goes from here. I also blogged about it and posted a temporary workaround at http://www.lullabot.com/blog/views-distinct-node-access-problems, which will hopefully be helpful to people with this problem and get more eyes on this issue.

    agentrickard’s picture

    I will try to carve out some time to run some tests using Domain Access.

    kentr’s picture

    OK, we have lots and lots of reports that #24 works to fix the problem. There are, unfortunately, a couple reports saying it didn't. But no one who reported that it didn't work provided enough information for anyone to do anything with, and none of them have come back to provide more clarification. So effectively, they derailed this patch without doing anything to help fix the problem.

    What would be helpful from others at this point?

    agentrickard’s picture

    Testing against a variety of node access modules.

    Someone should test against OG and TAC as well. You need the folks who use and maintain Node Access modules to test this.

    KarenS’s picture

    #25, #63, #66 were reports that it broke things, but none of them came back to respond with more information that could be used to determine whether there is a definite problem or not. If we could get some resolution on those it would help a lot, otherwise at some point we will, I guess, just have to ignore them.

    For the rest, saying 'it works' isn't particularly helpful. Saying 'it worked for me to fix a broken view when using the XXX node access module' is much more useful. Saying 'it fixed my broken query that looked like XXXX and correctly rewrote it to YYYY' is even better.

    Ryanbach’s picture

    Subscribing.

    Agileware’s picture

    When I said the patch fixed my problem in #96 my problem was that using distinct in a node view was returning no records when it should have been.

    I am not using any node access modules but I am using og, og_user_roles and og_vocab.

    KarenS’s picture

    Organic groups is a node access module.

    febbraro’s picture

    subscribing

    pribeh’s picture

    subscribing

    agentrickard’s picture

    @KarenS

    This is on my list to test as part of the Node Access cleanup for D7, so I should get to poke at it this week.

    ge’s picture

    I have two nearly identical servers, both running the same version of Drupal & modules against the same database back-end. Organic Groups is installed but the problem query was a Taxonomy query. Here are all the modules installed (not all are enabled):

    calendar
    cck
    content_access
    customerror
    date
    fckeditor
    filefield
    imce
    jquerymenu
    menu_per_role
    node_privacy_byrole
    og
    panels
    pathauto
    rules
    token
    views
    workflow

    I applied the patch on only one of the servers. Here are the queries generated by the View:

    SQL with Distinct set to No

    SELECT term_data.name AS term_data_name,
       COUNT(node.nid) AS num_records
     FROM node node 
     LEFT JOIN term_node term_node ON node.vid = term_node.vid
     INNER JOIN term_data term_data ON term_node.tid = term_data.tid
     WHERE node.status <> 0 OR (node.uid = ***CURRENT_USER*** AND ***CURRENT_USER*** <> 0) OR ***ADMINISTER_NODES*** = 1
     GROUP BY term_data_name
      ORDER BY term_data_name ASC

    SQL with Distinct set to Yes

    SELECT term_data.name AS term_data_name,
       COUNT(DISTINCT(node.nid)) AS num_records
     FROM node node 
     LEFT JOIN term_node term_node ON node.vid = term_node.vid
     INNER JOIN term_data term_data ON term_node.tid = term_data.tid
     WHERE node.status <> 0 OR (node.uid = ***CURRENT_USER*** AND ***CURRENT_USER*** <> 0) OR ***ADMINISTER_NODES*** = 1
     GROUP BY term_data_name
      ORDER BY term_data_name ASC

    Result on unpatched server

    user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DISTINCT(node.nid), term_data.name AS term_data_name FROM node node LEFT J' at line 1 query: SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), term_data.name AS term_data_name FROM node node LEFT JOIN term_node term_node ON node.vid = term_node.vid INNER JOIN term_data term_data ON term_node.tid = term_data.tid 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 = 0 AND na.realm = 'content_access_author') OR (na.gid = 1 AND na.realm = 'content_access_rid') OR (na.gid = 0 AND na.realm = 'og_public'))) AND ( (node.status <> 0 OR (node.uid = 0 AND 0 <> 0) OR 0 = 1) AND (term_data.name = 'Materials Management') )ORDER BY term_data_name ASC in /usr/local/drupal/sites/all/modules/views/includes/view.inc on line 759.

    Result on patched server

    No error

    VenDG’s picture

    subscribing

    finedesign’s picture

    subscribing

    p_alexander’s picture

    Confirming that the patch in #24/93 fixed my problem running a view using Drupal 6.12, Views 2.6, OG 2.0 RC3.

    A view using "Distinct" set to yes caused an error. There were two associated queries being rewritten as below:

    SELECT COUNT(*) FROM (SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), node.type AS node_type, node.vid AS node_vid, node.title AS node_title FROM node node LEFT JOIN content_field_workshop_date node_data_field_workshop_date ON node.vid = node_data_field_workshop_date.vid WHERE (node.status <> 0) AND (node.type IN ('product','workshop')) AND (node.type in ('workshop')) ORDER BY node_vid ASC ) count_alias
    
    SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), node.type AS node_type, node.vid AS node_vid, node.title AS node_title FROM node node LEFT JOIN content_field_workshop_date node_data_field_workshop_date ON node.vid = node_data_field_workshop_date.vid WHERE (node.status <> 0) AND (node.type IN ('product','workshop')) AND (node.type in ('workshop')) ORDER BY node_vid ASC LIMIT 0, 25
    

    These are the same queries after the patch:

    SELECT COUNT(*) FROM (SELECT DISTINCT(node.nid) AS nid, node.type AS node_type, node.vid AS node_vid, node.title AS node_title FROM node node INNER JOIN term_node term_node ON node.vid = term_node.vid LEFT JOIN content_field_workshop_date node_data_field_workshop_date ON node.vid = node_data_field_workshop_date.vid WHERE (node.status <> 0) AND (node.type IN ('product','workshop')) AND (node.type in ('workshop')) AND (term_node.tid = 3) ORDER BY node_vid ASC ) count_alias
    
    SELECT DISTINCT(node.nid) AS nid, node.type AS node_type, node.vid AS node_vid, node.title AS node_title FROM node node INNER JOIN term_node term_node ON node.vid = term_node.vid LEFT JOIN content_field_workshop_date node_data_field_workshop_date ON node.vid = node_data_field_workshop_date.vid WHERE (node.status <> 0) AND (node.type IN ('product','workshop')) AND (node.type in ('workshop')) AND (term_node.tid = 3) ORDER BY node_vid ASC LIMIT 0, 25
    
    igama’s picture

    I applied patch #93 and now Views works...

    *Update*
    Nevermind what I said about not working :)

    KarenS’s picture

    This issue is not about duplicates, it is about the query being rewritten and failing to run at all because it is invalid. You have a valid query that doesn't produce the results you intended. You duplicates are a natural outcome of some kinds of views that use taxonomy. That has nothing to do with this issue.

    brainski’s picture

    I tested the patch and it is working! I also have no duplicates.

    kentr’s picture

    Hoping to move the process along by encouraging better feedback...

    If you're submitting test results, please see these comments regarding what is and isn't helpful:

    http://drupal.org/node/284392#comment-1723396
    http://drupal.org/node/284392#comment-1723482
    http://drupal.org/node/284392#comment-1750512

    rootdownmedia’s picture

    Subscribe. #93 worked for me as well. Thanks tassoman!

    chawl’s picture

    Also fixes Drupal 6.13...

    How the hell is this not committed yet?

    InconceivableVizzini’s picture

    Version: 6.x-dev » 6.12

    Also experiencing this problem, though only when I move my view from the views ui into my module.

    You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DISTINCT(node.nid), node_galleries_gallery.gid AS node_galleries_gallery_gid ' at line 1 query: SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), node_galleries_gallery.gid AS node_galleries_gallery_gid FROM node node INNER JOIN users users ON node.uid = users.uid INNER JOIN node_galleries node_galleries_gallery ON node.nid = node_galleries_gallery.gid WHERE (users.uid = 1) AND (node.type in ('personal_gallery')) LIMIT 0, 10 in /Users/derek/Code/www/install_test/sites/all/modules/contrib/views/includes/view.inc on line 755.

      $view = new view;
      $view->name = 'gallery_views';
      $view->description = t('Gallery views - Public galleries, Personal gallery, etc');
      $view->tag = '';
      $view->view_php = '';
      $view->base_table = 'node';
      $view->is_cacheable = FALSE;
      $view->api_version = 2;
      $view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */
      $handler = $view->new_display('default', 'Defaults', 'default');
      $handler->override_option('fields', array(
        'gid' => array(
          'label' => '',
          'alter' => array(
            'alter_text' => 0,
            'text' => '',
            'make_link' => 0,
            'path' => '',
            'link_class' => '',
            'alt' => '',
            'prefix' => '',
            'suffix' => '',
            'help' => '',
            'trim' => 0,
            'max_length' => '',
            'word_boundary' => 1,
            'ellipsis' => 1,
            'strip_tags' => 0,
            'html' => 0,
          ),
          'link_to_node' => 1,
          'node_gallery_setting' => 'node_gallery_gallery',
          'exclude' => 0,
          'id' => 'gid',
          'table' => 'node_galleries_gallery',
          'field' => 'gid',
          'override' => array(
            'button' => 'Override',
          ),
          'relationship' => 'none',
        ),
      ));
      $handler->override_option('access', array(
        'type' => 'none',
      ));
      $handler->override_option('cache', array(
        'type' => 'none',
      ));
      $handler->override_option('distinct', 0);
      $handler->override_option('row_plugin', 'node');
      $handler = $view->new_display('page', 'My Galleries', 'page_1');
      $handler->override_option('fields', array(
        'gid' => array(
          'label' => '',
          'alter' => array(
            'alter_text' => 0,
            'text' => '',
            'make_link' => 0,
            'path' => '',
            'link_class' => '',
            'alt' => '',
            'prefix' => '',
            'suffix' => '',
            'help' => '',
            'trim' => 0,
            'max_length' => '',
            'word_boundary' => 1,
            'ellipsis' => 1,
            'strip_tags' => 0,
            'html' => 0,
          ),
          'link_to_node' => 1,
          'node_gallery_setting' => 'node_gallery_gallery',
          'exclude' => 0,
          'id' => 'gid',
          'table' => 'node_galleries_gallery',
          'field' => 'gid',
          'override' => array(
            'button' => 'Use default',
          ),
          'relationship' => 'none',
        ),
      ));
      $handler->override_option('filters', array(
        'uid_current' => array(
          'operator' => '=',
          'value' => '1',
          'group' => '0',
          'exposed' => FALSE,
          'expose' => array(
            'operator' => FALSE,
            'label' => '',
          ),
          'id' => 'uid_current',
          'table' => 'users',
          'field' => 'uid_current',
          'override' => array(
            'button' => 'Use default',
          ),
          'relationship' => 'none',
        ),
        'type' => array(
          'operator' => 'in',
          'value' => array(
            'node_gallery_gallery' => 'node_gallery_gallery',
          ),
          'group' => '0',
          'exposed' => FALSE,
          'expose' => array(
            'operator' => FALSE,
            'label' => '',
          ),
          'id' => 'type',
          'table' => 'node',
          'field' => 'type',
          'override' => array(
            'button' => 'Use default',
          ),
          'relationship' => 'none',
        ),
      ));
      $handler->override_option('title', 'My Galleries');
      $handler->override_option('use_pager', 'mini');
      $handler->override_option('distinct', 1);
      $handler->override_option('row_plugin', 'fields');
      $handler->override_option('path', 'gallery');
      $handler->override_option('menu', array(
        'type' => 'none',
        'title' => '',
        'description' => '',
        'weight' => 0,
        'name' => 'navigation',
      ));
      $handler->override_option('tab_options', array(
        'type' => 'none',
        'title' => '',
        'description' => '',
        'weight' => 0,
      ));
      $handler = $view->new_display('page', 'Public Galleries', 'page_2');
      $handler->override_option('fields', array(
        'gid' => array(
          'label' => '',
          'alter' => array(
            'alter_text' => 0,
            'text' => '',
            'make_link' => 0,
            'path' => '',
            'link_class' => '',
            'alt' => '',
            'prefix' => '',
            'suffix' => '',
            'help' => '',
            'trim' => 0,
            'max_length' => '',
            'word_boundary' => 1,
            'ellipsis' => 1,
            'strip_tags' => 0,
            'html' => 0,
          ),
          'link_to_node' => 1,
          'node_gallery_setting' => 'node_gallery_gallery',
          'exclude' => 0,
          'id' => 'gid',
          'table' => 'node_galleries_gallery',
          'field' => 'gid',
          'override' => array(
            'button' => 'Use default',
          ),
          'relationship' => 'none',
        ),
        'title' => array(
          'label' => '',
          'alter' => array(
            'alter_text' => 0,
            'text' => '',
            'make_link' => 0,
            'path' => '',
            'link_class' => '',
            'alt' => '',
            'prefix' => '',
            'suffix' => '',
            'help' => '',
            'trim' => 0,
            'max_length' => '',
            'word_boundary' => 1,
            'ellipsis' => 1,
            'strip_tags' => 0,
            'html' => 0,
          ),
          'link_to_node' => 0,
          'exclude' => 0,
          'id' => 'title',
          'table' => 'node',
          'field' => 'title',
          'override' => array(
            'button' => 'Use default',
          ),
          'relationship' => 'none',
        ),
      ));
      $handler->override_option('arguments', array(
        'name' => array(
          'default_action' => 'ignore',
          'style_plugin' => 'default_summary',
          'style_options' => array(),
          'wildcard' => 'all',
          'wildcard_substitution' => 'All',
          'title' => '%1\'s Galleries',
          'breadcrumb' => '',
          'default_argument_type' => 'fixed',
          'default_argument' => '',
          'validate_type' => 'none',
          'validate_fail' => 'not found',
          'glossary' => 0,
          'limit' => '0',
          'case' => 'none',
          'path_case' => 'none',
          'transform_dash' => 0,
          'id' => 'name',
          'table' => 'users',
          'field' => 'name',
          'validate_user_argument_type' => 'uid',
          'validate_user_roles' => array(
            '2' => 0,
          ),
          'override' => array(
            'button' => 'Use default',
          ),
          'relationship' => 'none',
          'default_options_div_prefix' => '',
          'default_argument_user' => 0,
          'default_argument_fixed' => '',
          'default_argument_php' => '',
          'validate_argument_node_type' => array(
            'webform' => 0,
            'poll' => 0,
            'advpoll_binary' => 0,
            'advpoll_ranking' => 0,
            'forum' => 0,
            'boatingrssfeeditem' => 0,
            'feed' => 0,
            'node_gallery_gallery' => 0,
            'node_gallery_image' => 0,
            'page' => 0,
            'profile' => 0,
            'personal_gallery' => 0,
            'personal_gallery_image' => 0,
            'rotato_gallery' => 0,
            'rotato_gallery_image' => 0,
            'story' => 0,
          ),
          'validate_argument_node_access' => 0,
          'validate_argument_nid_type' => 'nid',
          'validate_argument_vocabulary' => array(
            '1' => 0,
            '2' => 0,
          ),
          'validate_argument_type' => 'tid',
          'validate_argument_transform' => 0,
          'validate_user_restrict_roles' => 0,
          'validate_argument_php' => '',
        ),
      ));
      $handler->override_option('filters', array(
        'type' => array(
          'operator' => 'in',
          'value' => array(
            'node_gallery_gallery' => 'node_gallery_gallery',
          ),
          'group' => '0',
          'exposed' => FALSE,
          'expose' => array(
            'operator' => FALSE,
            'label' => '',
          ),
          'id' => 'type',
          'table' => 'node',
          'field' => 'type',
          'override' => array(
            'button' => 'Use default',
          ),
          'relationship' => 'none',
        ),
      ));
      $handler->override_option('title', 'Public Galleries');
      $handler->override_option('distinct', 1);
      $handler->override_option('row_plugin', 'fields');
      $handler->override_option('path', 'gallery/%');
      $handler->override_option('menu', array(
        'type' => 'none',
        'title' => '',
        'description' => '',
        'weight' => 0,
        'name' => 'navigation',
      ));
      $handler->override_option('tab_options', array(
        'type' => 'none',
        'title' => '',
        'description' => '',
        'weight' => 0,
      ));
      $handler = $view->new_display('page', 'Personal Galleries', 'page_3');
      $handler->override_option('filters', array(
        'uid_current' => array(
          'operator' => '=',
          'value' => '1',
          'group' => '0',
          'exposed' => FALSE,
          'expose' => array(
            'operator' => FALSE,
            'label' => '',
          ),
          'id' => 'uid_current',
          'table' => 'users',
          'field' => 'uid_current',
          'override' => array(
            'button' => 'Use default',
          ),
          'relationship' => 'none',
        ),
        'type' => array(
          'operator' => 'in',
          'value' => array(
            'personal_gallery' => 'personal_gallery',
          ),
          'group' => '0',
          'exposed' => FALSE,
          'expose' => array(
            'operator' => FALSE,
            'label' => '',
          ),
          'id' => 'type',
          'table' => 'node',
          'field' => 'type',
          'override' => array(
            'button' => 'Use default',
          ),
          'relationship' => 'none',
        ),
      ));
      $handler->override_option('title', 'Personal Galleries');
      $handler->override_option('distinct', 1);
      $handler->override_option('row_plugin', 'fields');
      $handler->override_option('path', 'user/galleries');
      $handler->override_option('menu', array(
        'type' => 'none',
        'title' => '',
        'description' => '',
        'weight' => 0,
        'name' => 'navigation',
      ));
      $handler->override_option('tab_options', array(
        'type' => 'none',
        'title' => '',
        'description' => '',
        'weight' => 0,
      ));
    
    Ryanbach’s picture

    Can we try to run this patch test (#93) for Drupal 6 HEAD?

    agentrickard’s picture

    FileSize
    1.96 KB
    2.06 KB

    I cannot replicate the error in #120. That needs to be researched as a separate issue.

    Tested against 6.13, works as expected. There were some minor code-style issues with the patch handling of string concatenation (corrected).

    Attached is my test View and the revised patch. Below are the queries. Testing Domain Access 6.x.2.0rc8, a node access module.

    Without patch, no distinct:
    
    SELECT DISTINCT(node.nid) AS nid, node.title AS node_title, node_revisions.teaser AS node_revisions_teaser, node_revisions.format AS node_revisions_format FROM node node 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 = 0 AND na.realm = 'domain_site') OR (na.gid = 3 AND na.realm = 'domain_id'))) LIMIT 0, 10
    
    Without patch, with distinct
    SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), node.title AS node_title, node_revisions.teaser AS node_revisions_teaser, node_revisions.format AS node_revisions_format FROM node node 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 = 0 AND na.realm = 'domain_site') OR (na.gid = 3 AND na.realm = 'domain_id'))) LIMIT 0, 10
    
    Error: You have an error in your SQL syntax
    
    With patch, no distinct
    SELECT node.nid AS nid, node.title AS node_title, node_revisions.teaser AS node_revisions_teaser, node_revisions.format AS node_revisions_format FROM node node 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 = 0 AND na.realm = 'domain_site') OR (na.gid = 3 AND na.realm = 'domain_id'))) LIMIT 0, 10
    
    With patch, distinct
    SELECT DISTINCT(node.nid) AS nid, node.title AS node_title, node_revisions.teaser AS node_revisions_teaser, node_revisions.format AS node_revisions_format FROM node node 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 = 0 AND na.realm = 'domain_site') OR (na.gid = 3 AND na.realm = 'domain_id'))) LIMIT 0, 10
    
    No error.
    
    

    Patch applies, tests clean. I wonder if the code can be optimized a little.

    Does this bug not affect pgSQL? Is there a reason not to re-use code from database.pgsql,inc?

    That aside, looks good.

    agentrickard’s picture

    Version: 6.12 » 6.x-dev
    Alex Andrascu’s picture

    Subscribing. Patch #93 indeed works. Tested on 6.12 with Content Access. Lovely ! Thanks!

    joshmiller’s picture

    Status: Needs review » Reviewed & tested by the community

    I applied the patch and it works with forum_access and lots of different Views2 distinct queries (which currently breaks horribly on Drupal 6.13).

    Can we get Dries or Gabor to put this in the que for Drupal 6.14 ?

    Still needs a changelog.txt addition, otherwise RTBC.

    Josh

    DamienMcKenna’s picture

    Same story as others - without the patch we get *lots* of errors like this on Views set to distinct=Yes:

    user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DISTINCT(node.nid), node_node_data_field_thumbnail_node_data_field_image.fiel' at line 1 query: SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), node_node_data_field_thumbnail_node_data_field_image.field_image_fid AS node_node_data_field_thumbnail_node_data_field_image_field_image_fid, node_node_data_field_thumbnail_node_data_field_image.field_image_list AS node_node_data_field_thumbnail_node_data_field_image_field_image_list, node_node_data_field_thumbnail_node_data_field_image.field_image_data AS node_node_data_field_thumbnail_node_data_field_image_field_image_data, node_node_data_field_thumbnail.nid AS node_node_data_field_thumbnail_nid, node_node_data_field_thumbnail.type AS node_node_data_field_thumbnail_type, node_node_data_field_thumbnail.vid AS node_node_data_field_thumbnail_vid, node.title AS node_title, nodequeue_nodes_node.position AS nodequeue_nodes_node_position, node_counter.totalcount AS node_counter_totalcount FROM node node LEFT JOIN nodequeue_nodes nodequeue_nodes_node ON node.nid = nodequeue_nodes_node.nid AND nodequeue_nodes_node.qid = 6 LEFT JOIN content_field_thumbnail node_data_field_thumbnail ON node.vid = node_data_field_thumbnail.vid INNER JOIN node node_node_data_field_thumbnail ON node_data_field_thumbnail.field_thumbnail_nid = node_node_data_field_thumbnail.nid LEFT JOIN content_type_image node_node_data_field_thumbnail_node_data_field_image ON node_node_data_field_thumbnail.vid = node_node_data_field_thumbnail_node_data_field_image.vid LEFT JOIN node_counter node_counter ON node.nid = node_counter.nid WHERE (node.status <> 0 OR (node.uid = 1 AND 1 <> 0) OR 1 = 1) AND (node.type in ('gallery')) ORDER BY nodequeue_nodes_node_position DESC, node_counter_totalcount DESC LIMIT 0, 10 in /mysite/sites/all/modules/platform/views/includes/view.inc on line 755.
    

    Running the patch in #93 removed such errors.

    agentrickard’s picture

    The patch in #93 is outdated and has code style issues.

    The current patch to review is #122.

    meba’s picture

    Tested #122 and works. Ready to go?

    joshmiller’s picture

    agentrickard / meba,

    I believe this patch is RTBC, thus, it does not require any more reviews (though, more are appreciated)... it just needs to get into the Drupal 6.14 branch...

    Josh

    naught101’s picture

    Subscribe. Perhaps applying the patch to 6.x would bring any problems with it out of the woodwork >:)

    Gábor Hojtsy’s picture

    Status: Reviewed & tested by the community » Fixed

    Committed to Drupal 6, thanks.

    agentrickard’s picture

    Version: 6.x-dev » 5.x-dev
    Status: Fixed » Patch (to be ported)

    I think we need to (can we?) backport this to Drupal 5.

    cpelham’s picture

    Version: 5.x-dev » 6.13
    Status: Patch (to be ported) » Active

    Works as expected!

    webchick’s picture

    Version: 6.13 » 5.x-dev
    Status: Active » Patch (to be ported)

    Restoring status.

    clau_bolson’s picture

    I have a variant of this issue that the patch doesn't fix. Using node_privacy_byrole or taxonomy_access, I get a mysql error. This is the query for taxonomy_access:

    SELECT n.title, DISTINCT(n.nid) FROM node n INNER JOIN term_node tn ON n.nid = tn.nid INNER JOIN node_access na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 1 AND na.realm = 'term_access'))) AND ( tn.tid in (1) AND n.status=1 )ORDER BY n.created DESC LIMIT 0, 1

    and it fails because n.title is before DISTINCT(n.nid)

    How can I fix it?

    agentrickard’s picture

    Open a new issue for one of those modules, please.

    benoitg’s picture

    subscribing

    extect’s picture

    Actually, we still seem to have a problem here!
    I applied #122 to Drupal 6.13, but using Content Access module the entire "DISTINCT" command gets stripped off.

    So, the query begins with:
    SELECT node.nid AS nid

    instead of:
    SELECT DISTINCT(node.nid) AS nid

    I guess this is caused by D6 core and is not a Content Access issue?

    Anyone else experiencing this problem?

    Alice Heaton’s picture

    @extect : This patch has now been commited to Drupal 6.13 - so reporting issues here is not helpful and will be ignored. Please upgrade to Drupal 6.13 and if you still see a problem, please report it separately.

    Leeteq’s picture

    I think it has been committed to the 6.x-dev version which will be the upcoming 6.14, not the existing 6.13?

    Alice Heaton’s picture

    I think it has been committed to the 6.x-dev version which will be the upcoming 6.14, not the existing 6.13?

    Yes you're correct - this is not included in 6.13, but has been commit to 6.x-dev. However my comment stands : this is now part of Drupal, not a patch anymore, so issues with it should be filed as new bugs against 6.x-dev ; you're unlikely to get a developer response here.

    opteronmx’s picture

    Just confirming patch@#122 works on D5 5.19 (database.mysql.inc,v 1.66.2.3).

    Patch manually applied

    Thanks folks!!!

    agentrickard’s picture

    @opteronmx

    Can you roll a D5 patch based on what you hand-rolled, please?

    grub3’s picture

    I would like to inform you of grave performance issues under Drupal 6.13:

    SQL performance issue : forum_get_topics executes in 720 ms
    http://drupal.org/node/558894

    SQL performance issue: SQL forum bloc executes in 450ms
    http://drupal.org/node/558886

    This seems to me completely *** LOL *** that db_rewrite_sql rewrites a query adding arbitrary DISTINCT on the {nodes} table.

    On large installations like mine (500.000 page), the parser will start build a huge distinct index on the {node} table.
    Then the parser processes the index.

    This eats-up a tremendous part of CPU power and index memory, not to say time.
    I wonder why your guys ever thought of such a solution.

    It is the developer job to build an ANSI query starting
    usually LEFT of the query (where unique records reside)
    and then going right using INNER JOINS or LEFT JOINs.

    Was this issue fixed in PostgreSQL-DEV?
    Was it also fixed in D7?

    Please inform me ASAP.
    I am migrating a 500000 PHPBB post and my quad core will probably explode when it gets online with 2000 users.

    Can you remove this creazy DISTINCT automation or not?

    This is always what happens when using MySQL.
    Franckly, you should always use PostgreSQL and read detailed logs to understand how the parser works.

    In most cases unless you have more than 1000 users online, there is no need for caching if SQL queries are executed normally.

    grub3’s picture

    Distinct should be set by hand by each developer on request, when need.

    We should either

    • remove this distinct automation and let developers decide by themselves what data should be distinct. DISTINCT has a huge cost in memory and CPU time
    • OR print of Drupal front page that it does not support more that 200 users simulnaneously. Then print "buy a web farm, add connection pooling, add caching, add a MySQL cluster, and spend 100.000 dollars".
    • Pardon me if I am a little bit tough, but I don't understand why this DISTINCT issue was added to Drupal.

    grub3’s picture

    I read part the abstraction layer.

    It seems that when a field is distinct in database, your add "DISTINCT" to the SQL query.
    This is absurd and contrary to SQL standards.

    On the converse, setting DISTINCT in a database helps the database produce distinct values.
    When starting left on the distinct table you will always obtain distinct records.

    Example:

    SELECT a from foo
    INNER JOIN foo.b = bar.b
    WHERE c="quick"

    OR

    SELECT a from foo
    LEFT JOIN foo.b = bar.b
    WHERE c="quick"

    will always produce distinct values if a is distinct.

    Distinct has huge cost, it can drive a database down.
    In fact, this may be why Drupal is SO slow.
    (of course, Drupal send 20 times the same query in a page, I will report back later).

    Removing distinct rewrite will double the speed immediately.
    Distinct is ANSI and SQL92 compliant, it has to be set by hand when needed.

    Looking at Drupal code, it is very rarely needed.

    Can we discuss?

    Damien, could you explain core developers that DISTINCT is the absolute evil to databases.

    Damien Tournoud’s picture

    It seems that when a field is distinct in database, your add "DISTINCT" to the SQL query.

    I don't know where you have found this, but it is clearly not true.

    When starting left on the distinct table you will always obtain distinct records.

    This is clearly not true either.

    Damien Tournoud’s picture

    @jmpoure: please test the patch in #122 and report if the rewritten queries are better.

    grub3’s picture

    Thanks, I will report back.

    grub3’s picture

    I think I understand, the patch applies to mysql and mysqli, no pg. Porting it. Will report.

    grub3’s picture

    I got a quick look into the present devel code (not patch):

    function db_distinct_field($table, $field, $query) {
      if (!preg_match('/FROM\s+\S+\s+AS/si', $query)
      && !preg_match('/DISTINCT\s+ON\s*\(\s*(' . $table . '\s*\.\s*)?' . $field . '\s*\)/si', $query)
      && !preg_match('/DISTINCT[ (]' . $field . '/si', $query)
      && preg_match('/(.*FROM\s+)(.*?\s)(\s*(WHERE|GROUP|HAVING|ORDER|LIMIT|FOR).*)/Asi', $query, $m)) {
        $query = $m[1];
        $query .= preg_replace('/([\{\w+\}]+)\s+(' . $table . ')\s/Usi', '(SELECT DISTINCT ON (' . $field . ') * FROM \1) \2 ', $m[2]);
        $query .= $m[3];
      }
      return $query;
    }
    

    The logic of the code is to add DISTINCT whenever needed and not in the query, right?
    So there is a possible risk that a normal query gets included in the script, by error, right?

    I think this way to go is a dead-end, because you cannot ask a single line of PHP code to optimize an SQL query.
    Database optimizers have 100.000 lines of code. If you think you can solve this issue and rewrite DISTINCT go ahead.

    Furthermore (SELECT DISTINCT ON (' . $field . ') * FROM foo )
    is a subquery.

    It can potentially run a select distinct clause on the node table, thus making a sequential scan.
    On my 650.000 rows server, this costs at least 650.000 CPU cycles.

    You can read my guide http://drupal.org/node/559302 and comment.

    Feel free to propose a patch for PostgreSQL driver.
    Personally, I will not take the risk to drive down Drupal writing pregs.
    I am too young in the community to take the risk to work on such a patch.
    Sorry.

    But on the converse, I can do intensive testing after someone submit a patch for PostgreSQL.
    IMHO, we should remove this query rewriting stuff and do intensive testing of Drupal to add DISTINCT manually where needed.

    drumm’s picture

    This issue is a bit of a long-winded mess. Is there a D5 patch? Who has reviewed it? Please bump out of 'to be ported' status if there is something reviewable for D5.

    agentrickard’s picture

    There is no d5 patch. in #142, @opteronmx reports that he patched D5 manually, but he never contributed anything back.

    Feel free to open a new issue, if this is too cloudy.

    tyr’s picture

    Version: 5.x-dev » 6.14
    Status: Patch (to be ported) » Needs work

    Confirming #138. The same is happening here with the patch which has been committed lately to drupal core 6.14.

    Apparently the DISTINCT()-modifier is not added by db_distinct_field when the field has an alias.
    I solved the problem by changing the regexp-replace slightly. Instead of
    '/((?:^|,)\s*)(?<!DISTINCT\()(?:'. $table .'\.)?'. $field .'(\s*(?:,|$))/is'
    in database.mysqli.inc, line 370 I'm using
    '/((?:^|,)\s*)(?<!DISTINCT\()(?:'. $table .'\.)?'. $field .'(.*?(?:,|$))/is'

    I don't know about any side-effects this may have on the original issue.

    agentrickard’s picture

    Is this a core problem or a Views problem? My understanding is that Views fixed this problem in its own way.

    #579018: Duplicates in views for anonymous users (duplicate)
    #501552: Provide workaround for core DISTINCT bug in db_rewrite_sql

    kentr’s picture

    IIRC, it reared its ugly head when people discovered that checking the "Distinct" option in Views produced these queries, but the rewriting issue that causes it is in core and would affect other queries depending on that rewriting.

    Junro’s picture

    subscribe

    izmeez’s picture

    subscribing.

    I wasn't sure where to place my comments and put it in the Views queue http://drupal.org/node/579892#comment-2064586 but I don't think it is a views problem per se.

    I have found that any user with permission to administer nodes does not see this problem.

    Izzy

    ajayg’s picture

    Just want to confirm I started seeing this problem after upgrading to 6.14. It shows only for OG node (another node access module) and non-admin users.
    As a workaround I have selected distinct option for the view (tracket) but there is a performance penalty. But I didn't have to select that in previous versions so this is a bummer. So from a end user perspective (who has very limited knowledge) this introduces new issue rather than fixing issue.

    agentrickard’s picture

    And it appears on normal list pages, like /node?

    flaviovs’s picture

    I'm also experiencing this issue.

    The view is very simple: show published nodes with content type = X. No sort, no grouping, no nothing.

    Admins and view preview show one entry per node, as expected.

    Anonymous users see duplicated entries in the form NODE1, NODE1, NODE2, NODE2 etc.

    If I set the "distinct" option to "yes" on the view, then the problem goes away.

    I've been tracking this issue here in Drupal.org and one thing I noticed is that for almost all reports, the person is using some form of node access module (which is my case, BTW). Don't know if this is related or just plain coincidence.

    Everyone see a normal (non-duplicated) node listing in "/node".

    izmeez’s picture

    Sorry for the duplicates but this issue has been discussed in other threads, http://drupal.org/node/579892#comment-2067648

    The patch in #33 of that thread solved the problem for me. Looks like Drupal core 6.14 may be ok.

    Izzy

    ajayg’s picture

    @agentrickard
    I am seeing this in tracker view which shows list of nodes belonging to OG. It does not show on list of nodes (in the same tracker) outside OG. So it is definitely related to node access. Obviously if you are seeing a single node in fullpageview it does not show.

    @izmeez
    the patch you have said is not a solution, it is a workaround. What that is doing is forcing every view to use distinct. You can achieve the same effect by checking the "distinct" option in view without applying the patch. That patch(or worakround) has performance implications.

    izmeez’s picture

    @ajayg, thanks for the explanation. Would the "distinct" option have to be set in each view or is there a global setting?

    This also explains why the problem and workaround works for nodequeues.

    ken54671’s picture

    Here is a patch for #154 suggested earlier.

    I tested this patch with Drupal 6.14 and vanilla Views 6.x-2.6.

    For me, it appears to resolve issues I was having with duplicate entries for node_access queries:

    Authors and only authors see dupplicate items after upgrading Drupal to 6.14

    and obviates the need to patch views with the various workarounds:

    Provide workaround for core DISTINCT bug in db_rewrite_sql

    I think it is at the very least on the right track. As merlinofchaos stated,

    clearly this is widespread and probably a bug in that core patch.

    Can others test this patch? If you are using a dev version of Views with the workaround patch, you may need to reverse that patch first.

    Patch for Drupal 6.14 (with vanilla Views 6.x-2.6).

    ajayg’s picture

    distinct option has to be set for each view. In some cases it is good things because you do not want each and every view to suffer from potential performance load. In some cases it may be hassle to set each view with distinct. In that case the patch is better option as it sets distinct for every view.

    flaviovs’s picture

    @ajayg, NO, DISTINCT should be set only when needed. Most small views using only a single table doesn't need distinct at all.

    For example, in my post #162, the view translate to this simple SQL query:

    select * from node where type = 'mytype'
    

    There's no need to use distinct with such type of queries.

    DISTINCT is a necessary evil when you cannot arrange complicated queries in a way to return distinct values (though if you're willing to work a little bit on manually rearranging your query, you may do it without distinct), because it is a performance and resource hog.

    ajayg’s picture

    Status: Needs work » Needs review

    Since we have a potential patch (#166) changing status to need review.

    p6’s picture

    Subscribe

    I'm having to do a dirty hook_views_pre_execute() to fix the query :-(

    deviantintegral’s picture

    Status: Needs review » Needs work

    Subscribing.

    tyr’s picture

    Status: Needs work » Needs review

    @171: why changing back to "needs work"?

    The patch solves the problem for me and there is no need for any kind of workaroud.

    Damien Tournoud’s picture

    Status: Needs review » Needs work

    It's time to write some tests for this. Could someone provide example of queries in which the DISTINCT fails to be added?

    agentrickard’s picture

    @Damien. See #122 for a View from before the patch. I need to write some new tests later today.

    agentrickard’s picture

    Status: Needs work » Needs review
    FileSize
    3.12 KB
    732 bytes

    OK, I did some deep digging into this, and the issue, as I suspected, is that the two patches conflict. Using 6.13 with the Views patch is fine. Using 6.14 without the Views patch throws duplicates IF multiple node access permissions are present.

    The solution seems to be to revert the patch to Views. Apply the patch in #166 to Drupal core, and use DISTINCT in Views where needed.

    A sample View and some replication instructions follow.

    I tested using Organic Groups and Domain Access (which I maintain).

    heacu’s picture

    subscribing

    Steven Jones’s picture

    subscribing.

    marcushenningsen’s picture

    For what it's worth, #166 works for me (using Drupal 6.14 and Views 2.6).

    Thanks =)

    brisath’s picture

    subscribing

    rfay’s picture

    subscribing

    realityloop’s picture

    #166 worked for me as well with Drupal 6.14 & Views 2.6

    kate’s picture

    subscribing

    agentrickard’s picture

    Status: Needs review » Reviewed & tested by the community

    Setting to RTBC, since no one else seems willing.

    Damien Tournoud’s picture

    Status: Reviewed & tested by the community » Needs work

    I suggest we add simpletests to that. Enough is enough.

    agentrickard’s picture

    Core simpletests that check against a contrib module (which is the only place this error occurs)?

    How and where do you suggest that we do that?

    tyr’s picture

    I don't have any experiences with simpletests, but I think one should test the function "db_distinct_field" against some example queries, not against other modules. Or am I mistaken?

    The function description says:

    Wraps the given table.field entry with a DISTINCT(). The wrapper is added to the SELECT list entry of the given query and the resulting query is returned. This function only applies the wrapper if a DISTINCT doesn't already exist in the query.

    For example db_distinct_field("node", "nid", "SELECT node.nid FROM node") should return "SELECT DISTINCT(node.nid) FROM node". And so should also more complex queries. For example, the following does NOT work without the patch (#166): db_distinct_field("node", "nid", "SELECT node.nid AS node_nid FROM node") should return "SELECT DISTINCT(node.nid) AS node_nid FROM node", BUT returns "SELECT node.nid AS node_nid FROM node".

    so, the tests may be:

    1. db_distinct_field("table", "field", "SELECT table.field FROM table") ?= "SELECT DISTINCT(table.field) FROM table"
    2. db_distinct_field("table", "field", "SELECT table.field AS table_field FROM table") ?= "SELECT DISTINCT(table.field) AS table_field FROM table"
    3. db_distinct_field("table", "field", "SELECT DISTINCT(table.field) FROM table") ?= "SELECT DISTINCT(table.field) FROM table"
    4. db_distinct_field("table", "field", "SELECT DINSTINCT(table.field) AS table_field FROM table") ?= "SELECT DISTINCT(table.field) AS table_field FROM table"
    5. db_distinct_field("table", "field", "SELECT table.field,table.field2 FROM table") ?= "SELECT DISTINCT(table.field),table.field2 FROM table"
    6. db_distinct_field("table", "field", "SELECT table.field AS table_field, table.field2 AS table_field2 FROM table") ?= "SELECT DISTINCT(table.field) AS table_field, table.field2 AS table_field2 FROM table"
    7. db_distinct_field("table", "field", "SELECT DISTINCT(table.field),table.field2 FROM table") ?= "SELECT DISTINCT(table.field),table.field2 FROM table"
    8. db_distinct_field("table", "field", "SELECT DINSTINCT(table.field) AS table_field, table.field2 AS table_field2 FROM table") ?= "SELECT DISTINCT(table.field) AS table_field, table.field2 AS table_field2 FROM table"

    (Number 2,4,6 and 8 will fail with the current code)

    Anyway, this whole database-API is bogus and also - luckily - past with drupal 7. For example this very function "db_distinct_field" is somehow nonsense, because it can create queries which are not mysql-conform (like having multiple distinct fields or something like this: "SELECT field1, DISTINCT(field2) FROM ..."). Moreover, mysql only has the distinct-row feature (see http://dev.mysql.com/doc/refman/5.0/en/select.html), thus the function should be named "db_distinct_query" and contain just one parameter ($query) and one line:

    return preg_replace('/^(SELECT)/is', 'SELECT DISTINCT',$query);
    

    In this case the resulting queries will look a little bit different: "SELECT DISTINCT field ..." vs. "SELECT DISTINCT(field) ...". For mysql both are equal (with the first beeing the "right" and documented syntax).

    Steven Jones’s picture

    FileSize
    2.85 KB

    Here's those assertions as a simpletest .test file.

    I think 4 and 8 were failing because of the 'DINSTINCT' rather than an error in the db_distinct_field function.

    On drupal 6.14, queries 2 and 6 fail.

    Steven Jones’s picture

    Crucially with the patch from #166, queries 7 and 8 fail.

    Steven Jones’s picture

    Status: Needs work » Needs review
    FileSize
    1.11 KB

    Don't we actually want to use the following method, and actually look for the 'AS' explicitly?

    This passes all the tests in #187.

    bryrock’s picture

    subscribe

    David Strauss’s picture

    I've committed Steven's work from #189 and #187 to Pressflow 6.

    David Strauss’s picture

    Status: Needs review » Reviewed & tested by the community

    Considering I've committed this, I may as well mark it RTBC for Drupal.

    Damien Tournoud’s picture

    Status: Reviewed & tested by the community » Needs work
    '(\s*?(?:,|(?:AS)|$))/is',
    

    The first '?' is probably not needed (* matches 0 or more), the non-capture pattern is probably not needed either. On the other hand, we need to make sure there is at least a space between the field and the 'AS' or <fieldname>AS would match.

    I suggest:

    '((?:\s*,|\s+AS|\s*$))/is',
    
    Steven Jones’s picture

    Which we can further reduce to:

    '(\s*,|\s+AS|\s*$)/is'

    Steven Jones’s picture

    Status: Needs work » Needs review
    FileSize
    1.1 KB

    Hence:

    crea’s picture

    Subscribing

    Steven Jones’s picture

    Can we get a review of #196 from one or two of the parties interested enough to 'subscribe'.

    crea’s picture

    Steven, I guess I can't really "review" it, cause I'm not that experienced but here is my report. My dev site is Drupal 6.14, Views 6.x-2.6, node access modules. I applied this patch and it fixed dupes problem with Views. Now I don't need to turn on "distinct" checkbox in Views setting to remove dupes. Before this I had dupes for node author even with simple views, e.g. "latest published nodes of type X".

    Kars-T’s picture

    Status: Needs review » Reviewed & tested by the community

    I tested the issue with some debug prints.

      if (preg_match('/^SELECT(.*?)FROM(.*)/is', $query, $matches)) {
        $select = preg_replace(
          '/((?:^|,)\s*)(?<!DISTINCT\()(?:'. $table .'\.)?'. $field .'(\s*,|\s+AS|\s*$)/is',
          '\1'. $field_to_select .'\2', $matches[1], 1
        );
        watchdog('PREG_REPL ORG', $query);
        watchdog('PREG_REPL REPLACED', 'SELECT'. $select .'FROM'.$matches[2]);
    
        return 'SELECT'. $select .'FROM'.$matches[2];
      }
    

    Before the patch:

    SELECT node.nid AS nid, node.title AS node_title, node_data_field_boole_bezahlt.field_boole_bezahlt_value AS node_data_field_boole_bezahlt_field_boole_bezahlt_value, node.type AS node_type, node.vid AS node_vid, node.created AS node_created FROM {node} node INNER JOIN {content_field_boole_bezahlt} node_data_field_boole_bezahlt ON node.vid = node_data_field_boole_bezahlt.vid INNER JOIN {users} users ON node.uid = users.uid WHERE (node.type in ('%s')) AND (node_data_field_boole_bezahlt.field_boole_bezahlt_value IN ('%s', '%s')) AND (users.uid = ***CURRENT_USER***) ORDER BY node_created DESC 
    

    is not rewritten eg. PREG_REPL ORG === PREG_REPL REPLACED .

    After the patch PREG_REPL REPLACED is:

    SELECT DISTINCT(node.nid) AS nid, node.title AS node_title, node_data_field_boole_bezahlt.field_boole_bezahlt_value AS node_data_field_boole_bezahlt_field_boole_bezahlt_value, node.type AS node_type, node.vid AS node_vid, node.created AS node_created FROM {node} node INNER JOIN {content_field_boole_bezahlt} node_data_field_boole_bezahlt ON node.vid = node_data_field_boole_bezahlt.vid INNER JOIN {users} users ON node.uid = users.uid WHERE (node.type in ('%s')) AND (node_data_field_boole_bezahlt.field_boole_bezahlt_value IN ('%s', '%s')) AND (users.uid = ***CURRENT_USER***) ORDER BY node_created DESC 
    

    The regular expression now accepts an AS as string end.

    $matches[1] is

    node.nid AS nid, node.title AS node_title, node_data_field_boole_bezahlt.field_boole_bezahlt_value AS node_data_field_boole_bezahlt_field_boole_bezahlt_value, node.type AS node_type, node.vid AS node_vid, node.created AS node_created 
    

    The regexps in words;

    (\s*(?:,|$))
    Match any spaces till you find the next "," or string end.

    (\s*,|\s+AS|\s*$)
    Match any spaces till "," or one space AS or any spaces till strings end.

    This now does match the first field in the query up to the next , or "AS" or line end and this is what I beleive we want to achieve with this patch. So my $match[1] "node.nid" is matched and replaced.

    I hope this review helps. I did test this and the rewrite seems correct to me as well as the regexp. I set this to tested and reviewed.

    toniw’s picture

    Don't know if this is the right thread for this, but this patch does not fix taxonomy_term_count_nodes() returning the wrong number.
    In D6.13 it did something like this:

    SELECT t.tid, COUNT( DISTINCT(n.nid)) AS c FROM term_node t ...

    In D6.14 with or without the #195 patch it does:

    SELECT t.tid, COUNT(n.nid) AS c FROM term_node t ...

    So if one thing is sure, this patch does not cure all the problems.

    Steven Jones’s picture

    Status: Reviewed & tested by the community » Needs work
    merlinofchaos’s picture

    Views eventually introduced a workaround for the original behavior. Which means we can probably just rollback the patch to the original behavior which is less broken than the current behavior.

    toniw’s picture

    I also tried the Views-patch. It has the same behavior. It fixes the display of duplicate nodes, but that one also does not get the taxonomy_term_count_nodes() right. Most noticable result is that (in my case) in image albums all image counts are a factor 4 too high. Manually setting 'distinct' in all related views does not solve that issue.

    tyr’s picture

    Oh, we forgot that DISTINCT can also be used in aggregate functions like COUNT. I'd suggest the following regex (basically just the one from #195, but matches also brackets):

    '/((?:^|,|\()\s*)(?<!DISTINCT\()(?:'. $table .'\.)?'. $field .'(\s*(?:,|\sAS|\)|$))/is'

    tyr’s picture

    Status: Needs work » Needs review

    additionally, there are some more queries to be included in the simpletests:
    maybe:

    • db_distinct_field("table", "field", "SELECT COUNT(table.field) FROM table") ?= "SELECT COUNT(DISTINCT(table.field)) FROM table"
    • db_distinct_field("table", "field", "SELECT COUNT(table.field) AS table_field FROM table") ?= "SELECT COUNT(DISTINCT(table.field)) AS table_field FROM table"
    • db_distinct_field("table", "field", "SELECT table.field1, COUNT(table.field), table.field2 FROM table") ?= "SELECT table.field1, COUNT(DISTINCT(table.field)), table.field2 FROM table"
    • db_distinct_field("table", "field", "SELECT COUNT(DISTINCT(table.field)) FROM table") ?= "SELECT COUNT(DISTINCT(table.field)) FROM table"
    Steven Jones’s picture

    FileSize
    3.69 KB

    Tests attached, and passing.

    toniw’s picture

    Thank you! Patch in #204 solved all issues (that I am currently aware of) in my site!

    sethcohn’s picture

    +1 on patch in #204, fixed the "missing distinct in views w/ node access enabled" issue for me.

    Kars-T’s picture

    Status: Needs review » Reviewed & tested by the community

    I applied the patch and made sure the double views items where gone. Than I did let the test from #206 run and it worked fine.

    The new regexp adds a check for ( and )

    The only problem that is left is that I can think of a good place in D6 to put this test as there are no db tests afaik. But for the patch itself I believe its fine and it now seems to cover all SQL that I could think of.

    @steve jones
    Thank you for the great test! This makes things much easier!

    ajayg’s picture

    +1 for patch in #204.

    I confirm that patch works. I tried this on one of the production machine and it is working fine.

    kentr’s picture

    @tyr:

    Is #204 against the 6.14 or 6.x-dev version? The patch isn't working for me b/c there are unmatched patch segments.

    Thanks.

    crea’s picture

    I confirm that with patch from #195 I was still getting dupes for number of nodes tagged with a term, and with patch from #204 I get no dupes at all.

    @kentr: make sure you reverse any previous patches first.

    dleong’s picture

    subscribing

    Gábor Hojtsy’s picture

    Keep the tests coming please! It would be best to not introduce another regression :)

    Steven Jones’s picture

    Is that a request for more tests? If so what other queries would you like testing? Subqueries might be worth a look, I suppose.

    Where should be put the tests? In D6 core, or in simpletest?

    tyr’s picture

    @kentr: The patch is against a fresh/unpatched 6.14

    brianV’s picture

    Issue tags: +Needs tests

    Just tagging as 'Needs Tests' since Gabor seems to be suggesting it needs more.

    brianV’s picture

    Version: 6.14 » 6.x-dev

    also updating to 6,x-dev, since this issue is in current dev.

    Gábor Hojtsy’s picture

    Tagging.

    Steven Jones’s picture

    What extra tests are needed?

    ngstigator’s picture

    Subscribe

    Parkes Design’s picture

    Subscribe

    ptoly’s picture

    I ran this patch on my 6.14 site.
    Indeed the duplicates do disappear. However I get this message when trying to create an organic groups (v 6.x-2.x-dev (2009-Oct-14)) node:

    user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'DISTINCT(n.nid) FROM node n WHERE n.type IN ('topic','group') AND n.status = 1 O' at line 1 query: SELECT n.title, DISTINCT(n.nid) FROM node n WHERE n.type IN ('topic','group') AND n.status = 1 ORDER BY n.title ASC in /home3/scgreeno/public_html/scgreen.org/sites/all/modules/custom/og/og.module on line 1612.

    Perhaps relevant is the fact that I never got the duplicate problem on my own dev server, running PHP 5.2.11, MySQL 5.4.1, but do consistently have the duplicates problem on my production system which is running PHP 5.2.9 and MySQL 5.0.81.

    It is a show stopper because I can't create OG records.

    Ah, spoke too soon.
    Reverted to OG 6.x-2.x and it now works!

    Postnote:
    Thanks to Steven and jcmarco in #224 and #225 below for input.
    Line 1629, the db_rewrite_sql, which is in the dev version and not the release version seems to be the culprit.

    Steven Jones’s picture

    @ptoly Seems like thats an og issue, it needs to reorder its columns in that query, raise a bug in og's issue queue.

    @Gábor What other tests would you like to see?

    jcmarco’s picture

    @ptoly That problem was reported here and patch seems to work fine, but it is still pending to be applied to OG
    #569110: user warning: You have an error in your SQL syntax

    JuliaKM’s picture

    I was running into a similar problem using Forum and the Node privacy by role modules and Drupal 6.14. The topic count and posts counts were doubled in my forum for users with one particular role assigned. I am using an access module, Node privacy by role.

    After applying the patch at #204, the topic counts were correct.

    However, now, instead of both the number of posts and the number of topics being doubled, just the $forum->num_posts value is doubled. After some investigating, I realized that it's doubled because while the COUNT(n.nid) part of the following query is rewritten by db_distinct_field, the SUM(l.comment_count) part is not. Here is the query from forum.module and before and after applying patch #204 and db_rewrite_sql.

    Query from forum.module in 6.14

    SELECT r.tid, COUNT(n.nid) AS topic_count, SUM(l.comment_count) AS comment_count 
    FROM {node} n INNER JOIN {node_comment_statistics} l ON n.nid = l.nid 
    INNER JOIN {term_node} r ON n.vid = r.vid 
    WHERE n.status = 1 GROUP BY r.tid
    

    Query before #204 with db_rewrite_query

    SELECT r.tid, COUNT(n.nid) AS topic_count, SUM(l.comment_count) AS comment_count FROM {node} n 
    INNER JOIN {node_comment_statistics} l ON n.nid = l.nid 
    INNER JOIN {term_node} r ON n.vid = r.vid INNER JOIN {node_access} na ON na.nid = n.nid 
    WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 2 AND na.realm = 'node_privacy_byrole_role') OR (na.gid = 5 AND na.realm = 'node_privacy_byrole_role') OR (na.gid = 7 AND na.realm = 'node_privacy_byrole_user'))) AND ( n.status = 1 )GROUP BY r.tid
    

    Query after applying #204 with db_rewrite_query

    SELECT r.tid, COUNT(DISTINCT(n.nid)) AS topic_count, SUM(l.comment_count) AS comment_count 
    FROM {node} n INNER JOIN {node_comment_statistics} l ON n.nid = l.nid 
    INNER JOIN {term_node} r ON n.vid = r.vid 
    INNER JOIN {node_access} na ON na.nid = n.nid 
    WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 2 AND na.realm = 'node_privacy_byrole_role') OR (na.gid = 5 AND na.realm = 'node_privacy_byrole_role') OR (na.gid = 7 AND na.realm = 'node_privacy_byrole_user'))) AND ( n.status = 1 )GROUP BY r.tid
    

    It looks like the regex in db_distinct_field doesn't modify the SUM part of the query but instead just the COUNT part. As a result, the topic count is accurate after applying patch #204 but not the comment_count.

    I'm not sure if this is relevant to the Views issue which started the discussion but thought I'd mention it just in case.

    tyr’s picture

    Status: Reviewed & tested by the community » Needs work
    FileSize
    1.27 KB

    Thanks Julia, this is definitely another drawback.

    The problem is, that the regex was constructed to insert DISTINCT() only once per query. This is fine as long as one doesn't handle with aggregate functions (or subqueries or something similar).

    In my opinion, letting the regex insert as many DISTINCTs as possible is the only way to fix this.

    tyr’s picture

    Status: Needs work » Needs review

    maybe "needs review" is a more appropriate status...

    dawansv’s picture

    Was experiencing same problem after upgrade from 6.13 to 6.14 with Views 6.x-2.6 and Workflow Access 6.x-1.1.

    Patch #227 fixed the problem for me.

    Thank you.

    JuliaKM’s picture

    I tested #227 against the following scenario:

    Drupal 6.14 with Node privacy by role. In order to trigger the count problem, I set up a forum with access restricted by content type and a new user (Jon Doe) with two different roles. (You can see way more detail about the ways I've tested forum access + forum in #113611: Forum count incorrect when using access control modules)

    After applying #227, the topic count was no longer duplicated but the post count (topic count + comment count) still was. I'm not sure that this is a problem with this patch or somehow connected to another issue.

    After applying #227, my new query was:

    SELECT r.tid, COUNT(DISTINCT(n.nid)) AS topic_count, SUM(l.comment_count) AS comment_count FROM {node} n 
    INNER JOIN {node_comment_statistics} l ON n.nid = l.nid 
    INNER JOIN {term_node} r ON n.vid = r.vid INNER JOIN {node_access} na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 2 AND na.realm = 'node_privacy_byrole_role') OR (na.gid = 3 AND na.realm = 'node_privacy_byrole_role') OR (na.gid = 4 AND na.realm = 'node_privacy_byrole_role') OR (na.gid = 12 AND na.realm = 'node_privacy_byrole_user'))) AND ( n.status = 1 )GROUP BY r.tid
    

    Also, I think that #227 is patched from the includes directory instead of the root one. Here's a re-rolled patch.

    Manuel Garcia’s picture

    I've tested the patch in #230, and it applied fine.

    I was having a problem after enabling the access control module of organic groups, just wanted to let you know that the problem is now gone (was seeing duplicated view rows only if current uid was 0 or 1).

    Not bold enough to set the status to reviewed though!

    Damien Tournoud’s picture

    Status: Needs review » Needs work

    The latest patches have lost the tests, plus those need to be extended to cover the newly identified use cases.

    tyr’s picture

    Status: Needs work » Reviewed & tested by the community
    FileSize
    1.13 KB

    forget everything I wrote in #227. It's just bullshit.

    The original patch from #204 is just fine... and the bug #226 (and #230) has nothing to do with this issue because forum.module doesn't call db_distinct_field() for the field to be summed. I don't know what I was thinking when I wrote this "patch". Sorry.

    To avoid confusion, I'm re-posting the patch from #204 above.

    ajayg’s picture

    Status: Reviewed & tested by the community » Needs work

    Sorry This should not be RTBC in light of the comment #226. #226 is clearly saying although patch worked mostly, it still showing some issue with forum module. Since Forum is in core, any patch here should include a fix for forum module as well. If that is a different issue with fourm, please atleast create another issue and provide a link here. But I wonder if forum issue is really a different issue since it wasn't there prior to 6.14 (unless I am mistaken)

    tyr’s picture

    Status: Needs work » Reviewed & tested by the community

    Here is the link to the issue within forum.module: #113611: Forum count incorrect when using access control modules

    Furthermore, the bug regarding the wrong posts-count was already present in <= 6.13 and has not been affected by any of the patches discussed here. (the second part of the issue in #113611 regarding the wrong topic-count indeed wasn't there in 6.13, but has been solved by the patch #204).

    JuliaKM’s picture

    Just wanted to clarify that this issue only relates to #113611: Forum count incorrect when using access control modules in that applying patch #233 adds a DISTINCT to the COUNT(n.nid) for this query in forum_get_forums:

        $sql = "SELECT r.tid, COUNT(n.nid) AS topic_count, SUM(l.comment_count) AS comment_count FROM {node} n INNER JOIN {node_comment_statistics} l ON n.nid = l.nid INNER JOIN {term_node} r ON n.vid = r.vid WHERE n.status = 1 GROUP BY r.tid";
        $sql = db_rewrite_sql($sql); //db_distinct_field called
    

    db_distinct_field is called in forum_get_forums within db_rewrite_sql and as a result wraps COUNT(n.nid) in DISTINCT since n.nid is the primary field. Since db_distinct_field can only wrap the primary field in DISTINCT, there doesn't seem to be any way that #233 could fix both the comment and node duplication described in #113611: Forum count incorrect when using access control modules.

    However, I think it's relevant that after #233 is applied, the COUNT(n.nid) is now accurate for sites using forum and a contributed access control module when it previously was not. This will be new behavior for anyone using using forum with an access control module. I'm not sure whether that's relevant for this issue, the other one, or both.

    For very detailed discussion of how the COUNT(n.nid) and SUM(l.comment_count) values are inaccurate, check out #113611: Forum count incorrect when using access control modules. Any help you can offer would be much appreciated. It is my understanding that this problem existed before 6.14.

    asb’s picture

    Damien Tournoud’s picture

    Status: Reviewed & tested by the community » Needs work

    Please roll the tests in.

    tyr’s picture

    Which tests, Damien?
    The patch #233 is the same as in #204. And that one was considered as RTBC...

    Steven Jones’s picture

    Probably the tests from #206. Where should they go? Probably a patch for the simpletest module?

    bwynants’s picture

    Status: Needs work » Reviewed & tested by the community

    #233 works just fine

    Damien Tournoud’s picture

    Status: Reviewed & tested by the community » Needs work

    @bwynants: how do you know that the patch works fine if there are no tests?

    I leave as an exercise for the reader to count the number of people that said that the patch in #122 was working fine.

    At the minimum, I would like to see the tests from #206 rolled in the patch, as modules/system/tests/database.test.

    bwynants’s picture

    @Damien: because I had a duplicate in several view lists and that node is no longer listed twice now....

    dugh’s picture

    After upgrading to 6.14 any views showing nodes that are in more than 1 group are duplicated (once per group), as described in this issue: http://drupal.org/node/583170

    The og maintainer says that it is because of this bug.

    realityloop’s picture

    I'm getting duplicates from some views with multiple taxonomies that I think is related to this:

    Export of View:

    $view = new view;
    $view->name = 'lerp_admin_coord_centre';
    $view->description = 'LERP Admin Coordination Centre';
    $view->tag = '';
    $view->view_php = '';
    $view->base_table = 'node';
    $view->is_cacheable = FALSE;
    $view->api_version = 2;
    $view->disabled = FALSE; /* Edit this to true to make a default view disabled initially */
    $handler = $view->new_display('default', 'Defaults', 'default');
    $handler->override_option('relationships', array(
      'flag_content_rel' => array(
        'label' => 'flag',
        'required' => 0,
        'flag' => 'lerp_appoved',
        'user_scope' => 'any',
        'id' => 'flag_content_rel',
        'table' => 'node',
        'field' => 'flag_content_rel',
        'relationship' => 'none',
      ),
    ));
    $handler->override_option('fields', array(
      'field_autotitle_value' => array(
        'label' => '',
        'alter' => array(
          'alter_text' => 0,
          'text' => '',
          'make_link' => 0,
          'path' => '',
          'link_class' => '',
          'alt' => '',
          'prefix' => '',
          'suffix' => '',
          'target' => '',
          'help' => '',
          'trim' => 0,
          'max_length' => '',
          'word_boundary' => 1,
          'ellipsis' => 1,
          'strip_tags' => 0,
          'html' => 0,
        ),
        'empty' => '',
        'hide_empty' => 0,
        'empty_zero' => 0,
        'link_to_node' => 0,
        'label_type' => 'none',
        'format' => 'default',
        'multiple' => array(
          'group' => TRUE,
          'multiple_number' => '',
          'multiple_from' => '',
          'multiple_reversed' => FALSE,
        ),
        'exclude' => 0,
        'id' => 'field_autotitle_value',
        'table' => 'node_data_field_autotitle',
        'field' => 'field_autotitle_value',
        'relationship' => 'none',
      ),
      'tid' => array(
        'label' => 'Building',
        'alter' => array(
          'alter_text' => 0,
          'text' => '',
          'make_link' => 0,
          'path' => '',
          'link_class' => '',
          'alt' => '',
          'prefix' => '',
          'suffix' => '',
          'help' => '',
          'trim' => 0,
          'max_length' => '',
          'word_boundary' => 1,
          'ellipsis' => 1,
          'strip_tags' => 0,
          'html' => 0,
        ),
        'type' => 'separator',
        'separator' => ', ',
        'empty' => '',
        'link_to_taxonomy' => 0,
        'limit' => 1,
        'vids' => array(
          '2' => 2,
          '1' => 0,
          '8' => 0,
          '7' => 0,
          '5' => 0,
          '6' => 0,
          '4' => 0,
          '3' => 0,
        ),
        'exclude' => 0,
        'id' => 'tid',
        'table' => 'term_node',
        'field' => 'tid',
        'relationship' => 'none',
      ),
      'tid_1' => array(
        'label' => 'Floor',
        'alter' => array(
          'alter_text' => 0,
          'text' => '',
          'make_link' => 0,
          'path' => '',
          'link_class' => '',
          'alt' => '',
          'prefix' => '',
          'suffix' => '',
          'help' => '',
          'trim' => 0,
          'max_length' => '',
          'word_boundary' => 1,
          'ellipsis' => 1,
          'strip_tags' => 0,
          'html' => 0,
        ),
        'type' => 'separator',
        'separator' => ', ',
        'empty' => '',
        'link_to_taxonomy' => 0,
        'limit' => 1,
        'vids' => array(
          '4' => 4,
          '1' => 0,
          '8' => 0,
          '7' => 0,
          '5' => 0,
          '6' => 0,
          '2' => 0,
          '3' => 0,
        ),
        'exclude' => 0,
        'id' => 'tid_1',
        'table' => 'term_node',
        'field' => 'tid',
        'relationship' => 'none',
      ),
      'tid_2' => array(
        'label' => 'Room',
        'alter' => array(
          'alter_text' => 0,
          'text' => '',
          'make_link' => 0,
          'path' => '',
          'link_class' => '',
          'alt' => '',
          'prefix' => '',
          'suffix' => '',
          'help' => '',
          'trim' => 0,
          'max_length' => '',
          'word_boundary' => 1,
          'ellipsis' => 1,
          'strip_tags' => 0,
          'html' => 0,
        ),
        'type' => 'separator',
        'separator' => ', ',
        'empty' => '',
        'link_to_taxonomy' => 0,
        'limit' => 1,
        'vids' => array(
          '3' => 3,
          '1' => 0,
          '8' => 0,
          '7' => 0,
          '5' => 0,
          '6' => 0,
          '2' => 0,
          '4' => 0,
        ),
        'exclude' => 0,
        'id' => 'tid_2',
        'table' => 'term_node',
        'field' => 'tid',
        'relationship' => 'none',
      ),
      'edit_node' => array(
        'label' => '',
        'alter' => array(
          'alter_text' => 0,
          'text' => '',
          'make_link' => 0,
          'path' => '',
          'link_class' => '',
          'alt' => '',
          'prefix' => '',
          'suffix' => '',
          'help' => '',
          'trim' => 0,
          'max_length' => '',
          'word_boundary' => 1,
          'ellipsis' => 1,
          'strip_tags' => 0,
          'html' => 0,
        ),
        'text' => 'edit',
        'exclude' => 0,
        'id' => 'edit_node',
        'table' => 'node',
        'field' => 'edit_node',
        'relationship' => 'none',
      ),
      'field_draggable_sort_value' => array(
        'label' => '',
        'alter' => array(
          'alter_text' => 0,
          'text' => '',
          'make_link' => 0,
          'path' => '',
          'link_class' => '',
          'alt' => '',
          'prefix' => '',
          'suffix' => '',
          'help' => '',
          'trim' => 0,
          'max_length' => '',
          'word_boundary' => 1,
          'ellipsis' => 1,
          'strip_tags' => 0,
          'html' => 0,
        ),
        'link_to_node' => 0,
        'label_type' => 'none',
        'format' => 'default',
        'multiple' => array(
          'group' => TRUE,
          'multiple_number' => '',
          'multiple_from' => '',
          'multiple_reversed' => FALSE,
        ),
        'exclude' => 0,
        'id' => 'field_draggable_sort_value',
        'table' => 'node_data_field_draggable_sort',
        'field' => 'field_draggable_sort_value',
        'relationship' => 'none',
      ),
      'name' => array(
        'label' => 'Faculty/Division',
        'alter' => array(
          'alter_text' => 0,
          'text' => '',
          'make_link' => 0,
          'path' => '',
          'link_class' => '',
          'alt' => '',
          'prefix' => '',
          'suffix' => '',
          'target' => '',
          'help' => '',
          'trim' => 0,
          'max_length' => '',
          'word_boundary' => 1,
          'ellipsis' => 1,
          'strip_tags' => 0,
          'html' => 0,
        ),
        'empty' => '',
        'hide_empty' => 0,
        'empty_zero' => 0,
        'link_to_taxonomy' => 0,
        'exclude' => 0,
        'id' => 'name',
        'table' => 'term_data_7',
        'field' => 'name',
        'relationship' => 'none',
        'override' => array(
          'button' => 'Override',
        ),
      ),
      'ops' => array(
        'label' => '',
        'alter' => array(
          'alter_text' => 0,
          'text' => '',
          'make_link' => 0,
          'path' => '',
          'link_class' => '',
          'alt' => '',
          'prefix' => '',
          'suffix' => '',
          'target' => '',
          'help' => '',
          'trim' => 0,
          'max_length' => '',
          'word_boundary' => 1,
          'ellipsis' => 1,
          'strip_tags' => 0,
          'html' => 0,
        ),
        'empty' => '',
        'hide_empty' => 0,
        'empty_zero' => 0,
        'link_type' => 'toggle',
        'exclude' => 0,
        'id' => 'ops',
        'table' => 'flag_content',
        'field' => 'ops',
        'relationship' => 'flag_content_rel',
      ),
    ));
    $handler->override_option('sorts', array(
      'field_draggable_sort_value' => array(
        'order' => 'ASC',
        'delta' => -1,
        'id' => 'field_draggable_sort_value',
        'table' => 'node_data_field_draggable_sort',
        'field' => 'field_draggable_sort_value',
        'override' => array(
          'button' => 'Override',
        ),
        'relationship' => 'none',
      ),
      'name' => array(
        'order' => 'ASC',
        'id' => 'name',
        'table' => 'term_data',
        'field' => 'name',
        'override' => array(
          'button' => 'Override',
        ),
        'relationship' => 'none',
      ),
    ));
    $handler->override_option('filters', array(
      'type' => array(
        'operator' => 'in',
        'value' => array(
          'lerp_coord_centre' => 'lerp_coord_centre',
        ),
        'group' => '0',
        'exposed' => FALSE,
        'expose' => array(
          'operator' => FALSE,
          'label' => '',
        ),
        'id' => 'type',
        'table' => 'node',
        'field' => 'type',
        'relationship' => 'none',
      ),
    ));
    $handler->override_option('access', array(
      'type' => 'role',
      'role' => array(
        '6' => '6',
        '3' => '3',
        '5' => '5',
        '8' => '8',
        '7' => '7',
      ),
      'perm' => '',
    ));
    $handler->override_option('cache', array(
      'type' => 'none',
    ));
    $handler->override_option('title', 'LERP Administration Coordination Centre');
    $handler->override_option('header_format', '1');
    $handler->override_option('header_empty', 0);
    $handler->override_option('footer_format', '1');
    $handler->override_option('empty_format', '1');
    $handler->override_option('use_ajax', TRUE);
    $handler->override_option('items_per_page', 0);
    $handler->override_option('use_pager', '1');
    $handler->override_option('style_plugin', 'draggabletable');
    $handler->override_option('style_options', array(
      'grouping' => '',
      'override' => 1,
      'sticky' => 1,
      'order' => 'asc',
      'columns' => array(
        'title' => 'title',
        'tid' => 'tid',
        'tid_1' => 'tid_1',
        'tid_2' => 'tid_2',
        'edit_node' => 'edit_node',
        'field_draggable_sort_value' => 'field_draggable_sort_value',
      ),
      'info' => array(
        'title' => array(
          'sortable' => 0,
          'separator' => '',
        ),
        'tid' => array(
          'separator' => '',
        ),
        'tid_1' => array(
          'separator' => '',
        ),
        'tid_2' => array(
          'separator' => '',
        ),
        'edit_node' => array(
          'separator' => '',
        ),
        'field_draggable_sort_value' => array(
          'sortable' => 1,
          'separator' => '',
        ),
      ),
      'default' => 'field_draggable_sort_value',
      'tabledrag_hierarchy' => array(
        'field' => 'none',
        'handler' => 'native',
      ),
      'tabledrag_order' => array(
        'field' => 'field_draggable_sort_value',
        'handler' => 'cck',
      ),
      'draggableviews_extensions' => array(
        'extension_top' => '0',
        'extension_bottom' => '0',
      ),
      'tabledrag_order_visible' => array(
        'visible' => 0,
      ),
      'tabledrag_hierarchy_visible' => array(
        'visible' => 0,
      ),
      'draggableviews_depth_limit' => '-1',
      'tabledrag_types_add' => 'Add type',
      'tabledrag_expand' => array(
        'expand_links' => 'expand_links',
        'collapsed' => 0,
        'by_uid' => 0,
      ),
      'tabledrag_lock' => array(
        'lock' => 0,
      ),
      'draggableviews_default_on_top' => '0',
    ));
    $handler->override_option('row_options', array(
      'inline' => array(),
      'separator' => '',
    ));
    $handler = $view->new_display('page', 'Page', 'page_1');
    $handler->override_option('path', 'lerp-admin-coord');
    $handler->override_option('menu', array(
      'type' => 'normal',
      'title' => 'LERP Coord Centre',
      'description' => '',
      'weight' => '0',
      'name' => 'menu-editorlinks',
    ));
    $handler->override_option('tab_options', array(
      'type' => 'none',
      'title' => '',
      'description' => '',
      'weight' => '0',
    ));
    

    SQL Query that Views is running:

    SELECT node.nid AS nid,
       node_data_field_autotitle.field_autotitle_value AS node_data_field_autotitle_field_autotitle_value,
       node.type AS node_type,
       node.vid AS node_vid,
       node.uid AS node_uid,
       node_revisions.format AS node_revisions_format,
       node_data_field_draggable_sort.field_draggable_sort_value AS node_data_field_draggable_sort_field_draggable_sort_value,
       term_data_7.name AS term_data_7_name,
       term_data_7.vid AS term_data_7_vid,
       term_data_7.tid AS term_data_7_tid,
       flag_content.content_id AS flag_content_content_id
     FROM node node 
     LEFT JOIN flag_content flag_content_node ON node.nid = flag_content_node.content_id AND flag_content_node.fid = 2
     LEFT JOIN content_type_lerp_coord_centre node_data_field_autotitle ON node.vid = node_data_field_autotitle.vid
     LEFT JOIN node_revisions node_revisions ON node.vid = node_revisions.vid
     LEFT JOIN content_field_draggable_sort node_data_field_draggable_sort ON node.vid = node_data_field_draggable_sort.vid
     LEFT JOIN term_node term_node ON node.vid = term_node.vid
     LEFT JOIN term_data term_data_7 ON term_node.tid = term_data_7.tid AND term_data_7.vid = 7
     LEFT JOIN flag_content flag_content ON node.nid = flag_content.content_id AND (flag_content.fid = 2 AND flag_content.uid = ***CURRENT_USER***)
     WHERE node.type in ('lerp_coord_centre')
       ORDER BY node_data_field_draggable_sort_field_draggable_sort_value ASC
    

    Example of output:(this output is the same nid)
    H6.90 Board Room / H / 6 / H6.90 / edit / / Revoke
    H6.90 Board Room / H / 6 / H6.90 / edit / Berwick Campus Faculty of Information Technology / Revoke

    RoboPhred’s picture

    suscribe

    Sohodojo Jim’s picture

    Sorry for being a Johnny-Come-Lately at the last moment of this valuable conversation, but has anyone considered what apprears to me (as a non-DB expert) to be the valuable insight from Gary Wisniewski here: http://drupal.org/node/580838?

    In his attempt to eliminate view dupes under Domain Access Advanced, he describes his success with hacking node module's hook_db_rewrite_sql to take advantage of subselects rather than distinct. He notes the accuracy and efficiency of this approach but cautions that the DB, such as MySQL 5+, must support subselects. So couldn't a db_check on this take advantage of his subselect recommendation while using the challenging distinct approach when the DB won't support it? Again, sorry if this is a ridiculous contribution to a valuable conversation. But sometimes insights/recommendations get lost in the vast mountain of info that the Drupal conversation has become.

    TCRobbert’s picture

    Subscribing

    alexpott’s picture

    Subscribing

    samchok’s picture

    subscribe

    jannalexx’s picture

    patch conflicts with views 2.7 http://drupal.org/node/579892

    Tim_O’s picture

    Running Drupal 6.14 I had this appearing after upgrading Views from 6.x-2.6 to 6.x-2.7 with postings, groups, i.e. in different views. I applied the patch from http://drupal.org/node/579892#comment-2220974 which seems to solve this for now ... The thread with the patch indicates it's really this issue, so subscribing here ...

    nonzero’s picture

    Subscribing. #233 eliminates duplicates for me on Drupal 6.14, OG 6.x-2.0, and Views 6.x-2.7. I haven't tested with forum.

    halcyonCorsair’s picture

    subscribing.

    deggertsen’s picture

    subscribe

    mathieu’s picture

    Patch in #233 "worksforme". Tested under Drupal 6.14, fixes issues I was having with Views 2.7 (no conflict, contrary to what was said in #251) and OG (2.0).

    @Damien Tournoud: not sure what tests are needed: this is only applicable to D6 and afaik there are no unit tests in this version.

    mathieu’s picture

    Not sure whether that's what was asked for, but here's a re-roll combining the latest patch with the latest tests, as described in #242. Not changing status, as Damien has asked for tests for use cases that are not covered at this time.

    mancide’s picture

    patch in #233 works for me Drupal 6.14 with views 2.7

    DanilaD’s picture

    Patch in #233 helped me also. Thanks a lot!

    izmeez’s picture

    Patch in #233 also works for me in tests with core 6.14, Views 2.7, OG 2.0, and nodequeues.

    EDIT With further testing, confirmed the existence of the problem with gmap marker not displaying output correctly, http://drupal.org/node/623234 Reverted to views 2.6 and so far all seems to be working.

    brianV’s picture

    Status: Needs work » Reviewed & tested by the community

    @DamZ - You keep insisting on tests, but this is a D6-specific patch. If you think we require tests for D6 (when there is no testing framework), please post why.

    #233 is RTBC. Setting as such.

    DamienMcKenna’s picture

    Following on the "D6 doesn't have a testing framework" notion, what other key modules should this be verified against? Several people have verified it works correctly with the latest OG and Views, what others are people concerned with?

    agentrickard’s picture

    @DamienMcKenna

    Other modules from this thread that I can recall:

    1) Any node access module (I tested with Domain Access)
    2) Taxonomy
    3) Forum
    4) Views

    @jannalexx

    Views should not patch this issue. This needs to be fixed in core. #579892: Authors and only authors see duplicate items after upgrading Drupal to 6.14 is not the proper solution.

    brianV’s picture

    @agentrickard, this patch fixed the problem with forum and forum access, views and taxonomy on my own projects.

    markuskb’s picture

    Coming from issue #586738: Duplicate items on og_ghp_ron when marked as "Public" and can confirm that the patch in #233 works for me.

    Thanks, tyr!

    Steven Jones’s picture

    Status: Reviewed & tested by the community » Needs review

    Please only test the patch from #257 from now on.

    Steven Jones’s picture

    Status: Needs review » Reviewed & tested by the community

    Patch from #257 applied cleanly to DRUPAL-6 and all tests passed.

    Skippy-2’s picture

    All - I am very new to Drupal and am enjoying working with it immensely.

    I've noted in the patches that MySQL is the database being modified. Has anyone modified the patch for PostgreSQL? I'm seeing the same effects in OG as noted in these posts - all items are duplicated. I'm configured with Drupal 6.14, Views 6.x-2.7, and Organic Groups 6.x-2.0 with PostgreSQL 8.3.8. Please let me know what I can do to fix the issue in my configuration. Also, I will provide any additional information necessary and any assistance I can give in the analysis. Thanks much!

    EDIT - noted in some previous testing that if I give the user role _administer nodes_ under _node module_ at /admin/user/permissions removes the duplication in OG. Not sure I want every user to have that permission however.

    Steven Jones’s picture

    Status: Reviewed & tested by the community » Needs work

    Hi Skippy,

    Are you able to apply the patch from #257, and set up simpletest too, and then run the database tests #257 adds? Don't do this on a production site. We could point you at the relevant documentation if needed.

    Skippy-2’s picture

    Steven - thanks for the quick reply. I have two sites (production and development) but haven't gone live yet. I haven't tried applying the patch yet as it looks like the code is vastly different but I will try it. Til now I've only looked at the code in patches and haven't actually applied them. Any relevant documentation for patch application, simpletest setup, and database tests would be awesome and I'll post back with my results.

    Steven Jones’s picture

    Applying patches:
    http://drupal.org/patch/apply

    Simpletest:
    http://drupal.org/project/simpletest
    Installs like any other module, but you also need to patch core by following step 1 here: http://drupalcode.org/viewvc/drupal/contributions/modules/simpletest/INS...

    I'll be in IRC, nick darthsteven if you need more personable help!

    adrinux’s picture

    @mathieu
    Please use git's no-prefix option when generating patches with git, like: git diff --no-prefix

    For everyone else, you can apply the patch in #257 with: patch -p1 < db_rewrite_and_tests-D6.patch

    Patch applies cleanly. Doesn't appear to break anything. Didn't fix the problem I was having with views and finder, but then I hadn't pinned it down to this bug anyway =D

    Skippy-2’s picture

    Steven - thanks for the info. I've gotten a little ways through the install and then had an issue but took too long to figure out IRC to catch you so I'll post here. Here is what I've done...

    Downloaded #257 patch to drupal root directory (with index.php)
    Attempted install of patch with input and results shown below:

    [root]# patch -p0 < db_rewrite_and_tests-D6.patch
    can't find file to patch at input line 5
    Perhaps you used the wrong -p or --strip option?
    The text leading up to this was:
    --------------------------
    |diff --git a/includes/database.mysql.inc b/includes/database.mysql.inc
    |index 43342e9..a06bd17 100644
    |--- a/includes/database.mysql.inc
    |+++ b/includes/database.mysql.inc
    --------------------------
    File to patch: includes/database.mysql.inc
    patching file includes/database.mysql.inc
    can't find file to patch at input line 18
    Perhaps you used the wrong -p or --strip option?
    The text leading up to this was:
    --------------------------
    |diff --git a/includes/database.mysqli.inc b/includes/database.mysqli.inc
    |index a2415a1..aeb980c 100644
    |--- a/includes/database.mysqli.inc
    |+++ b/includes/database.mysqli.inc
    --------------------------
    File to patch: includes/database.mysqli.inc
    patching file includes/database.mysqli.inc
    patching file b/modules/system/tests/database.test
    [root]#

    Per post #272 (http://drupal.org/node/284392#comment-2261144)
    - re-ran db_rewrite_and_tests-D6.patch with -p1

    [root]# patch -p1 < db_rewrite_and_tests-D6.patch
    patching file includes/database.mysql.inc
    Reversed (or previously applied) patch detected! Assume -R? [n] y
    patching file includes/database.mysqli.inc
    Reversed (or previously applied) patch detected! Assume -R? [n] y
    patching file modules/system/tests/database.test

    Downloaded and installed SimpleTest Module and applied patch.
    Activated SimpleTest Module and received DOMDocument error.
    - repaired with addition of php-xml module for FC11
    Receiving two notices of db error as below:

    Notice: Undefined index: in views_query->add_table() (line 263 of
    /var/www/html/devsite/sites/all/modules/views/includes/query.inc).

    I don't know where to go next and don't know if the add_table() error is from the patch or from SimpleTest - I'm not that smart. I'll muck around with it some more and will look for any suggestions you have. Thanks and have a great weekend.

    jim0203’s picture

    @Skippy: rather than using patch -p0 < patchname.patch, use patch -p1 < patchname.patch. You would usually use -p0 but this patch isn't structured quite the way other Drupal patches are structured, so you need a different flag.

    Tim_O’s picture

    I applied the patch from #257 after I had applied the patch needed for and delivered by simpletest. The patches ran successfully (notice the -p1 needed for the patchfile from #257):

    /site$ patch -p0 < D6-core-simpletest.patch 
    patching file includes/bootstrap.inc
    patching file includes/common.inc
    Hunk #1 succeeded at 2605 (offset 10 lines).
    Hunk #2 succeeded at 3731 (offset 10 lines).
    patching file install.php
    /site$ patch -p1 < db_rewrite_and_tests-D6.patch 
    patching file includes/database.mysql.inc
    patching file includes/database.mysqli.inc
    patching file modules/system/tests/database.test
    

    The patch seems to solve my problems with duplicate items otherwise (problems appeared after upgrading views from 6.x-2.6 to 2.7). Testing the created database functionality with simpletest yields: Database functionality: 13 passes, 0 fails, and 0 exceptions.

    Relevant versions used:
    -- Drupal 6.14
    -- Content Construction Kit (CCK) 6.x-2.6
    -- Organic groups 6.x-2.0 (patched with #431944: Public Audience checkbox still visible with all private group settings #11)
    -- Content Type Administration by Organic Group 6.x-1.2
    -- OG Forum 6.x-2.0
    -- SimpleTest 6.x-2.9
    -- Views 6.x-2.7

    Anything I can do further to help get this towards a release?

    mathieu’s picture

    Status: Needs work » Needs review

    Any other tests needed?

    Steven Jones’s picture

    We need to get someone to test this on a pgsql system.

    marcp’s picture

    We've been experiencing this with OG Access Control. Here's the minimum that it takes to reproduce:

    Steps to reproduce, apply the patch, and confirm that the patch fixes "something," which in this case is the display of duplicate entries at yoursite.com/og when OG Access Control is turned on:

    1. Install Drupal 6.x-dev, Organic Groups 6.x-2.0, and Views 6.x-2.7
    2. Enable OG, OG Views Integration, Views and Views UI
    3. Create a group content type and a group post content type
    4. Create a group as user 1
    5. Create a new user (jane) and add her to the group
    6. Make jane an admin of the group
    7. Log in as jane
    8. Visit yoursite.com/og
    9. Notice that there is one group listed -- this is what you would expect. We haven't turned on OG Access Control yet.
    10. Enable OG Access Control
    11. Visit admin/content/node-settings/rebuild and rebuild permissions
    12. Log in as jane again and visit yoursite.com/og -- notice now that there are 2 rows for the group you created in step 1
    13. Apply the patch from #257 with:
         patch -p1 < db_rewrite_and_tests-D6.patch
      
    14. As jane, visit yoursite.com/og and notice that there is now only 1 group listed

    Looking at the patch, and repeating what has already been said above, there is nothing in the patch that addresses pgsql. It still appears that we need pgsql users to:

    1. Confirm that there is a problem (or problems) there
    2. The patch fixes the problem(s)

    If the patch doesn't fix the problem(s) then we need someone to resubmit the patch with a pgsql fix in there.

    izmeez’s picture

    Please, excuse me if I am not understanding something.

    @marcp, in comment #278, have you tried this with Views 2.6 ?

    As I shared in comment #260 with 6.14 patched, og 2.0 and views 2.6 there does not appear to be a problem. We are using og_access. We do not have a test suite setup that we can run to test all functions but so far it seems to be ok.

    marcp’s picture

    @izmeez - No, I haven't tried it with Views 2.6. I'm not sure there's any point, really, since Views isn't going to go backwards and, as far as I can tell from reading this thread and others Merlin believes this is a core bug and there's no code in Views to work around the issue.

    My only question was whether to try this with the latest dev versions of OG and Views.

    izmeez’s picture

    @marcp - Thanks. After making the comment I realized you seemed to be trying to move forward on resolving this.

    I tried the dev version of Views a few days ago and found it was working with the core 6.14 patch but had the same problems with gmap (where a patch is being developed) but have not tried it since.

    I also tried the dev version of og a few days ago because of another issue I was try to track down and ran into some major problems that I didn't have time to investigate so I reverted back to og 2.0

    Sorry, I can't be of much help.

    serenecloud’s picture

    Subscribing

    sultancillo’s picture

    subscribing

    Hunabku’s picture

    Version: 6.x-dev » 6.14

    OK patch at #257 fixed my duplicate listings (D6.14, Views 6.x-2.7, OG 6.x-2.0)

    Steven Jones’s picture

    Version: 6.14 » 6.x-dev

    This should be against drupal 6.x-dev.

    Steven Jones’s picture

    Under PgSQL the database tests in #257 fail, specifically:

    • Add DISTINCT to SELECT table.field FROM table

      Query was rewritten to: SELECT table.field FROM table, expected query: SELECT DISTINCT(table.field) FROM table

    • Add DISTINCT to SELECT table.field AS table_field FROM table

      Query was rewritten to: SELECT table.field AS table_field FROM table, expected query: SELECT DISTINCT(table.field) AS table_field FROM table

    • Add DISTINCT to SELECT table.field,table.field2 FROM table

      Query was rewritten to: SELECT table.field,table.field2 FROM table, expected query: SELECT DISTINCT(table.field),table.field2 FROM table

    • Add DISTINCT to SELECT table.field AS table_field, table.field2 AS table_field2 FROM table

      Query was rewritten to: SELECT table.field AS table_field, table.field2 AS table_field2 FROM table, expected query: SELECT DISTINCT(table.field) AS table_field, table.field2 AS table_field2 FROM table

    • Add DISTINCT to SELECT COUNT(table.field) FROM table

      Query was rewritten to: SELECT COUNT(table.field) FROM table, expected query: SELECT COUNT(DISTINCT(table.field)) FROM table

    • Add DISTINCT to SELECT COUNT(table.field) AS table_field FROM table

      Query was rewritten to: SELECT COUNT(table.field) AS table_field FROM table, expected query: SELECT COUNT(DISTINCT(table.field)) AS table_field FROM table

    • Add DISTINCT to SELECT table.field1, COUNT(table.field), table.field2 FROM table

      Query was rewritten to: SELECT table.field1, COUNT(table.field), table.field2 FROM table, expected query: SELECT table.field1, COUNT(DISTINCT(table.field)), table.field2 FROM table

    So the code needs work to make PgSQL work properly too. Is there are reason why the PgSQL code is different in this function? Are we implementing some MySQL specific functionality with regard to the distincts and our tests?

    [The fact the first test in the list is failing is worrying]

    mathieu’s picture

    Status: Needs review » Needs work

    The patch doesn't fix Postgres (doesn't even touch the postgres file): we need someone using Postgres to submit a patch for it.

    voxpelli’s picture

    @mathieu: Agreed - as long as it doesn't block the patch for mysql - those two can be submitted independently - right?

    mathieu’s picture

    EDIT: copy/paste error in queries..

    About PgSQL... Hoping to move this forward, here's all that I found, using the test case described by marcp in #278.

    What Devel shows as query that was executed (and that returns two rows):

    SELECT n.nid FROM (SELECT DISTINCT ON (nid) * FROM node) n INNER JOIN node_access na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 0 AND na.realm = 'og_public') OR (na.gid = 1 AND na.realm = 'og_admin'))) AND ( n.status = 1 AND n.nid IN (1))
    

    What it seems it should be instead (...I'm no PgSQL expert, someone else should confirm):

    SELECT DISTINCT ON (nid) n.nid FROM (SELECT * FROM node) n INNER JOIN node_access na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 0 AND na.realm = 'og_public') OR (na.gid = 1 AND na.realm = 'og_admin'))) AND ( n.status = 1 AND n.nid IN (1));
    

    This is the line that needs to be modified:

    $query .= preg_replace('/([\{\w+\}]+)\s+(' . $table . ')\s/Usi', '(SELECT DISTINCT ON (' . $field . ') * FROM \1) \2 ', $m[2]);
    
    Skippy-2’s picture

    Steven - re: pgsql and the #257 patch...

    I created a new site(Drupal 6.14, Views 6.x-2.7, and Organic groups 6.x-2.0) with a new PostgreSQL 8.3.8 database. I then followed the steps outlined by marcp in #278 and got the exact results expected until step 14 after the patch application where I still have the duplication evident.

    I am no coding or sql guru like all of you but am willing to use this test site to verify operations of the magic you (all) create. Thank you immensely for your efforts and expertise.

    Skippy

    evoltech’s picture

    This is an interesting problem when it comes to pgsql, while mysql has the capability to added DISTINCT clauses inside select statements, pgsql does not, at least not dorectly. Postgress documentation for DISTINCT says to prepend the the DISTINCT statement to the select list.

    If db_distinct_field was called at the end of the query building this would be a simple patch to db_distinct_fiel in includes/database.pgsql.inc:

    426c426,428
    <     $query = "SELECT DISTINCT ON ($table.$field) * FROM (". $query .") node ";
    ---
    >     $query = $m[1];
    >     $query .= preg_replace('/([\{\w+\}]+)\s+(' . $table . ')\s/Usi', '(SELECT DISTINCT ON (' . $field . ') * FROM \1) \2 ', $m[2]);
    >     $query .= $m[3];
    

    But it seems that the node_access module is adding join clauses after db_distinct_field() is called and incorrectly places the clause after this substitution is made.

    I will be looking more into this tomorrow.

    On a side note I have confirmed that the patch in #257 fixes this issue for mysql using the steps from #278.

    izmeez’s picture

    I'm not sure if this is of any help or if this is even the right issue to post to because of the cross over with http://drupal.org/node/579892

    Anyway, the setup is Drupal 6.14 patched with #257 without tests active. OG 2.0 and Views tested with 2.6, 2.7 and the 2.x-dev 2009-11-12. A view of a list of users without a relationship shows only single items. When a relationship is added for OG: Group node (member) triplicates appear unless distinct is set to yes.

    myDRU’s picture

    I found out that at my side the number of times the same teaser in a frontpage is presented corresponds to the number of roles a user is assigned to + 1. E.g. A if a user has 2 roles assiged to, he sees all the teasers on the frontpage 3 times if he/she is logged in.

    Does this point bring value in this discussion?

    Moreover, can sb somehow briefly summarize which problem is exactly discussed in this thread and in which cicumstances it is likely to occur?

    marcp’s picture

    @myDRU - I'm assuming you are using some sort of node access control module on your site. Try applying the patch in #257 and see if it fixes your problem.

    evoltech’s picture

    Looking more into this the solution for pgsql support seems to be adding an modified version of the includes/database.inc:db_rewrite_sql() function to includes/database.pgsql.inc. Specifically we need to allow for a slightly differn sql pattern then the one matched for in db_rewrite_sql():

     $pattern = '{                                                             
          # Beginning of the string                                               
          ^                                                                       
          ((?P<anonymous_view>                                                    
            # Everything within this set of parentheses is named "anonymous view" 
            (?:                                                                   
              [^()]++                   # anything not parentheses                
            |                                                                     
              \( (?P>anonymous_view) \)          # an open parenthesis, more "anonymous view" and finally a close parenthesis.
            )*
          )[^()]+WHERE)                                                           
        }x';
    

    There is some use of recursive pattern matching here that I don't have my head fully wrapped around yet. I will try to have a patch for this tomorrow.

    arianek’s picture

    subscribing

    myDRU’s picture

    Yes, patch #257 solves the problem! Happy.
    FYI: I'm using
    node privacy byrole 6.x-1.5
    Views 6.x-2.7
    Drupal 6.14
    ... and many roles and content types.

    evoltech’s picture

    This patch fixes the bug described by @marcp in #278 on pgsql. Included in the patch is the work from #257.

    As I started looking at the test cases provided by #257 I realize that this "fix" for pgsql may not be completely ideal. The solution implemented patches both database.pgsql.inc:db_distinct_field() and database.inc:db_rewrite_sql(). The fix uses the non-standard "SELECT DISTINCT ON" format. This has a side effect that if ther is an 'ORDER BY" clause and the the distinct field and the "ORDER BY" field are different the sb will through an error.

    I was reading that the "proper" way to do this is "With judicious use of GROUP BY and subqueries in FROM the construct can be avoided, but it is often the most convenient alternative. "

    mathieu’s picture

    @evoltech: thanks for the patch! It does indeed fix the issue, but testing the use case described in #278, I get the following warnings:

        * warning: pg_query() [function.pg-query]: Query failed: ERROR: missing FROM-clause entry for table "n" LINE 1: SELECT DISTINCT ON (n.nid) * FROM (SELECT n.nid FROM node n ... ^ in /home/mathieu/drupal/drupal-6.14/includes/database.pgsql.inc on line 139.
        * user warning: query: menu_tree_check_access SELECT DISTINCT ON (n.nid) * FROM (SELECT n.nid FROM node n WHERE n.status = 1 AND n.nid IN (1)) node in /home/mathieu/drupal/drupal-6.14/includes/menu.inc on line 1006.
    
    miraclestyle’s picture

    subscribe

    marcp’s picture

    Status: Needs work » Needs review

    So the patch in #298 gets pgsql a bit further down the road. This is a confirmed issue for both mysql and pgsql.

    The patch in #257 has been confirmed by quite a few people to have fixed the problem on mysql.

    @Gábor - is there any chance that #257 can get committed without a fix for the problem in pgsql, or does this issue need to get fixed there at the same time?

    @all-the-pgsql-experts-out-there - anyone else trying to get this working on pgsql?

    deggertsen’s picture

    Here's another confirmation that the patch in #257 is working with mysql.

    el56’s picture

    subscribing; this appears to be the root cause of a bug I reported that appeared, at the time, to be within Organic Groups (http://drupal.org/node/621274).

    Mercury500’s picture

    I'm always wary of patching core and other modules because it gets to be a massive headache when it's time to upgrade. What to write-over what not, and will the new files work with the previous patch, etc.

    Any thoughts on this particular patch and it's impact/compatibility on the future update of core?

    marcp’s picture

    @Mercury500 - you are wary for good reason. I haven't helped deliver a site with a patched core in years. The patch in #257 appears to have been reviewed and tested a lot. It appears to fix an important core bug in MySQL. If you don't want to patch core, there's a module attached to one of the later comments at http://www.lullabot.com/blog/views-distinct-node-access-problems that appears to work around this issue for us.

    At this point, it seems like unless there's input on this issue from Gábor, or someone else who can commit the patch, your choices are:

    1. Patch core with #257
    2. Use the module referenced above to work around the core bug.

    mathieu’s picture

    @Mercury500 Indeed patching core is never a good thing but in this case and as noted earlier in this thread, this is a patch only for D6. The function we're patching here as been removed in D7 and thus this bug does not arise and no patching will be necessary. An apparent attempt at a workaround seems to have been in Views (pre 2.7) but was removed, so there is currently no other way around this.

    Hopefully someone with PgSQL will come up and fix the warnings left in #257... And hopefully this will find its way into the next release of D6.

    alxsvdr’s picture

    @evoltech: I just applied the patch at #298 and the problem I had with Organic Groups has been solved. Almost the same scenario as http://drupal.org/node/621274. The problem was as follows: user A who's member of 2 roles sees double postings inside every subscribed group's landing page. Such page is actually a view. Changing its properties to Distinct, then clearing caches, didn't solve the problem. User B, on the other hand, belonging to 1 role only, sees no duplicate entries. Thus, the involved queries are duplicating entries because node_access table is being referenced. Thanks a lot!

    opteronmx’s picture

    FileSize
    1.23 KB

    #143, #152 and #153

    Sorry for late, I was very busy on that time and I had lost track of my post on this issue.

    Here is the patch, if is still useful to anyone.

    EDIT: Patch is for D5.19

    bricef’s picture

    subscribe, will this be fixed in drupal 6.15?

    Dave Cohen’s picture

    Users of tac_lite (another node access module) are reporting the problem with DISTINCT caused by the patch in this thread which was checked in. #640156: Duplicates in views for anonymous users

    marcp’s picture

    @Dave Cohen - does the patch in #257 fix the problem in tac_lite you linked to?

    Ether’s picture

    Any chance that we will have Drupal 6.15 soon, where this problem (and some other problems) will be fixed ?
    I really not into patching core :(

    morthylla’s picture

    I would also like to know if this problem will fixed the next release... I'm afraid of patching the core.

    mathieu’s picture

    Status: Needs review » Needs work

    To everybody who's so in a hurry to get this into core: please help test this patch on pgsql and help fix the warnings in #299 (or tell me that they're bogus). After that, we can mark this RTBC. :-)

    marcp’s picture

    @mathieu - Does that mean there is no chance of getting this committed for MySQL only?

    brianV’s picture

    @marcp - we don't commit things that break supported databases. So no, this won't be committed unless it works for all the databases Drupal supports.

    Coornail’s picture

    Subscribed (because of #647206: Pager counts nodes twice)

    tamasd’s picture

    subscribe

    izmeez’s picture

    This issue has become so long (>300 comments) that jump to First unread comment no longer works for me using FF. This is a tad frustrating. Is there anyway to divide the issue into two parts, older and newer?

    Thanks,

    Izzy

    marcp’s picture

    @brianV - The patch in #257 fixes the bug for MySQL and doesn't change anything for pgsql, so it wouldn't break things any more than they are already broken in pgsql. If this issue were to go away for MySQL, "most" Drupal users would no longer be affected by the bug. We could then change the title to "db_rewrite_sql causing issues with DISTINCT in Postgres" or something like that.

    But it sounds like your answer is that we need to fix the bug in both databases (completely separate files) in order to get the patch committed.

    brianV’s picture

    @marcp:

    That's more or less it. This is a release blocker for 6.15, and 6.15 won't be released until this is fixed on both DBs.

    What this is waiting on is talented PGSQL people to look at it, and they are in short supply in the community.

    ñull’s picture

    +subscribing

    kentr’s picture

    What this is waiting on is talented PGSQL people to look at it, and they are in short supply in the community.

    (With respect for your desire to fix it entirely and preserve ideals [edit], as well as sympathy and respect for the affected PGSQL users) If PGSQL people are so few in the community and the MySQL patch doesn't changing anything about the PGSQL behavior, it doesn't make sense to make MySQL users wait for a userbase that hasn't found it a high enough priority to provide the resources to fix it.

    How about a compromise of committing #257 to 6.x-dev?

    I'd go so far as to suggest that they're two similar but different bugs...

    benoitg’s picture

    Well, they are not bogus, I could reproduce. I'm investigating to find a solution.

    Francesco Sottile’s picture

    thanks, it works !!!

    glennw_d’s picture

    Applying patch 257 fixed the tac-lite/views problem for us.

    Glenn

    benoitg’s picture

    Ok, I have a working patch. I still have to clean it up, remove debuging code and write a few tests. Il send it in tommorow morning.

    jannalexx’s picture

    conflicts with views. latest patch (eg. #233/257 ) worked ok with views 2.6, but views 2.8 (probably also 2.7) acts differently over distinct option and for me, I had to revert to 6.14 original database.mysqli.inc. One characteristic with #257 is the missing pager on views when access rights modules are enabled (eg. acl). I have also some other side effects on other ajax pager issues on views 2.8/original database.mysqli.inc (wrong url sometimes). Needs serious work and tests against views code in parallel. If you have some other propositions for $select = preg_replace plesase post so we can test. it seems there are conflicts/issues in '(\s*(?:,|\sAS|\)|$))/is' with views code

    marcp’s picture

    @jannalexx - Views is not going to address this problem, so you'll need to add "Distinct" onto your views. After adding distinct and applying the patch in #257, are you are still seeing problems?

    benoitg’s picture

    Status: Needs work » Needs review
    FileSize
    3.16 KB
    5.69 KB

    @brianV: Ok, here is an updated patch, that works with postgresql, doesn't change prior MySql patches, and should be much faster than the last patch (no subquery), and has additionnal tests (that it passes).

    It fixes the use case described in, #278, removes any patches to database.inc, and doesn't touch (but does includes) the mysql patches previously submitted.

    That being said, I haven't been working on Drupal for that long, but it -looks- as if this entire issue is caused by some misunderstanding of what DISTINCT actually does in both mysql and postgres by module authors, and that misunderstanding has leaked into core. See the second file (drupal_distinct_sql_misuse.sql_.txt) for some hopefully clear explanations and SQL samples.

    igorik’s picture

    patch from #257 works for me.
    Is there any reason for now to add distinct = false to each view in views?

    thanks
    Igor

    marcp’s picture

    @benoitg - Thank you for providing the patch and explanation. I don't have a pgsql environment to test this on, but I did notice that there are some coding style issues with the patch. I suspect those will have to get cleaned up (use Coder to find the issues) before this has a chance to get accepted.

    It would be great for the pgsql users to test this out.

    brianV’s picture

    @benoitg:

    First off, thanks for writing this! It looks like it should work, although I don't have a pgsql environment to test it on.

    I am going to post a link to this patch on the Drupal dev mailing list to try to get someone to test it. In the meantime, I've added a few comments below. Mainly, they have to do with coding standards, the complete list of which may be found here: http://drupal.org/coding-standards

    +++ includes/database.pgsql.inc	4 Dec 2009 19:13:21 -0000
    @@ -419,14 +419,68 @@
    +  //var_dump($table, $field, $query);
    ...
    +      //var_dump ($regex);
    ...
    +        //var_dump($matches);
    ...
    +  //var_dump($query);
    

    Please remove the commented-out var_dump()s.

    
    +++ includes/database.pgsql.inc	4 Dec 2009 19:13:21 -0000
    @@ -419,14 +419,68 @@
    +  // The second avoids messing with the query if there is alerady a distinct, this may or may not be a good idea
    

    Can you please wrap your comments to 80 characters wide?

    +++ includes/database.pgsql.inc	4 Dec 2009 19:13:21 -0000
    @@ -419,14 +419,68 @@
    +      // We should replace DISTINCT(), which doesn't do anything different than DISTINCT on the whole query
    +      // with a DISTINCT ON.  However, as DISTINCT() n ever works on either MySql or PostgreSQL (at least in postgres
    +      // 8.4.1 and MySQL 5.1.37, this may have unintended consequences.  So we leave the query as is for now.
    

    Please wrap these as well. Also, you have a space in the word 'never' on the second line.

    +++ includes/database.pgsql.inc	4 Dec 2009 19:13:21 -0000
    @@ -419,14 +419,68 @@
    +      $qualified_primary_field = $table .'.'. $field;
    ...
    +        $field_to_select = 'DISTINCT ON ('.$qualified_primary_field.') '.$qualified_primary_field;
    +        $field_to_select_count = 'DISTINCT('.$qualified_primary_field.')';
    

    Please put spaces on both sides of the concatenation operator (.). There are also other instances of this in the patch.

    +++ includes/database.pgsql.inc	4 Dec 2009 19:13:21 -0000
    @@ -419,14 +419,68 @@
    +          $order_by_elements = preg_split  ('/\s*,\s*/', $matches[4]);
    

    Please remove the spaces between the function name (preg_split) and it's arguments.

    +++ includes/database.pgsql.inc	4 Dec 2009 19:13:21 -0000
    @@ -419,14 +419,68 @@
    +          foreach($order_by_elements as $key=>$value) {
    

    Should have a space between foreach and the following parameters.

    +++ includes/database.pgsql.inc	4 Dec 2009 19:13:21 -0000
    @@ -419,14 +419,68 @@
    +          if($key_found!==FALSE) {
    

    Comparison operators should have spaces around them.

    This review is powered by Dreditor.

    hlykos’s picture

    subscribe

    drupov’s picture

    subscribe

    benoitg’s picture

    Ok, here is a rerolled patch with two additional tests.

    benoitg’s picture

    Damn CVS, tests were missing, here is the complete patch.

    mathieu’s picture

    FileSize
    5.72 KB

    Tested on pgsql - no more warnings, fixes the issue, works fine. Re-roll is *only* coding style changes.

    @benoitg ... merci!

    mathieu’s picture

    FileSize
    11.38 KB

    Forgot the test file in the previous post - also fixed some style issue in the test file.

    mathieu’s picture

    FileSize
    14.11 KB

    Sorry - still learning how to deal with git and previous version was patching the test file, not creating it. This is the good file.

    brianV’s picture

    Just a quick coding style suggestion - please put spaces on both sides of the concatenation operator (.).

    jannalexx’s picture

    MISSING PAGERS in VIEWS 2.6, this was the reason to patch core with those first generation patches that have been committed to CORE 6.14 database.mysqli.inc

    So it could be related to views. some views needs distinct some other don't, but surprisingly behavior seems different than views 2.6 when using 2.8.
    ORIGINAL 6.14 database.mysqli.inc works for me ONLY with views 2.8 but with different distinct on/off settings (fuzzy, some duplicated posts, in organic groups views also).
    NOW... CURRENT patch produced MISSING PAGERS again in views 2.8

    trial and error-->
    this one works (without those... | ... | ) I dont have a clue why:

    '/((?:^|,|\()\s*)(?<!DISTINCT\()(?:'. $table .'\.)?'. $field .'(\s*(?:,\sAS\)|$))/is',

    also... organic groups views have to be DISTINCT, some other views reverted back to NO DISTINCT option (producing single record results in 2.8!)
    ... be careful, all those tested only with access rights modules enabled (ACL, Forum access)

    take a quick look at this ACL/Views2 issue "incopatibility with views 2 (missing pager)"... http://drupal.org/node/367761

    benoitg’s picture

    @jannalexx: Can you var_dump the imput and output of db_distinct_field($table, $field, $query), with and without your changes to the regex (4 sql queries in total)? That way we can at best turn it into a unit test and fix it or at worst at least figure out why it's regressing.

    DanilaD’s picture

    Unrolled #233, patched with #257 - things seem to work as they should, no duplicates in og listing page.

    Views 2.7
    OG 6.x-2.0

    DamienMcKenna’s picture

    Could everyone who has tried with Views 2.6 or 2.7 please try Views 2.8 with the patch from #340. Thanks.

    update: corrected patch comment reference.

    benoitg’s picture

    I checked that with views 2.8 the patch from #340 still fixes our original issue on a copy of a production site (on mysql).

    I did NOT have a chance to re-test the use case in #278 under either MySql or postgres.

    mathieu’s picture

    I've been testing the use case in #278 under postgresql, works fine... but then, that was the re-roll.

    @jannalex: can you point to a detailed procedure (as in #278) that explains how to reproduce this issue? I gave it a try, but haven't been able to reproduce.

    jannalexx’s picture

    @benoitg, I have devel module, if you could give me some info or code to produce those results I could do so, sorry but some exteriments with devel vars were unsuccessful, please give me some tips
    @mathieu, you can at first follow instuctions at http://drupal.org/node/367761, but I am not sure if this will happen at once with views 2.8, if I will find the time I will try a clean installation but for now, just install acl/views and experiment with patch / views versions. Is there someone else who had views missing pager issues? bugged

    benoitg’s picture

    FileSize
    1.45 KB

    @jannalexx: I don't know if it could be done with the devel module, but here is a method:
    1- Patch with the patch from #340 (presumably you already did)
    2- Apply this patch on top of the one you are testing: patch -p1 < temp_debug.patch
    3- reproduce the bug with views 2.8
    4- Paste the output at the top of the screen here
    5- try the same test with views 2.7
    6- Paste the output at the top of the screen here
    7- Remove the patch: patch -p1 -R < temp_debug.patch
    Thanks

    evoltech’s picture

    Tested patch in #340 from @mathieu and @benoitg on postgres, by using the use case described by @marcp in #278. Verified that I was unable to reproduce the warnings described by @mathieu in #299. This patch solves this issue on postgres.

    Thank you @benoitg and @mathieu.

    jannalexx’s picture

    @benoitg
    tested as proposed, thanks for the method. Tested against views 2.6 and 2.8
    I suppose there is an issue with "group by" on views

    you can find those examples of queries in the attached file

    BBC’s picture

    Subscribing.

    A quick test with views 2.8 using the patch from #340 didn't work, had to roll back to patched views 2.7 (from http://drupal.org/node/579892).

    benoitg’s picture

    FileSize
    45.07 KB
    44.9 KB

    Humm, unfortunately I couldn't find the problem at this level. My debug patch was incorrect (didn't output the modified query if it was modified), but I could still do a meaningful comparison by reformatting the files provided by jannalexx (kompare views26.txt views28.txt &), see attached files. Both version of views generate IDENTICAL queries, except for a few group by additions. So I think the regression is indirect.

    If I had to guess, it may be caused by a count no longer matching the real query. Group-by having been used in 2.8, the node ids will return without duplicates. There are several COUNT(*) in the queries, but when passed to db_distinct_field, they will currently not be modified.

    So, in the test file, should we add the following assertion (and make it become true) to match the semantics of the db_distinct_field function? I think it should, but it may have side effects, as in many cases db_distinct_field function will still NOT return distinct values for the selected field.

    $assertions[] = array(
    'input' => array("table", "field", "SELECT COUNT(*) FROM table"),
    'expected' => 'SELECT COUNT(DISTINCT(table.field)) FROM table',
    );

    jdwfly’s picture

    FileSize
    90.84 KB
    103.44 KB

    I was directed to this issue from several other issues (in views queue and og queue) that claim this is the root cause. Error is easily reproduced with a clean install and steps listed below.

    Steps to produce error

    1. Clean install of Drupal 6.14
    2. Installed and Enabled modules - Views 6.x-2.8, Organic Groups 6.x-2.0 including OG Views Integration and OG Access Control
    3. Set Page as Group Node and Story as Group Post
    4. Setup a test user with sufficient privileges to make a group post
    5. Created a group with test user as admin of group
    6. Test user created 2 group posts (storys)

    Group page showing duplicate entires (see attached picture)

    Applied patch from #340 and group page was displaying as normal. (see attached picture)

    TyraelTLK’s picture

    Subscribing.

    webel’s picture

    subscribe.

    el56’s picture

    subscribe.

    bleen’s picture

    superscribe

    bleen’s picture

    tested patch from #340 with Domain Access 6.x-2.0 & Views 6.x-2.7. Domains has a similar problem to OG caused by this bug (#579018: Duplicates in views for anonymous users)

    This patch solves the problem of duplicate content being displayed for domain related views

    What more needs to be done to mark this RTBC?

    EDIT: tested on mysql

    marcp’s picture

    It looks like brianV is the point person for getting this to RTBC, and benoitg and mathieu have the best handle on the status of the patch in Postgres. If any of those folks would care to chime in here it may help move this thing along faster and/or get other folks to test the right things.

    If you are just getting here or returning to check the status of this issue and want to help, it seems like the best way to do so would be to test the patch in #340 against either MySQL or Postgres and report your findings here. You should use the latest released version of any modules, especially Views, during your testing.

    Report back the database you are testing and the versions of all modules involved in the test.

    realityloop’s picture

    using #278 I'm still seeing duplicates on at least one of my views that seems to be either related to this issue or the views module:

    As you can see in the image I am getting duplicate records with different data showing, hovering over edit link confirms that they are the same node though.

    Attached are export of the view, the sql that gets run and image captures of view output and node edit screen vocabularies sections.

    brianV’s picture

    Earlier, I discussed this issue with chx (who originally wrote db_rewrite_sql many moons ago) via IRC, and he suggested that since applying DISTINCT() to a specific field doesn't apply to MySQL, we should just simplify the processing by merely inserting a DISTINCT flag after the SELECT and before the list of columns to return. I've attached a patch which reflects this concept.

    Secondly, after conversation with several other 'notables' of the Drupal dev community, the consensus seems to be to get a fix out for MySQL ASAP, and do a follow up issue for Postgres.

    So, to that end, let's focus on getting a MySQL fix perfected. I personally don't care whether we do a simplification, as in the attached patch, or stick with what was basically done in #340, as long as we have something that tests out correctly.

    realityloop’s picture

    I've just tested brianV's patch at #362, it doesn't seem to break anything at my end but doesn't resolve my issues at #361.

    chx’s picture

    There is no point in changing the function signature just because the receiver does not use all params. No harm is done by passing more around. Also, I would add a comment that we use stristr because stripos is PHP5 only.

    Edit: also it must be noted that the fix might work for pgsql too because the handbook page shows SELECT DISTINCT valid there too.

    Edit2: just because people dream that slapping a DISTINCT a query would magically tell SQL they want distinct nodes, that won't happen and has little to do with the issue. If you can run your SQL with SELECT DISTINCT in phpmyadmin or command line and get different results than what Drupal runs or Drupal throws an error then follow up the issue. Otherwise, don't.

    brianV’s picture

    @chx

    I agree that the same code should be used for postgres as well, if it's valid. No sense applying DISTINCT on a per-field level in pgsql if we can't do that in MySQL.

    To that end, here is a patch incorporating your suggested changes. I've moved the function into the main database.inc, since it is now shared between all three mysql, mysqli, and pgsql.

    bleen’s picture

    just because people dream that slapping a DISTINCT a query would magically tell SQL they want distinct nodes, that won't happen and has little to do with the issue. If you can run your SQL with SELECT DISTINCT in phpmyadmin or command line and get different results than what Drupal runs or Drupal throws an error then follow up the issue. Otherwise, don't.

    @chx: I think most people are seeing duplicate nodes as the symptom of this problem when using Domain Access and/or OG and thats why comments are generally "No more duplicates..." or something like that.

    That said, I just tested the patch in #340 with Domain Access 6.x-2.0 & Views 6.x-2.8 (updated from 2.7 in my last comment) and with mySQL. That patch still works swimmingly.

    Is it now out of contention with the introduction of brianV's patches in #362 & #365 ? Should i be testing those instead?

    jdwfly’s picture

    Just tested #365 and it seems to work fine with MySQL.

    Enabled modules are Views 6.x-2.8, Organic Groups 6.x-2.0 including OG Views Integration and OG Access Control.

    jannalexx’s picture

    #365 has the views missing pager issue here (only with acl or other access module enabled),
    but if changed from
    if (!stristr('DISTINCT', $query) && preg_match('/^SELECT(.*?)FROM(.*)/is', $query, $matches)) {
    to
    if (!stristr('DISTINCT', $query) && preg_match('/^SELECT(.*?)FROM(\s*(?:,|$))/is', $query, $matches)) {
    or
    if (!stristr('DISTINCT', $query) && preg_match('/^SELECT(.*?)FROM(s*(?:,|$))/is', $query, $matches)) {
    is back to normal
    (views 2.8)

    brianV’s picture

    @jannalexx

    Not sure how these help:

    (\s*(?:,|$)) (capture all whitespace characters until the end of the line or a comma?)
    (s*(?:,|$)) (capture all 's' characters until the end of the line or a comma?)

    Is there any way you can record a problematic query from Views 2.8 as it enters the function?

    Gábor Hojtsy’s picture

    Given this turned out to be more complex then to be possible for D6.15, it did not actually block that release.

    jannalexx’s picture

    i wish i could help more but I just used those lines of original 6.14 patch code as "try and error". As I can see original (\s*(?:,|$)) has something to do in back stages.. under specific situations maybe? (in conjunction with an access rights module query?). (\s*(?:,|$)) is working for me as original 614 so far.
    views preview / query report is the same, the only side effect is the missing pager. I do not have a clue why, this is an old but still there bug...
    I cannot capture modified queries, if you have a way please tell us with detailed info like #349 so we can report back.

    Gábor Hojtsy’s picture

    Cross-post?

    brianV’s picture

    @jannalex:

    1. Apply the earlier patch (#365)
    2. Overwrite the db_distinct_field() function in includes/database.inc with the below code.
    3. Visit a view that is missing a pager.
    4. Copy the generated messages into a text file, and post the text file in this issue.

    This way I will be able to see how this rewriting may be affecting Views.

    function db_distinct_field($table, $field, $query) {
      $matches = array();
      
      // We use stristr() because stripos() is only available in PHP 5.
      if (!stristr('DISTINCT', $query) && preg_match('/^SELECT(.*?)FROM(.*)/is', $query, $matches)) {
      
        // If the user is an admin, print the original query.
        if (user_access('administer users')) {
          drupal_set_message('Original query: ' . $query);
        }
        
        $query = 'SELECT DISTINCT '. $matches[1] .'FROM'. $matches[2];
        
        // If the user is an admin, print the modified query.
        if (user_access('administer users')) {
          drupal_set_message('Modified query: ' . $query);
        }
      }
      
      return $query;
    }
    
    iwkse’s picture

    Drupal: 6.14
    OG: 6.x-2.0
    DB: PostgreSQL

    When i create a group it's listed twice both in the homepage & the group list. The patch fixes the issue with listing once in the group list but it breaks the homepage showing the following error and the default welcome page.

    * warning: pg_query() [function.pg-query]: Query failed: ERROR: syntax error at end of input LINE 1: ...ealm = 'og_admin'))) AND ( n.promote = 1 AND n.status = 1 ) ^ in /home/www/htdocs/drupal/includes/database.pgsql.inc on line 139.
    * user warning: query: SELECT COUNT(*) FROM (SELECT n.nid, n.sticky, n.created FROM drup_node n INNER JOIN drup_node_access na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 5 AND na.realm = 'user_relationship_node_access_1') OR (na.gid = 4 AND na.realm = 'user_relationship_node_access_author') OR (na.gid = 0 AND na.realm = 'og_public') OR (na.gid = 119 AND na.realm = 'og_admin') OR (na.gid = 123 AND na.realm = 'og_admin'))) AND ( n.promote = 1 AND n.status = 1 ) in /home/www/htdocs/drupal/modules/node/node.module on line 1745.
    * warning: pg_query() [function.pg-query]: Query failed: ERROR: missing FROM-clause entry for table "n" LINE 1: SELECT DISTINCT ON (n.nid) * FROM (SELECT n.nid, n.sticky, n... ^ in /home/www/htdocs/drupal/includes/database.pgsql.inc on line 139.
    * user warning: query: SELECT DISTINCT ON (n.nid) * FROM (SELECT n.nid, n.sticky, n.created FROM drup_node n INNER JOIN drup_node_access na ON na.nid = n.nid WHERE (na.grant_view >= 1 AND ((na.gid = 0 AND na.realm = 'all') OR (na.gid = 5 AND na.realm = 'user_relationship_node_access_1') OR (na.gid = 4 AND na.realm = 'user_relationship_node_access_author') OR (na.gid = 0 AND na.realm = 'og_public') OR (na.gid = 119 AND na.realm = 'og_admin') OR (na.gid = 123 AND na.realm = 'og_admin'))) AND ( n.promote = 1 AND n.status = 1 )ORDER BY n.sticky DESC, n.created DESC) node LIMIT 10 OFFSET 0 in /home/www/htdocs/drupal/modules/node/node.module on line 1745.

    iwkse’s picture

    For some reasons i missed a lot of posts..i've tried the patch #365 and it fixes the issue with groups and PostgreSQL too.

    chx’s picture

    I was not paying enough attention. Why the complicated preg? just check whether it starts with SELECT and change that to SELECT DISTINCT. Ie if preg_match('/^SELECT/' preg_replace('/^SELECT/', 'SELECT DISTINCT

    brianV’s picture

    Patch implementing chx's latest suggestion:

    butler360’s picture

    Guess this didn't make it into 6.15. Subscribing.

    jdwfly’s picture

    Patch from #377 is working for me with MySQL. I also can confirm that I have a pager as well.

    Enabled modules are Views 6.x-2.8, Organic Groups 6.x-2.0 including OG Views Integration and OG Access Control.

    benoitg’s picture

    > Earlier, I discussed this issue with chx (who originally wrote db_rewrite_sql many moons ago) via IRC, and he
    > suggested that since applying DISTINCT() to a specific field doesn't apply to MySQL, we should just simplify
    > the processing by merely inserting a DISTINCT flag after the SELECT and before the list of columns to return. > I've attached a patch which reflects this concept.

    No argument from me there, but in this case why DO we keep a db_distinct_field() function that doesn't do what it's name says, doesn't use it's first two parameters, and apparently causes endless confusion? How do we define, much less test if such a function works "properly"

    > Secondly, after conversation with several other 'notables' of the Drupal dev community, the consensus seems > to be to get a fix out for MySQL ASAP, and do a follow up issue for Postgres.

    Why? No one has reported any postgresql issues with the postgres patches provided, and the patches include a test campaign. That would be dangerously close to intentionally breaking postgres. That there are still issues with MySql isn't a valid reason to refuse the postgresql part of the patch.

    That being said, even though my initial issues are with MySql, I provided a postgresql patche that makes the function do what it says it does, and fixes the test case from this ticket.

    So from my point of view, there are 3 possibilities:
    1- Rewrite the function to make it do what it says it does in MySql.
    This is possible with complex rewriting of GROUP BY and ORDER BY statements, but highly non-trivial to do with regexes.
    2- Rename the function db_distinct()
    This does not really make no sense (as long as db_rewrite_sql exists, the final select list are unknowable, and as such the result is indeterminate).
    3- Get rid of the function completely, forcing modules to use group by appropriately if they want distinct nodes.
    The big problem with this it that it's hard to know how many modules that would touch.
    4- Same as 2, but name the function deprecated_db_distinct() (and document in db_rewrite_sql's comments).
    But it would certainly help put an end to the confusion, but may still cause regressions in some modules.

    > So, to that end, let's focus on getting a MySQL fix perfected. I personally don't care whether we do a
    > simplification, as in the attached patch, or stick with what was basically done in #340, as long as we have
    > something that tests out correctly.

    What we need is for someone to provide a query improperly transformed by the MySql part of the patch in #340 to move forward.

    I SUSPECT the issue is that COUNT will not return the same count as the number of rows of the "full" query in some circumstances.

    But it's hard to fix (and make consistent) unless chx (or someone else) can tell what is the actual expected result of passing a query to db_distinct_field(). If someone can, I'll be glad to write tests to represent the behaviour, and make them pass.

    jannalexx’s picture

    @brianV

    tried to isolate view in views edit page, results are refreshed on the fly on "preview"
    tested against views 2.8 and 2.6
    tested both rev2 and #377 rev3, same results

    (edit) additional info: Not all views behave this way. This is a view with a "Provide default argument" Node: Nid Node ID from URL

    merlinofchaos’s picture

    When testing with Views, be sure to use 2.8 or latest -dev and try it both with the Distinct setting ON and OFF, because the results could be different.

    drein’s picture

    I have drupal 6.14 and I used this patch with good result.
    Now I should install views 2.8 but overall drupal 6.15.
    Is the last patch good also for d6.15?

    alexpott’s picture

    +1 for the patch in #377 as this will fix this issue as well... http://drupal.org/node/596102

    brianV’s picture

    @benoitg:

    You make good points, but there are a few things that need to be kept in mind:

    1. In the 15 releases of D6 to date, this function has not ever worked as intended on MySQL.
    2. All core code, and any contrib that uses this function and db_rewrite_sql() have been made to work with the incorrect output of this function.

    The net result is, while we *could* put a lot of effort into making the MySQL version of db_distict_field() work in a similar manner to the Postgres version, this would be a distinct change from the historical functionality.

    The patch posted in 377 basically maintains the previous behavior for MySQL users, and makes Postgres behave the same while hopefully correcting the bug that initiated this issue.

    If we were to alter the nature of the function, as you are suggesting, even to make it do what it was originally 'supposed' to do, this would be an API change. That is, the results of calling db_rewrite_sql(), whether in core or contrib, will return functionally different queries when the 'distinct' flag is requested. And that is not likely to fly this late in the game.

    benoitg’s picture

    Fine, that's essentially option 4.

    The only question is should we rewrite COUNT so it returns the same number of results as the same query without count?

    deggertsen’s picture

    Just confirming that the patch in #377 worked for me with my views duplicates patched against 6.15.

    brianV’s picture

    New patch, shuffled the function so that it now exists in both database.mysql.inc and database.pgsql.inc.

    realityloop’s picture

    #388 breaks the view I posted at #361, causes whitescreen when viewing the page that view is displayed on

    realityloop’s picture

    Status: Needs review » Needs work

    #388 breaks the view I posted at #361, causes whitescreen when viewing the page that view is displayed on

    agentrickard’s picture

    Yes, I think we should fix COUNT() as well.

    realityloop’s picture

    Using #377 and selecting distinct in my view does reduce the duplicate records as per #361.

    But it is not displaying the taxonomy for Faculty/Division for all records as I'd expect
    http://drupal.org/files/issues/rewrite-output.png (without distinct)

    Subsequent reloads of the page displaying the view sometimes display the Faculty/Division, sometimes don't.

    Resolved by changing method the taxonomy field was getting output as.
    Instead of
    "Taxonomy: Terms for Faculty/Division"
    Now
    "Taxonomy: All terms Faculty/Division"

    advseb’s picture

    Was the problem with views and duplicate entries fixed in 6.15 release? Or do I still need to apply this patch?

    bleen’s picture

    @advseb: this has not yet made it into core ... please test #388

    @brianV: can I assume #340 is out of contention (for now at least)?

    bleen’s picture

    In order to try and get this sorted so we can reach a happy state of patched-ness, here is a summary of what we've got.

    benoitg offered a patch (which was cleaned up a bit for coding by mathieu) which the most current version of can be found in #340. This has been tested by several people (#346,#347,#350,#354,#359) and seems to fix the issue.

    brianV offered a patch (#365) and ammended it in #377. This also has been tested by several people (#374/5, #379,#383,#384 )

    Some notable posts can be found in #380, #385 & #386.
    To summerize, benoitg suggested that we fix the functionality of db_distict_field so that it works as intended for mysql (similar to the PostgreSQL version of the function):

    So from my point of view, there are 3 possibilities:
    1- Rewrite the function to make it do what it says it does in MySql.
    This is possible with complex rewriting of GROUP BY and ORDER BY statements, but highly non-trivial to do with regexes.
    2- Rename the function db_distinct()
    This does not really make no sense (as long as db_rewrite_sql exists, the final select list are unknowable, and as such the result is indeterminate).
    3- Get rid of the function completely, forcing modules to use group by appropriately if they want distinct nodes.
    The big problem with this it that it's hard to know how many modules that would touch.
    4- Same as 2, but name the function deprecated_db_distinct() (and document in db_rewrite_sql's comments).
    But it would certainly help put an end to the confusion, but may still cause regressions in some modules.

    brianV counters that:

    The patch posted in 377 basically maintains the previous behavior for MySQL users, and makes Postgres behave the same while hopefully correcting the bug that initiated this issue.

    If we were to alter the nature of the function, as you are suggesting, even to make it do what it was originally 'supposed' to do, this would be an API change. That is, the results of calling db_rewrite_sql(), whether in core or contrib, will return functionally different queries when the 'distinct' flag is requested. And that is not likely to fly this late in the game.

    In #386 (and #390) there is call for supporting "COUNT" quieries and not just "SELECTS". So to move forward, can people please describe precisely what the mean wen they talk about "fixing count." In IRC conversation with brianV he pointed out that the original function did not rewrite COUNT queries either so this brings into question what exactly we mean when we say "fix COUNT."

    It appears we should move forward with the patch in #377 (note: #388 seems to have some problems) and figure out exactly what we mean when we say "fix COUNT" and get this thing wrapped up.

    benoitg’s picture

    > In #386 (and #390) there is call for supporting "COUNT" quieries and not just "SELECTS". So to move forward, can people please describe precisely > what the mean when they talk about "fixing count." In IRC conversation with brianV he pointed out that the original function did not rewrite COUNT > queries either so this brings into question what exactly we mean when we say "fix COUNT."

    That's correct, the original function did not mess with COUNT. The mysql part of the patch from 340 did, and this possibly caused the regressions reported by some people. The patch in 377 does not mess with count, and will likely return consistent count results.

    However, it CANNOT be commited as is. I ran some preliminary tests, and it will duplicate DISTINCT on some queries, causing syntax errors. I am rewriting the tests as we speak to (hopefully) match what was decided for the expected behaviour of the function.

    So, to summarise:
    - Everyone agrees we ARE moving forward with the patch from 377
    - It definitely can't be commited as is (will cause syntax error in at leas one case).

    I'll try to send the test campaign before the end of the day. I may not have time to make them pass however.

    benoitg’s picture

    Ok, here's is a revised patch, with tests, that fixes the issues I discovered in the patch from 377.

    - I tested that it still fixes the use case described in #278 under MySql.
    - I tested that it still fixes the use case described in #278 under PostgreSQL.
    - All unit tests pass

    Izz ad-Din’s picture

    One weird thing is that Administrator doesnt get duplicates, but anonymous does.

    Izz ad-Din Ruhulessin

    Coornail’s picture

    User 1 has no access restriction, so the query will be easier and doesn't require DISTINCT().

    bleen’s picture

    benoitg's patch in #397 worked to address the problems I have been seeing with Domain Access.

    TESTING WITH:
    drupal 6.15
    views 2.8
    domain access 2.0
    mysql

    can we get a couple more people testing and then maybe we can get Gabor to come and give his blessing.

    BenK’s picture

    Subscribing and will do testing...

    evoltech’s picture

    I am confirming @benoitg's patch in #397 provides a fix. Tested against Drupal-6.x-dev with postgres 8.3 using the recipie from @marcp in #278.

    benoitg’s picture

    Status: Needs work » Needs review

    Well, it seems to work for most people.

    However, it's unlikely to fix @jannalexx's issue.

    Since we decided to go with simply adding DISTINCT on the whole query, and thanks to the report in #381 (jannalexx you create GREAT test reports) and the description in http://drupal.org/node/647206, I am now convinced that the problem is indeed an incorrect COUNT and must be fixed in the individual modules.

    Since views 2.8 uses GROUP BY on nid (2.6 did not), it's very likely that naïve COUNT queries will frequently return 1. For example, let's suppose that there are 15 nodes in the database:

    SELECT COUNT(node.nid) FROM node.nid GROUP BY node.nid

    will return 15 rows with 1 inside it, quite unlike:

    SELECT COUNT(node.nid) FROM node.nid

    which will return 1 row with 15 in it.

    This would explain why jannalexx only sees only one result.

    It's simple to see for such simple queries, but most real queries will have joins in them. Creating a COUNT query that will return the number of rows an arbitrary query with GROUP BY would return requires:
    - striping GROUP BY table1.row1
    - replacing the SELECT list by: COUNT(DISTINCT table1.row1)

    That can only be done by the code that added the GROUP BY, which is NOT in core.

    Marking "needs review", as remaining known issue is neither a core issue, nor a regression (the problem with views 2.8 seems caused by a code change in views 2.8 that possibly incorrectly anticipated the effect of this patch).

    The solution in http://drupal.org/node/647206 is quite likely the correct one, we just need to find which module(s) create the COUNT query.

    garywiz’s picture

    I hate to say this, but it really appears this thread consists of band-aid after band-aid attempting to repair the symptoms rather than the causes of these problems.

    Using DISTINCT, in any form, to eliminate duplicates for node access queries is a bad idea. Yes, it sure seems like if you are getting duplicates, then using DISTINCT as a "band aid" could work, but DISTINCT queries just attempt to mask the problem, and valid uses of DISTINCT are too prevalent elsewhere in the code. Worse, this band-aid has been applied multiple times, in evidence from the checkboxes in views which attempt to eliminate duplicates in taxonomy queries and others places. It's even worse when you start trying to use regexes to edit SQL queries lexicographically while ignoring the semantics of complex queries. This will just lead to headaches, non-deterministic solutions, and poor performance.

    In this specific case of node access, the original design of Drupal (as I understand it) was to have a single entry in node_access for each node. In that simple world, this kind of query could tell you which nodes to display:

    SELECT n.nid FROM node n INNER JOIN node_access na ON n.nid = na.nid WHERE na.grant_view >= 1;
    

    As OG and other modules came along, the node_access tables started getting populated with multiple node_access realms, the core code has not deviated from this fundamentally flawed theory of using joins to augment queries with access qualifiers, resulting in duplicates. I think somewhere, somebody said "Duplicates? Just add DISTINCT to the query!" and it has just gotten totally out of hand.

    Until nested selects, formulating a query which would work well was hard, and maybe DISTINCT (or GROUP BY) may have been the best hack. But with more modern engines that support nested selects (both mysql and pgsql), we can express the actual intention in a way that allows rewrites while avoiding duplicates entirely:

    SELECT n.nid FROM node n WHERE ((SELECT COUNT(*) FROM node_access na WHERE na.nid = n.nid AND na.grant_view >= 1) > 0);
    

    Without using DISTINCT or any workarounds, this query chooses only nodes which have one or more matching entries in the grant table. It will work with node query rewrites whether it occurs in simple queries, pager counts, or complex view queries.

    When we saw the problem in the query logs, it was obvious what was happening. Despite many attempts, all we could find was workarounds, and so we patched our node.module (as described in #580838: Duplicate nodes in views - how to eliminate them) to replace joins with nested selects. This instantly eliminated virtually every single duplicate in our site, including those which used OG, TAC, and Domain Access in combination. It hasn't been a problem since, and others have been using it with success as well. Applying this generally probably requires consideration for older MySQL implementations and how to deal with the absence of nested selects, but corrupting the entire codebase to accommodate obsolete versions seems highly undesirable.

    Maybe I am missing something, I must be. I haven't read every single post on this subject and I surely have not even dared start applying patches which attempt to solve this with regex rewrites, which are bound to make things worse (even if they fix most of the issues). I hate to see kludgy DISTINCT rewrites get written into core and propagating the mess to all kinds of Drupal modules, so that's why I wrote this comment.

    Why isn't a simple fix as I described the cure-all for this?

    Please, tell me this is more complicated than I think it is.

    jannalexx’s picture

    @benoitg.
    1. As a matter of fact I think single record result could be right, because the parent view has a "nid from url" argument on top. I dont know.
    2. All modules that handle access rights can produce the missing pager. Disabling ALL of them one by one (and only after rebuilding access permissions) i have the pager again. Tested with. ACL, og_access and flag_friend_access (probably others, maybe all!). Also same behavior if enabled separately. I am sure you can reproduce as noted in #348 (clean installation and instructions @ http://drupal.org/node/366419#comment-1233418, http://drupal.org/node/367761)

    But the question is, what "similar" part of code / query, could cause this behavior on all access modules? If someone has a code patch on one of those modules, we can test again. But it has to be more than that.
    If this is a side effect or my rare case, do not take it as the rule on the progress of building this patch, but it could propose the way to make this better. I wish I could help u more, but just a single drupal user here, not a pro. if you ever need any further info, i will help. For now, i will live ok with original code on my site, with some views distinct on/off tweaks for dublicates.

    merlinofchaos’s picture

    In this specific case of node access, the original design of Drupal (as I understand it) was to have a single entry in node_access for each node. In that simple world, this kind of query could tell you which nodes to display:

    This assumption is incorrect. There is 1 record per grant id. There may be many grant ids in many realms and a user may have multiple grant IDs to a node. It is highly UNlikely that there will be a single record per node. That simply wouldn't function.

    garywiz’s picture

    This assumption is incorrect. There is 1 record per grant id. There may be many grant ids in many realms and a user may have multiple grant IDs to a node. It is highly UNlikely that there will be a single record per node. That simply wouldn't function.

    Thanks for clarifying this. I was making this assumption based based upon the observation that only single-row node_access entries could possibly be used with JOIN rewrites and still preserve the integrity of the original query. These days, the node_access table is being populated in a way that leads to multiple results for JOINs regardless. I will need to read-up on just how it got to this point sometime.

    bleen’s picture

    Status: Needs review » Reviewed & tested by the community

    Now that merlinofchaos has explained #404, and jannalexx's case seems to be an outlier (#405) I think we can move ahead with the patch in #397

    Marking as RTBC so we can hopefully get Gabor in here to make the final decision and finally have a fix in place.

    brianV’s picture

    +1 for #397.

    Thanks for cleaning it up, benoitg!

    benoitg’s picture

    > I hate to say this, but it really appears this thread consists of band-aid after band-aid attempting to repair the symptoms
    > rather than the causes of these problems.

    We all agree (I think). The original db_distinct_field was designed to abstract out a complete solution but never did.

    Now since we can't just rip off the function in the middle of the lifetime of a stable release, there are only imperfect solutions.

    > Until nested selects, formulating a query which would work well was hard, and maybe DISTINCT (or GROUP BY) may
    > have been the best hack. But with more modern engines that support nested selects (both mysql and pgsql), we can
    > express the actual intention in a way that allows rewrites while avoiding duplicates entirely:
    >
    > SELECT n.nid FROM node n WHERE ((SELECT COUNT(*) FROM node_access na WHERE na.nid = n.nid AND na.grant_view >= 1) > 0);
    >
    > Without using DISTINCT or any workarounds, this query chooses only nodes which have one or more matching entries
    > in the grant table. It will work with node query rewrites whether it occurs in simple queries, pager counts, or complex view queries.

    I believe that would be a mistake. It's arguably even more of a hack (GROUP BY IS designed for that sort of thing) and is likely to be much, much slower as the tables grow than:

    SELECT n.nid FROM node AS n JOIN node_access AS na ON ( na.nid = n.nid AND na.grant_view >= 1) GROUP BY n.nid

    Yes, your query is clearer for the programmer, but it's nearly impossible for the query optimizer to avoid nested loops as the query get's more and more complex. It solves the problem in node access, but what about node relationships? Or any other joins for that matter? Do we add additional subselects in the WHERE? That just won't scale. Because of their very nature, relational databases are quite bad at subqueries.

    Solving the root cause (assuming we keep allowing modules to mangle queries) is for everyone to understand that without special specification those queries can and frequently will return multiple duplicate node ids. The module that is the final consumer of the query must specify any grouping it needs using GROUP BY. If the JOIN list is kept flat, the query optimizer can do it's job, and relational databases are really good at optimizing joins.

    garywiz’s picture

    > Yes, your query is clearer for the programmer, but it's nearly impossible for the query optimizer to avoid nested
    > loops as the query get's more and more complex. It solves the problem in node access, but what about node
    > relationships?

    To me, the method Drupal core uses to filter out inaccessible nodes should not introduce duplicate rows in all modules. The kind query optimizations you're talking about are complex, and introducing the notion that "you will have duplicates, deal with it" pushes the task of eliminating these duplicates to each module. Not only isn't the solution centralized and optimal, but it is now being handed by many module designers with varying levels of database skill, leading to bugs where duplicates appear as well as inappropriate solutions that use database resources inefficiently. At least with the solution I am suggesting be considered, the problem of how queries are formulated is dealt with in one place where a small group can focus on its efficiency, and module designers do not get unexpected results from queries. That seems like a reasonable goal.

    > Or any other joins for that matter? Do we add additional subselects in the WHERE? That just won't scale.
    > Because of their very nature, relational databases are quite bad at subqueries.

    Relational databases are not bad at subqueries because of the nature of subqueries, but rather because they have bugs or poorly-written optimizers. In general, it is always better to express intentions to the database directly rather than trying to second-guess it's query optimization strategy. Sure, that's an ideal, and investigating query plan results is a good idea, but Drupal will inherit this decision for years and years, and it is better to have the database designers fixing their bugs than Drupal module designers implementing poor solutions which introduce application-level bugs.

    In benchmarks I have done after patching our system, the nested select queries ran faster, not slower, except in one case. And, that is using mysql 5.0.77 which has serious known bugs in subselects. Assuming that such queries are implemented carefully, I see no reason why such queries should not be as efficient or significantly more efficient than JOINS or GROUP BY implemented at the module level. MySQL's query optimizer is based upon decades-old research on query optimization and is improving in every version. PGSQL is already superior because of its long history going back to Ingres. Both optimizers collapse subselects into JOINs, so that in many cases you end up with the same query anyway. Moreover, subselects can be cached indepedently by the optimizer depending upon how they are written (without derived references). This optimization is simply not possible at the application level. Therefore, many subselect queries won't be executed at all if multiple queries refer to the same conditions (as they do in many Drupal pages). Improving the subselect optimizer is something that is important to the MySQL developers and is being actively enhanced and improved, so it will just keep getting better. Unfortunately, once every module starts trying to prune out duplicates according to their own techniques, nobody will be able to manage the mess of trying to optimize and improve all of that code en masse.

    I am definitely not saying that the nested selects I suggest are perfect. I am suggesting that they should be given more serious consideration than they are right now. At the moment, the path appears to be to push this problem out to every single person that does a query. I just think that is bad for the codebase and needlessly complicates the API. It especially solves little for D6 users who can't wait until every module steps in line with the duplicate elimination strategy.

    OK, I've said my 0.02 worth. I am not trying to upset the apple cart, only trying to do the responsible thing by pointing out alternatives.

    Izz ad-Din’s picture

    On which file do I need to apply the patch in 397?

    bleen’s picture

    @Izz ad-Din ... apply the patch from the root of your Drupal install ... see http://drupal.org/patch/apply

    Izz ad-Din’s picture

    Thanks! I was used to applying patches to specific files under winXP, so that caused the confusion.

    Izz ad-Din’s picture

    Just applied the patch, I can confirm that it is working on

    Domain Access 2.0
    Views 3.0 Alpha
    Drupal 6.15
    MySQL

    Kind regards,

    Izz ad-Din Ruhulessin

    izmeez’s picture

    Another report of success with patch in #397 applied to Drupal 6.15 using OG 2.0 and Views 2.x-dev 2009-12-08.

    Thanks,

    Izzy

    bleen’s picture

    Issue tags: +drupal-6.16-blocker

    tagging

    invi’s picture

    patch #397 saved my ass! Thanks a lot!!!

    nonzero’s picture

    @garywiz, I agree with you that our current use of DISTINCT masks the problem and we need another approach. Do you have a patch that does what you intend with the subselects or is this something that involves patching every module that uses the db? I skimmed #580838: Duplicate nodes in views - how to eliminate them but it looks like it applies to just one module.

    garywiz’s picture

    @nonzero, No I haven't formulated a formal patch for this mainly because I am still hesitant to recommend it as it doesn't have much support among those who have been discussing this far more than I have (such as in this thread), so maybe I am missing something obvious, who knows.

    That being said, comment #3 in #580838: Duplicate nodes in views - how to eliminate them does contain the exact replacement for the node_db_rewrite_sql() function in the node.module. We are using it in production as are some others and it has virtually eliminated the problem for us and we have yet to find any side-effects. It was a very pragmatic change for us, as we simply needed to get our site working and modifying such a pivotal core function was a last resort.

    agentrickard’s picture

    The two questions for the patch in #580838: Duplicate nodes in views - how to eliminate them would be:

    1) Does it work on all supported D6 database platforms?

    2) Does it have any performance impact?

    If the answers are Yes and No, then that solution might move forward.

    garywiz’s picture

    1) Does it work on all supported D6 database platforms?

    Yes, I believe. MySQL and PgSQL minimum version requirements include support for subqueries. As per #105855: Up requirements to MySQL 4.1 the choice of MySQL 4.1 (instead of 4.0) was made specifically so that there would be support for subqueries in all supported databases. Does it actually work within the context of D6 without side-effects? I believe this is a yes as well. There are some issues (discussed in #151910: Support subqueries, db_rewrite_sql broken) which prevent subqueries from being used at in modules due to lack of support for subqueries in the input arguments to db_rewrite_sql(). However, that limitation does not apply to WHERE clauses added by hook_db_rewrite_sql() and thus node_db_rewrite_sql() where the patch would be applied.

    2) Does it have any performance impact?

    A definitive answer can only come from benchmarks. Each query has its own optimization path and performance can vary based upon whether the PGSQL or MySQL optimizer is effective at the query in question. Informal benchmarks of my own indicated a performance improvement (both overall and on the ms-execution times of individual queries) after adding the patch to our (pretty complex) site, but that is just one anecdotal bit of data. It makes sense however, that there is more overhead in pruning out duplicates at the application level than any performance penalty for the subquery, but this is just a theory. My tests were done with MySQL 5.0.45 and significant improvements have been made in later versions, so if it is acceptable in my tests, I am assuming that it can only get better.

    Definitive performance information on subqueries is hard to find. MySQL has been undergoing significant subquery performance enhancements and it is an active area of development. PgSQL subquery optimization dates back to early versions of PgSQL and is (according to what I have read) already superior to MySQL.

    I am tempted to say "No, there is no performance penalty", but I believe better benchmarks need to be done on a wider variety of sites before that answer could truly be verified.

    Yes, I am hedging a bit here. :-) In a thread with 422 comments, I don't want to make any glib recommendations.

    agentrickard’s picture

    Personally, I'd like to see the current patch committed, and the subquery approach (and optimizations) moved to a new thread.

    Someone needs to lasso Gabor into looking at this patch.

    bleen’s picture

    @agentrickard: agreed ++ .... he hasn't been in IRC for weeks

    kenorb’s picture

    #397 is working, duplicates disappeared thank you!

    benoitg’s picture

    @garywiz: There is nothing in the current patch stopping someone fixing the underlying issues (mostly with node access modules) using a subselect approach, nor enabling such an approach with a patch like the one in #580838. But it really belongs in a different issue.

    Everyone agrees that not only the current patch, but the entire db_distinct_field function is a (problematic) band-aid. However, this patch has been widely tested, and fixes issues now without any apparent regressions, it should be committed.

    brianV’s picture

    I spoke with Gabor about getting #397 committed, and he asked for a summary post explaining:

    (a) who tested it
    (b) who runs it already on live sites with success
    (c) what's the summary of the issue being solved and how it is being solved

    So, without further ado:

    Who tested it:

    I've tested it personally, and it solves the initial issue. There are half a dozen further people who have posted since #397 who have found similarly that this patch solves the issue. Furthermore, the patch contains a battery of tests that thoroughly verify that the function works properly post-patch.

    Who runs it already on live sites with success

    I do, for one. We are currently using it on a 250,000+ node which makes extensive use of nodeaccess, domain access, and forum access modules, and it resolves all issues nicely, with no noticeable speed differences. In fact, a review of the patch suggests it is slightly faster, since we are using much less complex regexes.

    What's the summary of the issue being solved and how it is being solved

    The basic issue was that the function db_distinct_fields() would try to apply a DISTINCT flag on a per-field basis, and in a horribly broken way at that.

    Example:

    for query SELECT node.nid AS nid, node.uid AS uid.... and passing it 'nid' as the field to be distinct, it would try to give you SELECT DISTINCT(node.nid) AS DISTINCT(node.nid), node.uid AS uid.....

    If you used the same example query and told it to be distinct on uid, the you would get SELECT node.nid as nid, DISTINCT(node.uid) AS DISTINCT(node.uid)...

    The first apparent issue, the 'DISTINCT AS DISTINCT' syntax, is bad SQL, and that was fixed in a commit you made in #131.

    The second issue, which hasn't been dealt with and has existed since D6's release, is that MySQL doesn't support DISTINCT on a per-field basis anyhow. That is, the SQL generated by the function after your commit in #131 is only valid in Postgres.

    The effects of this are, for instance, when multiple node access modules are enabled, each is added via a LEFT JOIN to the primary table in a query, then db_distinct_field was called on the primary table's primary key. Since the DISTINCT flag added by db_distinct_field() was done incorrectly more often then not, and ignored by MySQL, users would see multiple copies of the same node appear in Views and other content listings - once for each LEFT JOIN of a node access table.

    The patch in #397 solves this by simply forcing the distinct flag to be added at the beginning of the query instead of on a per-field basis. This results in consistent behaviour in both MySQL and Postgres. That is, regardless of which field is passed to db_distinct_field(), the function is changed from SELECT node.nid AS nid, node.uid AS uid.... to SELECT DISTINCT node.nid as nid, node.uid as uid.....

    This nicely cleans up the duplicate records in content listings, and brings the behaviour to be consistent between Postgres and MySQL platforms.

    The final point I want to make is that it probably, at first glance seems like we are removing functionality by disabling the 'distinct fields' capability that db_distinct_fields() was obviously supposed to have. The counter argument here is that it has never worked on MySQL, even from day 1. To suddenly make this function behave as originally intended would be an API change, and would have ramifications throughout core and contrib as any functions that call db_rewrite_sql() would get different results.

    garywiz’s picture

    @brianV, Thanks for the lucid summary. I came to this thread late because a cross-reference was posted here to a related, but different patch that solves problems with duplicates caused node_access joins.

    @benoitg, as per your suggestion, I have created a new issue to separate the discussion of the node_access subselect patch, which solves a problem which sometimes results in similar symptoms. If anybody has input or further discussion regarding the subselect solution introduced at #419, please move such discussion to #681760: Try to improve performance and eliminate duplicates caused by node_access table joins.

    This thread is long enough and it seems best to focus on the original problem as just summarized by @brianV.

    benoitg’s picture

    Nothing to add to brianV's concise summary (considering this patch had 426 posts already) of what the patch is and does.

    As for testing:
    - We are running it in production (MySql).
    - I also tested (but not in production) on postgres.
    - The following people reported the latest version of the patch as working: bleen18, evoltech, brianV, Izz ad-Din, izmeez, invi, kenorb

    Next steps:
    Just to clarify, this patch isn't the end of all duplicate node issues. It primarily fixes sql errors that would sometimes occur, and secondarily eliminates duplicates in SOME cases where the previous code did not. It's just a better band-aid.

    To fix the fundamental problems:

    -Node access: A solution for duplicate nodes introduced by node access modules are subselects. This effort is tracked at http://drupal.org/node/681760

    -Views: Duplicate nodes in views could be caused (among other things) by node relationships (even using distinct), if the view query causes the select list to use values from joined tables. This is partially fixed in views 2.8 by using GROUP BY on node id, but this change apparently caused the missing pager issue). Most likely a count query must use the same group by. One additional difficulty views may face with this is making sure that the group by on node doesn't skew the results of queries in aggregate queries (especially since that's not easy to see except in unit tests). Subselects could solve that, but the performance cost may be high in current MySql.

    mathieu’s picture

    As per #427 suggestion

    Who tested it:

    I tested this patch personally and it solves the duplication issue in views when used together with og, on a site running MySQL 5.0.

    Who runs it already on live sites with success:

    I'm running it on a site with 10k+ nodes making extensive use of views and og, along with a bunch of views plugins and custom modules. This patch has fixed the initial problem and no further issue has been reported.

    bwynants’s picture

    Tested and Running it on 4 sites. No duplicates seen anymore

    realityloop’s picture

    I have tested this (MySQL) on 5 live sites with success

    extect’s picture

    Running on 2 live sites, each with over 70 Drupal modules and heavy views usage. No problems so far. Thank you!

    agerson’s picture

    #397 fixed the issue on my site with views showing duplicate results. Thank you.

    mariagwyn’s picture

    I applied patch in #388, and after initial testing, it appears to work.

    I applied it against 6.15, using Views 2.x-dev and OG, development site. Given that it has limited traffic at this point, I will continue to check for duplication.

    bleen’s picture

    @mariagwyn: currently #397 is the patch to test

    Martin.Schwenke’s picture

    Tested patch in #397 on one of my sites with Drupal 6.15, views-2.8 and about 20 other modules. The unwanted duplicates disappeared and I don't see any other problems.

    The patch has been running live on my site for about 24 hours. The site uses 4 views on the front page (2 blocks, 2 embedded in sticky nodes using views_embed_view()). Prior to applying the patch all views showed varying numbers of duplicates (usually 2, but each entry was shown 4 times in 1 view!). All better now. :-)

    Thanks to those who worked hard on this fix!

    TomSherlock’s picture

    Similar to comment #437, I was able to successfully suppress the duplicates which were introduced with the installation of content access.
    I upgraded to Drupal 6.15 from 6.14 and Views 2.8 from views 2.7 and applied patch from #397 using Mac's native patch program.

    I am very grateful for this thread and the patch as I had introduced content access to address a previous problem.

    Hobbes-2’s picture

    ...subscribe...

    Thanks for the patch.

    Patch fixed my duplicate listings problem on Drupal 6.15, views 6.x-2.8.

    Hope to see this in the official release soon!
    :-)

    Thanks for the patch

    Hugo

    bomarmonk’s picture

    subscribe/testing patch

    Darrin Southern’s picture

    subscribe

    quiptime’s picture

    #19: database.patch queued for re-testing.

    mabgfounder’s picture

    subscribe

    Gábor Hojtsy’s picture

    Version: 6.x-dev » 5.x-dev
    Status: Reviewed & tested by the community » Patch (to be ported)

    Thanks for all your hard work and testing. Especially for providing feedback of your test results. That is most helpful. Committed to Drupal 6. Moving down to Drupal 5 for porting.

    Gábor Hojtsy’s picture

    Oh, and by the way, I committed this patch to Drupal 6 (which has a typo fix in "guarantee" and a code style fix within the function being "moved"). Just so we carry on these fixes.

    bleen’s picture

    woooo hoooo!!!! It's a christmas miracle ... Did we hit the record for most comments needed to get a patch?

    Thanks Gabor!

    agentrickard’s picture

    There is a minor language issue in the code comment. "Kept to remain API compatibility." should be "Kept to retain API compatibility."

    bomarmonk’s picture

    Any idea when this will roll into an official release? I'm wondering if it's worth my time to upgrade to the development version.

    vitis’s picture

    subscribe

    mattiasj’s picture

    subscribe

    theunraveler’s picture

    +1

    brianV’s picture

    Version: 5.x-dev » 6.x-dev
    Status: Patch (to be ported) » Reviewed & tested by the community
    FileSize
    5.54 KB
    893 bytes

    Here is a quick fix-up patch for D6 to fix the 'remain' -> 'retain' spelling mistake.

    Also, I am attaching a D5 port of the previously committed patch. Note that the 'RTBC' is for the trivial D6 spelling correction. My D5 roll should still be reviewed.

    brianV’s picture

    Issue tags: -drupal-6.16-blocker

    Removing 6.16-blocker tag, since I don't think we are going to hold up the release based on a typo in a comment.

    acb’s picture

    Getting the problem in Drupal 6.15 and Views 6.2.8

    bleen’s picture

    @acb: have you applied the patch from #452 (or #397)? While this has been committed to HEAD, it will not be part of an official release until 6.16 lands.

    mgifford’s picture

    This looks like something that should be fixed in core, right? Would really rather avoid patching core!

    Edit: just realized there was a page 2 in the list of comments on this issue.

    Seems there are a number of patches being considered, would like to know what is the recommended one to use that is most likely to get into core.

    AntiNSA’s picture

    subscribing

    bleen’s picture

    @mgifford: Gabor already committed #445 (#452 fixes a typo in that patch)

    mgifford’s picture

    So this problem will be fixed with the next security release. In the mean time feel free to apply the patches #445 & #452 to resolve known problems with this issue.

    When the spelling mistake is fixed, then this can be marked fixed, right?

    brianV’s picture

    mgifford,

    Once the spelling mistake is committed, the issue should be changed to D5, and Needs Review, since I've added a patch in 452 which backports the D6 fix to D5.

    tborrome’s picture

    subscribe

    webservant316’s picture

    I am running Drupal 6.15 and Views 6.x-2.8. I started a post at http://drupal.org/node/715074, but was directed over here. I was recommended to install the patch at #397, but will need to wade through the above to make sure that is the right thing to do.

    I am using taxonomy, taxonomy access control, views and roles with this result...

    View query couldn't be simpler: SELECT node.nid AS nid, node.title AS node_title FROM drupal_node node

    This simple view list the titles of all nodes in my database. However, if the user is in multiple roles, it lists the title multiple times! Why!! I also have a taxonomy, 'ACCESS', defined for access permissions using the TAC module. The taxnomy is set to allow multiple selections, but none required. Here is a curious thing, all the node titles repeat multiple times if I have made no assignment of the node to a term in the 'ACCESS' taxonomy. However, if I have made an assignment of a node to a term in the 'ACCESS' taxonomy then the node title will only be diplayed 1 time!

    Wow what is going on here? Is the error in Taxonomy, TAC, Views, or myself?

    Skorpjon’s picture

    Status: Reviewed & tested by the community » Needs work

    I tested it with Workflow, Workflow_access, Views and Views_calc and and patch at #445 works only partial. The duplicates disappeared, but all queries which use COUNT, SUM, etc. still calculate wrong.

    Here is an example: http://drupal.org/node/691712

    The same happens, if you use the summary list for arguments.

    benoitg’s picture

    Status: Needs work » Reviewed & tested by the community

    That is EXPLICITELY said in the comment to this patch. This patch will NOT try to mangle aggregate functions. It has to be fixed in views. Depending on the case:

    - COUNT(DISTINCT nid) instead of COUNT(*) will likely do what you want.
    - SUM(DISTINCT expr) will probably NOT do what you want. You may have to do a subselect.

    Only the modules can know the semantically correct queries.

    Skorpjon’s picture

    Okay, thank you for the advice. :)

    greg.harvey’s picture

    For what it's worth, #298 fixes this OG issue for me: #64672: Posting to a group and to Public results in double post in group

    AntiNSA’s picture

    So is that the final answer? We should go with post #298?

    greg.harvey’s picture

    I didn't even realise there was a page 2! Doh! Looks like #397 is the patch to use and has been committed (will be part Drupal 6.16) so I don't need to save the patch! Yay! =)

    webservant316’s picture

    A summary of my research into the duplicate nodes in Views bug.

    1. When I saw this bug I started a post at http://drupal.org/node/715074 titled 'Node view filtered by taxonomy term results in duplicates by user roles!?'. Someone directed me to another post...

    2. So I joined the mega post at http://drupal.org/node/284392 titled 'db_rewrite_sql causing issues with DISTINCT'. Various patches were suggested with the leader seeming to be #397. I was ready to install the patch when another person directed me to yet another thread.

    3. So I also joined the post at http://drupal.org/node/681760 titled 'Eliminating duplicate nodes caused by node_access table joins'. Frankly this guys patch seemed to make more sense to me using the sub-select instead of the DISTINCT to get rid of duplicates which shouldn't have been there in the first place.

    4. While meditating over this issue with extremely limited understanding it seemed to me that duplicates are being created in the Views module because joins of various nature are included in the query which the views module does not seem to be aware of. In my case I figured out that my node table was joined with tables from TAC and roles because the number of duplicates matched the number of roles that my current user was linked to. The error here, again, is that the test for permission to access the node should result in one node returned, not one node for each role I am a member of.

    5. And while waiting to hear back from my posts on this issue, I had to continue with my work and while creating yet another View I noticed that the Views interface provides the option of 'distinct' yes/no. Curious. The note by this option says 'This will make the view display only distinct items. If there are multiple identical items, each will be displayed only once. You can use this to try and remove duplicates from a view, though it does not always work. Note that this can slow queries down, so use it with caution.' It seems to me that the author of the Views module recognized that sometimes duplicates would appear because of mysterious joins or whatever beyond their control and so they simply added the distinct flag to deal with the issue. In the end I simply used this flag, inspite of the performance warnings, and all the duplicates disappeared in my Views without any patch. This seemed safer for now rather than patching core. As for performance issues, I can understanding the warning because in my case my Views are reading NODE X USER-ROLES instead of just NODE number of records.

    6. I am thinking that using this distinct flag within the View interface itself might be about the same as using #397 anyway, so I plan to just use the flag instead of the patch unless someone explains something differently. Yet, the sub-select solution over at http://drupal.org/node/681760 seems to be headed in the direction of a solution for integration into the Drupal core.

    7. Last question...I only observed the duplicates in my use of the Views module and so using the 'distinct' flag in the Views module cured my problem. However, do the patches in the threads above deal with possible duplication in others areas as well or is this problem limited to the Views module?

    Well my Views problem is solved for now, but am I hoping that a solution is integrated into Drupal core that restores performance back to the optimum.

    I think I transgressed by posting this exact comment on this post and two others. Please respond to this particular comment here, http://drupal.org/node/681760#comment-2636434. Thank you.

    merlinofchaos’s picture

    If you are using a node_access based module, such as TAC lite, all of your node queries will be made DISTINCT, which is a core part of the issue here.

    bomarmonk’s picture

    I just upgraded to drupal 6.x-dev and it fixed my duplicate items showing up in views.

    mansspams’s picture

    subscribe

    zeroonnet’s picture

    #24: database.patch queued for re-testing.

    AntiNSA’s picture

    Status: Reviewed & tested by the community » Needs review
    agentrickard’s picture

    Version: 6.x-dev » 5.x-dev
    Status: Needs review » Patch (to be ported)

    Why are we still beating this way-too-long issue that has already been committed? It should either be ported to D5 or closed.

    Let's start over somewhere else if people have issues after the patch lands, please.

    brianV’s picture

    @agentrickard:

    In #452, I have:

    1. a followup patch to the D6 patch that fixes a spelling mistake.
    2. A D5 port of the committed D6 patch (and typo fix).

    If you are anxious to move this issue forward, please review these.

    brianV’s picture

    Version: 5.x-dev » 6.x-dev
    agentrickard’s picture

    Status: Patch (to be ported) » Reviewed & tested by the community

    Well, the typo fix is RTBC. But both of those would probably get more attention as separate issues so we can finally kill this monster.

    webservant316’s picture

    Perhaps the patches to core proposed above can kill this discussion thread, but aren't these patches are only a superficial solution to the underlying problem. The DISTINCT query will reduce the duplicate nodes created by a join of the node table with various access control tables, true. However, it seems to me that the core query to retrieve nodes should not retrieve duplicate nodes for every role that gives a user permission to access the node in the first place. Doesn't this force the database engine to access multiple rows of duplicate nodes and then use its DISTINCT engine to eliminate duplication unneccessarily. Why not use a sub-select to first test for permission to access the node and then read get one copy of the node. I think a similar point is made in http://drupal.org/node/681760.

    Is it even possible for core to make the test for permission to read the node and retrieve only one copy since the access control modules causing the duplication are not part of core? Perhaps the DISTINCT flag is the only way core has to eliminate the duplicates.

    If this discussion thread ends here will another thread continue about the best way to optimize the reading of nodes? Maybe I do not understanding database engines sufficiently, but I am staying in the discussion to promote keeping Drupal lightning fast.

    iva2k’s picture

    subscribe

    pgillis’s picture

    subscribe

    grub3’s picture

    Rewriting SQL queries dynamically in Drupal is a grave design flow. Drupal is the only application which does that.

    Only database query parsers like MySQL and PostgreSQL insider code does rewrite queries.

    Drupal should never ***ever*** do it.

    grub3’s picture

    First, to present myself I wrote part of this guide:
    http://drupal.org/node/555514

    I took part in the design of pgAdmin3, PostgreSQL GUI with SSL access.

    Rewriting SQL queries dynamically in Drupal is a grave design flow:

    Only database query parsers like MySQL and PostgreSQL optimizers do rewrite queries.
    And they have hundred thousands of lines in C or C++.

    A database parser and optimizer is the heart of a database, it cannot be rewritten in PHP, just for fun.
    When I read you PHP code, I don't know really if we should laugh or cry.
    We are already at issue > 482 of this thread and people are still running pregs.

    My guess is that you cannot cover all cases and this can only lead to bugs and vulnerabilities.
    I encourage you to rewrite all database queries of all modules and add distinct whenever needed, at hand.

    My very HUMBLE opinion is that you cannot write an SQL parser in PHP and this will only lead to developers writing poor SQL code without thinking whether JOINs have distinct values or not. I encourage you to contact core MySQL and PostgreSQL developers and discuss how deep you can get into problems trying to rewrite queries. My HUMBLE opinion would be to review all existing modules and add DISTINCT by hand whenever needed.

    By the way, I hope that D7 does not rewrite queries on the fly ...

    YK85’s picture

    subscribing

    pgillis’s picture

    I had duplicate content in all my views caused by the workflow access module. Upgrading drupal core to 6.16 makes the problem go away. So, from my perspective, this issue is resolved. Thanks!!

    AlexisWilke’s picture

    I just got this problem with the new 6.16 installation.

    I had to fix the blog module to make sure that my site would continue to work...

    In the following statement, I simply added the n.sticky field to the SELECT part. Having an extra field is not a killer and the blog works again! Now, it would be nice to have the Views DISTINCT work again...

      $result = pager_query(db_rewrite_sql("SELECT n.nid, n.created, n.sticky FROM {node} n WHERE n.type = 'blog' AND n.status = 1 ORDER BY n.sticky DESC, n.created DESC"), variable_get('default_nodes_main', 10));
    

    Thank you.
    Alexis Wilke

    P.S. I also posted an issue with the SimpleNews project since this new code generated a problem with that system too. #734196: Conflict with access system and PostgreSQL in Drupal 6.16

    AntiNSA’s picture

    Is this thread why I am having this problem with nodequeeue?

    user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ASC LIMIT 0, 10' at line 7 query: SELECT DISTINCT node.nid AS nid, position FROM node node INNER JOIN nodequeue_nodes nodequeue_nodes_node ON node.nid = nodequeue_nodes_node.nid AND nodequeue_nodes_node.qid = 1 LEFT JOIN nodequeue_subqueue nodequeue_nodes_node__nodequeue_subqueue ON nodequeue_nodes_node.sqid = nodequeue_nodes_node__nodequeue_subqueue.sqid WHERE nodequeue_nodes_node__nodequeue_subqueue.reference = '1' ORDER BY ASC LIMIT 0, 10 in /home/cyberfan/htdocs/modules/views/includes/view.inc on line 765.

    How can I fix it? I am using 6.16

    AntiNSA’s picture

    Status: Reviewed & tested by the community » Active

    I tried patching the patch with the spelling mistake in cygwin above and it did not work with 6.16.

    Can someone make a patch that works with 6.16 so I can use nodeque? Im changing it to active since the patch didnt work with the current version of 6.16

    brianV’s picture

    Status: Active » Reviewed & tested by the community

    @AntiNSA:

    This patch will be released with the next update of Drupal.

    In the meantime, please don't change the status of the issue.

    brianV’s picture

    Status: Reviewed & tested by the community » Patch (to be ported)

    I think this issue is becoming increasingly useless in terms of having any meaningful conversation. It's now 490 posts, of which most are not relevant to the conversation. Furthermore, 'Subscribe' comments and new reports of the same bug, and other irrelevant posts are piling on so fast that anything meaningful is quickly buried.

    I've created two new issues to hold the relevant two remaining tasks from this issue:

    1. Fix the spelling mistake in the original D6 patch -> #735114: Fix mistake in database.inc comment
    2. Backport this patch to D5 -> #735120: Fix changes to db_distinct_field() in D6.16 / PostgreSQL

    I'm leaving this at CNW for now, and will close it when either the backport from #452 in this issue, or from the new issue is committed.

    AntiNSA’s picture

    Beings I am using 6.16 and having errors with nodeque, what the problem with releasing a patch that will work on 6.16? Why only release the patch for earlier versions of drupal?

    I could use this patch now.

    Junro’s picture

    Fix with 6.16 release in my case. I don't need patch anymore :)

    Damien Tournoud’s picture

    Status: Patch (to be ported) » Fixed

    Let's consider this as fixed. See #490 for related issues.

    Locking comments.

    Status: Fixed » Closed (fixed)
    Issue tags: -Needs tests, -select distinct as distinct bug

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