optimize taxonomy_check_vocabulary_hierarchy() [#1925638]

taxonomy_check_vocabulary_hierarchy() is horribly inefficient as it calls taxonomy_get_tree() in order to check the parents.

Also, it seems that the only reason $changed_term is needed is because of the static caching in taxonomy_get_tree().

The job taxonomy_check_vocabulary_hierarchy() could be better solved by a couple of queries, which would also remove the need for $changed_term.

Patch coming up...

Comment	File	Size	Author
#15	taxonomy_check_vocabulary_hierarchy-1925638-15.patch	3.66 KB	p-neyens
#15	7.x: PHP 7 & MySQL 5.5 2,035 pass PHP 5.6 & MySQL 5.5 2,035 pass
#4	1925638-4-taxonomy_check_vocabulary_hierarchy.patch.patch	3.33 KB	alchemist_ua
#4
#1	1925638-1-taxonomy_check_vocabulary_hierarchy.patch	4.85 KB	gielfeldt
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

gielfeldt CreditAttribution: gielfeldt commented 23 February 2013 at 21:37

Status:

Active

» Needs review

File	Size
1925638-1-taxonomy_check_vocabulary_hierarchy.patch	4.85 KB

Comment #2

c960657 CreditAttribution: c960657 commented 5 March 2013 at 08:40

This is a great improvement.

+    ->condition('h.parent', 0, '>')

I wonder if it is better/more efficient to use '<>' here?

A few style issues:

The naming of $has_parents and $multiple_parents is inconsistent wrt the has_ prefix.

First the variables $has_parents and $multiple_parents contain a query object, but then they are overwritten by the result of the query. Reusing variables in this way is not good for readability.

Comment #3

gielfeldt CreditAttribution: gielfeldt commented 5 March 2013 at 15:05

Status:

Needs review

» Needs work

I wonder if it is better/more efficient to use '<>' here?

Can't say if it makes a difference, but I guess it can't hurt to be as explicit as possible to the db-engine.

The naming of $has_parents and $multiple_parents is inconsistent wrt the has_ prefix.

First the variables $has_parents and $multiple_parents contain a query object, but then they are overwritten by the result of the query. Reusing variables in this way is not good for readability.

Of course. A bit sloppy :-). I'll fix.

Comment #4

alchemist_ua CreditAttribution: alchemist_ua commented 11 April 2013 at 09:12

Version:

8.x-dev

» 6.x-dev

File	Size
1925638-4-taxonomy_check_vocabulary_hierarchy.patch.patch	3.33 KB

Here it is the backport of this to drupal 6.x.

Comment #5

alchemist_ua CreditAttribution: alchemist_ua commented 11 April 2013 at 09:48

Status:

Needs work

» Needs review

Comment #6

alchemist_ua CreditAttribution: alchemist_ua commented 11 April 2013 at 12:12

Version:	6.x-dev	» 8.x-dev
Status:	Needs review	» Needs work

Comment #7

klonos

he/him

English

90% Melbourne, Australia - 10% Larissa, Greece

CreditAttribution: klonos commented 31 May 2013 at 07:31

Issue tags:

+Needs backport to D6

...coming from #353775-59: Vocabulary edit form resets the 'hierarchy' (D7) field to '0' and 'relations' field (D6) to '1'

Does this qualify for a backport to D7 too besides D6?

Comment #8

gielfeldt CreditAttribution: gielfeldt commented 31 May 2013 at 16:23

Yes, this issue is also relevant for D7.

Comment #9

klonos

he/him

English

90% Melbourne, Australia - 10% Larissa, Greece

CreditAttribution: klonos commented 2 June 2013 at 02:36

Issue tags:

+Needs backport to D7

...thought so ;)

Comment #10

gielfeldt CreditAttribution: gielfeldt commented 2 July 2013 at 14:32

Anybody interested in fixing the point mentioned in #3 ?

Comment #11

2 July 2013 at 14:32

Version:

8.0.x-dev

» 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #12

2 July 2013 at 14:32

Version:

8.1.x-dev

» 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #13

2 July 2013 at 14:32

Version:

8.2.x-dev

» 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Comment #14

2 July 2013 at 14:32

Version:

8.3.x-dev

» 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.