db_delete('term_relation')->condition('tid1', $term->tid, 'OR')->condition('tid2', $term->tid)->execute() creates this kind of SQL:
DELETE FROM {term_relation} WHERE (tid1 OR :db_condition_placeholder_1331) AND (tid2 = :db_condition_placeholder_1332)

'tid1 OR :db_condition_placeholder_1331' is wrong and I'm not sure as to why this passes in MySQL but the 'OR' should be '='

This query causes many exceptions when running simpletests on a postgres database.

attached is patch which makes these tests pass.

2 in particular blogapi and taxonomy (obviously)

CommentFileSizeAuthor
#9 taxonomy_term_save.patch1.98 KBcatch
taxonomy.module.patch499 bytesjosh waihi

Comments

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
commented

This patch looks good, and all the taxonomy and Blog API tests pass, with no exceptions, after applying the patch.

Pre-patch:

Taxonomy tests: 224 passes, 14 fails, and 60 exceptions
Blog API tests: Site breaks after running the tests, with this error:

Fatal error: Call to undefined function blog_help() in /home/jonathan/svn/drupal/drupal_cvs_head/includes/menu.inc on line 1226

Presumably because the cleanup can't happen due to exceptions?

with patch applied:

Taxonomy tests: 283 passes, 0 fails, and 0 exceptions
Blog API tests: 52 passes, 0 fails, and 0 exceptions

serenecloud’s picture

serenecloud commented

Applied patch to latest CVS HEAD, it looks good. (PostgreSQL 8.1)

Pre patch I got an uncaught exception, Post patch I get "335 passes, 0 fails, and 0 exceptions"

kaotien’s picture

kaotien commented

Fresh Drupal 7 install on MySQL

Pre-Patch: No fails or exceptions

--

Post-Patch: No fails or exceptions

Full results
Blog API - 52 passes, 0 fails, and 0 exceptions
Taxonomy - 283 passes, 0 fails, and 0 exceptions

serenecloud’s picture

serenecloud commented
Status: Needs review » Reviewed & tested by the community

Works in PostgreSQL and MySQL,

I think we can put this as reviewed + tested by the community.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work

I'm actually surprised this is working on MySQL. The function declaration of condition() is:
public function condition($field, $value = NULL, $operator = '=') {
Clearly, $operator is never meant to be something like 'OR'.

It looks like we need to have a test for this because it wasn't even tested on MySQL to begin with.

catch’s picture

catch
he/him
Primary language English
commented

So the 'OR' in there was my fault - bad dbtng conversion which slipped in with one of the taxonomy patches, mea culpa.

Looks like we're missing a test for deleting terms which have related terms - ought to be easy enough to add to the existing tests. Will roll a patch with that included in the next day or so if no-one beats me to it.

josh waihi’s picture

josh waihi commented

@Dries what work needs to done on the patch to get it applied?

catch’s picture

catch
he/him
Primary language English
commented

@mayer - we need to add to taxonomy.test so it explicitly tests deleting a term which has related terms associated term relationship.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new taxonomy_term_save.patch1.98 KB

Here it is with a test, apparently the delete just failed silently on MySQL without throwing an exception - so I added an explicit check that old relationships are removed when new ones are saved. Has one fail without the changes to taxonomy_term_save().

damien tournoud’s picture

damien tournoud commented
Title: taxonomy_term_save fails under postgres » Wrong query in taxonomy_term_save()

This is not PostgreSQL specific.

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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