Hi

Is using v 7.27-dev

When Cancel/delet an account and
mark> Delete the account and make its content belong to the Anonym user.

Messages are:
The update has been performed. - (GREEN)

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '17-0-111.111.111.111' for key 'PRIMARY': UPDATE {poll_vote} SET uid=:db_update_placeholder_0 WHERE (uid = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => 0 [:db_condition_placeholder_0] => 211 ) in poll_user_cancel() (line 992 of /home/www/xx.xxxxx.xx/modules/poll/poll.module). - (RED)

But the account is not deleted

What to do?

Pafla

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

secretsayan’s picture

Hello,

This is happening when we are deleting a user

1) who has voted for a poll and

2) Another user who had also voted in the same poll has been deleted and in both cases "Delete the account and make its content belong to the Anonymous user" option has been used.

3) Both the above mentioned users have similar host names

I am fixing it right now

Pafla’s picture

Hi

When it is fixet, do i have to create a user for this and if what shall i call it?
Or is it build in Drupal

Do you write here when fixet?

Pafla

secretsayan’s picture

Component: block.module » poll.module
Status: Active » Needs review
FileSize
591 bytes

Hey I got it fixed. Hope it helps.
Check it out...

Pafla’s picture

THX

Is it included in next drupal 7 ?

secretsayan’s picture

Yes , if its reviewed and tested by the community.

mgifford’s picture

Seems like a direct enough patch.

What's the best way to replicate this problem though?

secretsayan’s picture

@mgifford please check comment #1....

mgifford’s picture

I couldn't replicate it, sorry.

secretsayan’s picture

@mgifford replicate using the following steps-
0) Assume users John and Lucy has same host-names
1) User John has voted for a poll created by admin.
2) The admin now deletes John and uses "Delete the account and make its content belong to the Anonymous user" while deleting.
3) Now user Lucy also comes in and votes for the same poll.
4) The admin now deletes Lucy using the same "Delete the account and make its content belong to the Anonymous user" options.

And we get the following error-

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '17-0-111.111.111.111' for key 'PRIMARY': UPDATE {poll_vote} SET uid=:db_update_placeholder_0 WHERE (uid = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => 0 [:db_condition_placeholder_0] => 211 ) in poll_user_cancel() (line 992 of /home/www/xx.xxxxx.xx/modules/poll/poll.module). - (RED)

dcam’s picture

Priority: Critical » Normal
Status: Needs review » Needs work

