In the _tracker_remove() we can find the next lines:
$connection->update('tracker_node')
->fields([
'changed' => $changed,
'published' => $node->isPublished(),
])
->condition('nid', $nid)
->execute();
If $node->isPublished() returns FALSE we can see the next error:
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[22007]:
Invalid datetime format: 1366 Incorrect integer value: '' for column `tracker_node`.`published` at row 1:
UPDATE {tracker_node} SET changed=:db_update_placeholder_0, published=:db_update_placeholder_1 WHERE nid = :db_condition_placeholder_0; Array ( [:db_update_placeholder_0] => _TIMESTAMP_ [:db_update_placeholder_1] => [:db_condition_placeholder_0] => _NID_ ) in _tracker_remove()
(line 377 of /core/modules/tracker/tracker.module).
Simmilar issue was fixed by addign (int) and commited here 2887490
but not all the 'published' => $node->isPublished(), entries were fixed.
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | Selection_999(457).png | 108.75 KB | gilmord |
| #5 | interdiff-2-5.txt | 691 bytes | hardik_patel_12 |
| #5 | 3119131-5.patch | 771 bytes | hardik_patel_12 |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes, plain diff MR !2623
Comments
Comment #2
gilmordComment #3
gilmordPlease review the patch.
Comment #4
gilmordComment #5
hardik_patel_12 commentedPatch at #2 looks good to me , but there is also duplicate code for updating tracker_node table , removing duplicate code also kindly review the patch.
Comment #8
quietone commentedIs this reproducible from the UI? If, so please add step to reproduce.
Comment #12
larowlanWhich database engines does this occur on?
Comment #13
gilmord@larowlan InnoDB
Comment #14
gilmordNot sure why this issue stuck, tracker_cron uses the exact same approach for a long time:
@larowlan I think we can rely on this and use the same fix in the _tracker_remove without digging into database engines etc.
Any thoughts on this?
Comment #15
larowlanYep, that will be fine
We need a test here
Comment #17
gilmordHi @larowlan
I do not think there is a good way to cover this with tests
_tracker_remove() is already covered by tests, and it does not fail
as mentioned before - conditions may be specific, like database engine
Comment #19
gilmordHi again, according to previous comment:
- we can`t cover with tests this environment-specific case (this functionality is already covered by tests, but it can not catch this type of issue)
- we have exactly the same fix for the similar code in the same module
Prepared MR (also includes the fix for the issue mentioned by @Hardik_Patel_12 about duplicated update)
Can we push on it to get merged?
Comment #21
smustgrave commentedFor the Issue summary update.
Comment #22
quietone commentedThis extension is deprecated and scheduled for removal in Drupal 11.
This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.
It will be moved to the contributed extension once the Drupal 11 branch is open.
Comment #24
andypost