Initial conversion was performed using the deadwood project, with some manual cleanup. The entire code base needs to be audited, and functionality needs to be broken out into smaller include files where applicable.

CommentFileSizeAuthor
#6 ad_click_filter_replace_db_num_rows_2.patch1.36 KBjrglasgow
#3 ad_click_filter_replace_db_num_rows.patch1.71 KBjrglasgow
#1 click_filter-294037-1.patch1.38 KBmehmeta

Comments

mehmeta’s picture

mehmeta commented
Status: Active » Needs review
StatusFileSize
new click_filter-294037-1.patch1.38 KB

Hi, I went through the Coder module and changed db_num_rows() s to db_fetch_object()s, and != to <> in an sql query, pretty simple. Reinstalled the module, seems to go through the existing clicks and update them as intended. This is a patch for the install file, module file itself seems to work just fine. This is my first patch, so please do not hesitate to point out if there's anything I'm doing wrong. Thanks.

jeremy’s picture

jeremy commented
Status: Needs review » Fixed

Thanks for the patch! Committed.

jrglasgow’s picture

jrglasgow commented
StatusFileSize
new ad_click_filter_replace_db_num_rows.patch1.71 KB

in #1 he was referring to http://drupal.org/node/114774#db-num-rows , There were a couple of instances of db_num_rows() in the click_filter.mode as well, this patch will take care of that,

jeremy’s picture

jeremy commented
Status: Fixed » Needs review

Re-opening issue to review second patch.

jeremy’s picture

jeremy commented
Status: Needs review » Needs work

"SELECT COUNT(*)" is a performance hit on InnoDB and PostgreSQL. The patch should only replace the call to db_num_rows().

jrglasgow’s picture

jrglasgow commented
Status: Needs work » Needs review
StatusFileSize
new ad_click_filter_replace_db_num_rows_2.patch1.36 KB

I was unaware of the performance hit when using "COUNT(*)", here is the revised patch

jeremy’s picture

jeremy commented
Status: Needs review » Fixed

Thanks, patch committed.

Status: Fixed » Closed (fixed)

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