There are a few code-style problems with #3:

  1. +++ b/modules/poll/poll.install
    @@ -213,3 +213,15 @@ function poll_update_7004() {
    + * Adding timestamp to primary key as the
    + * present composite primary key is insufficient.
    

    The docblock needs to be on one line. Something like "Add the timestamp field to the primary key." is enough information.

  2. +++ b/modules/poll/poll.install
    @@ -213,3 +213,15 @@ function poll_update_7004() {
    +  // Drop the present primary key
    ...
    +  // Add a new primary key
    

    These comments need to have a period at the end.

After that, I think this is RTBC. It may need to have a test added, but I'll let the committer make that call.

dcam’s picture

Title: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062... » Deleting users can result in a duplicate primary key in poll_vote
secretsayan’s picture

Here's the updated patch with changes according to #11 by dcam...

secretsayan’s picture

Status: Needs work » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 12: pollmoduleprimarykeychange-2268673-12.diff, failed testing.

secretsayan’s picture

Status: Needs work » Needs review
secretsayan’s picture

secretsayan’s picture

dcam’s picture

Thanks for continuing to work on this, @secretsayan!

+++ b/modules/poll/poll.install
@@ -213,3 +213,15 @@ function poll_update_7004() {
+ * Add the timestamp field to the primary key.
+ *

I hate to nitpick, but that second, blank line in the docblock needs to be removed.

dcam’s picture

Status: Needs review » Needs work
secretsayan’s picture

secretsayan’s picture

Thanks dcam! Any feedback will be appreciated..... Please go ahead if you have any. That's not at all a problem. Plz check this one out

secretsayan’s picture

Status: Needs work » Needs review

The last submitted patch, 12: pollmoduleprimarykeychange-2268673-12.diff, failed testing.

The last submitted patch, 17: pollmoduleprimarykeychange-2268673-12.patch, failed testing.

The last submitted patch, 18: pollmoduleprimarykeychange-2268673-18.patch, failed testing.

secretsayan’s picture

Status: Needs review » Reviewed & tested by the community
secretsayan’s picture

Assigned: Unassigned » secretsayan

dcam’s picture

Status: Reviewed & tested by the community » Needs review

I'm setting this back to Needs Review for now. I know I said that the patch would probably be RTBC back in #10, but no one has done a final review of it yet after your most recent changes nor have I actually tested to make sure it fixes the problem. Then as the reviewer I should be the one to set the status (or anyone else doing the review).

I'll try to get back to this tomorrow, but I can't guarantee it. I'm very busy at work right now.

rcls’s picture

I'm having the same problem.

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '585-0-' for key 'PRIMARY': UPDATE {poll_vote} SET uid=:db_update_placeholder_0 WHERE (uid = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => 0 [:db_condition_placeholder_0] => 634 ) funktiossa poll_user_cancel() (rivi 992 tiedostossa /www/username/public_html/site/modules/poll/poll.module).

I ran the patch #22, disabled \ enabled Poll, cleared cache, ran cron, but no effect. Running Drupal 7.31.

This error occurs when I attemp to clear out users who haven't logged in over 3 years and they seem to have some past content with Polls. I will attempt to go past this by disabling Poll.

*Successfully executed user deletion after disabling Polls.

secretsayan’s picture

@rcls you need to uninstall and install the poll module.

dcam’s picture

@rcls Or you could try to apply #22, run update.php, and report back as to whether it solves your problem. That would help us get this issue solved for everyone.

code-brighton’s picture

Hi
I'm also experiencing this issue. Here is the error from the logs when trying to delete a user that does have a similar username (e.g the oser I'm trying to delete is fred.smith_1 and there is another user called fred.smith one user is 3 years 1 month old, the other user is 2 years 9 months old):

PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '456024-0-' for key 'PRIMARY': UPDATE {poll_vote} SET uid=:db_update_placeholder_0 WHERE (uid = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => 0 [:db_condition_placeholder_0] => 68653 ) in poll_user_cancel() (line 992 of C:\xampp\htdocs\projects\csl_portal_drupal_7\modules\poll\poll.module).

Has any progress been made on this issue? Is the patch scheduled to be committed to core? And if so when?

secretsayan’s picture

@code-brighton the patch can only be committed to the core if someone would review it and make it RTBC

quotesBro’s picture

1) I successfully applied patch #22 but I got 1 warning:

$ git apply -v pollmoduleprimarykeychange-2268673-22.patch
pollmoduleprimarykeychange-2268673-22.patch:9: trailing whitespace.
 
Checking patch modules/poll/poll.install...
Applied patch modules/poll/poll.install cleanly.
warning: 1 line adds whitespace errors.

2) #22 didn't solve the issue for me (I applied #22 and ran update.php). I still get errors like
PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '187860-0--0' for key 'PRIMARY': UPDATE {poll_vote} SET uid=:db_update_placeholder_0 WHERE (uid = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => 0 [:db_condition_placeholder_0] => 38025
(before applying the patch duplicate entries were like '176593-0-' - with only 1 zero)
I suppose it's because my site was upgraded from Drupal 6. In D6' poll module there was no 'timestamp' field in {poll_votes} table, so after upgrade to D7 timestamp fields of old poll votes got value '0'.

My solution of this issue: I changed PRIMARY to INDEX in {poll_vote}.

dcam’s picture

Status: Needs review » Needs work

Thanks for your hard work, @secretsayan! Unfortunately, there are at least four problems with #22.

1. The first, minor issue is that there is a space in the "blank" line 217. That's what is creating the whitespace issue when applying the patch.

2. The function needs to be placed before the

/**
 * @} End of "addtogroup updates-7.x-extra".
 */

since it is one of the 7.x "extra" updates.

3. The timestamp field also needs to be added to the indexes array of $schema['poll_vote'] in poll_schema(). Without it, the timestamp field won't be added to the primary index on install.

4. The most serious problem is that this patch won't solve the issue entirely. The problem will still exist if two deleted users happen to have voted in a poll at the same second. While this is kind of unlikely, #36 reports this exact problem with a site that has been upgraded from 6.x that has lots of timestamps = 0, which is where we're more likely to see this issue to continue popping up.

So, we need another solution.

dcam’s picture

dcam’s picture

This could use some input from @David_Rothstein. It seems like we might have to add a unique ID column to fix this issue. As mentioned in #37, even extending the Primary index to all four of the table's columns won't solve the issue.

David_Rothstein’s picture

Issue tags: +Needs tests

Hm, we could remove that primary key entirely (and replace it with an index on the same columns) but then we're violating the assumption that the module makes elsewhere that the hostname can be used to define an anonymous user's vote. For example, poll_cancel_form() (with submit handler poll_cancel()) is apparently available to anonymous users and when submitted will delete all anonymous votes from the hostname; it is essentially assuming there only can be one. If there ever is more than one, this would delete votes it shouldn't.

It's a tough call; maybe to fix this we should just remove the code in poll_user_cancel() that tries to anonymize the votes in the first place? It's not an ideal solution but it's not like the user IDs mean much anyway once the account has been deleted, so perhaps they can just stay in the database table (although there could be some privacy concerns with that?). I am also not sure what else might break if we did that.

Tests sound like a good idea here too.

dcam’s picture

It's a tough call; maybe to fix this we should just remove the code in poll_user_cancel() that tries to anonymize the votes in the first place? It's not an ideal solution but it's not like the user IDs mean much anyway once the account has been deleted, so perhaps they can just stay in the database table (although there could be some privacy concerns with that?). I am also not sure what else might break if we did that.

This sounds like a good place to start, if someone wants to roll a new patch. My guess is that some test will blow up if the anonymizing is removed, most likely something to do with node/%nid/votes. Maybe not though.

secretsayan’s picture

Issue tags: +SprintWeekend2016
secretsayan’s picture

Ok then....I have created a patch...I have removed the poll_user_cancel function. This works fine.
Another option can be to update the hostname field after appending with the uid before setting it to '0'. But then any reverse proxy lookup would fail.

Please check this out and suggest.

GlenRanson’s picture

Is this patch going to be applied anytime soon?

It works for me

secretsayan’s picture

Status: Needs work » Needs review
vpanicke’s picture

Status: Needs review » Fixed
Issue tags: -Needs tests, -SprintWeekend2016
dcam’s picture

Status: Fixed » Needs review

Why was this marked as being fixed? No patch has been applied. If you're going to change the status like that, at least explain why.

dcam’s picture

Issue tags: +Needs tests

Re-added the Needs Tests tag because none have been written yet.

secretsayan’s picture

Assigned: secretsayan » Unassigned
andriyun’s picture

Status: Needs review » Needs work

The last submitted patch, 50: pollusercancel-2268673-50.patch, failed testing. View results

scott_euser’s picture

@andriyun I don't think this is the solution. There may be code referencing the user ID so leaving it as is with the user ID referencing a user that no longer exists could have knock-on effects.

I think the better solution is enforcing unique poll submissions with poll form validation but not requiring unique at the database table level (ie, altering the table structure).

danyg’s picture

I've fixed the solution suggested in patch #22 what changes the database's primary key to use all 4 fields to determine the unique vote at all.

Update: If you are still facing errors on the mass update as I have done then you probably migrated your site from D6 to D7 after the poll module had introduced the timestamp column in poll_vote table. In this case, the timestamp field has been filled with the creation date of the poll node and equals in all related records. So, if you are in this case, this snippet will help. It adds 100 seconds to all equal timestamps and helps move on the user cancel issue.

<?php
ini_set('max_execution_time', 300);
$result = db_query("SELECT nid, pv.timestamp FROM {poll_vote} pv GROUP BY nid, pv.`timestamp` HAVING count(`timestamp`) > 2;")->fetchAll();
if (!empty($result)) {
$polls = $votes = 0;
  foreach ($result as $item) {
    $timestamp = $item->timestamp;
    $records = db_query("SELECT * FROM {poll_vote} pv WHERE nid = :nid AND pv.timestamp = :timestamp", array(':nid' => $item->nid, ':timestamp' => $item->timestamp))->fetchAll();
    $offset = 0;
    foreach ($records as $record) {
      db_update('poll_vote')->expression('timestamp', 'timestamp + :offset', array(':offset' => $offset))->condition('chid', $record->chid)->condition('nid', $record->nid)->condition('uid', $record->uid)->condition('hostname', $record->hostname)->condition('timestamp', $record->timestamp)->execute();
      $votes++;
      $offset += 100;
    }
   $polls++;
  }
drupal_set_message(t('%poll polls and %vote votes have been updated.', array('%poll' => $polls, '%vote' => $votes)));
}

?>
izmeez’s picture

There seem to be two different patches in this queue. One is reflected in comment #50 where hook_user_cancel() is removed and the second is in comment #53 that follows up from the patch in #22 to Add the timestamp field to the primary key.

Do both patches need to be combined into one patch? How do each or both of them relate to the issue summary and the steps to reproduce described in comment #9 ?

danyg’s picture

@izmeez: In my opinion and according to @scott_euser disabling the implementation of hook_user_cancel is not the right way. It simply turns the method off and ignores to work with the records which should be changed. The patch in comment #50 causes orphan records among the records of the poll_vote table.
So, I suggest to use the patch in comment #53.