From 274c83c74d128fc2e5a63c7c178250c862f90182 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Sun, 12 Aug 2012 01:25:01 +0530 Subject: [PATCH 01/11] [ticket/11040] Search subject and text together Instead of searching for post_subject and post_text seperately and then OR them in where clause we use concatenation of the two columns and search PHPBB3-11040 --- phpBB/phpbb/search/fulltext_postgres.php | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/phpBB/phpbb/search/fulltext_postgres.php b/phpBB/phpbb/search/fulltext_postgres.php index 063bf52a19..7df61ee583 100644 --- a/phpBB/phpbb/search/fulltext_postgres.php +++ b/phpBB/phpbb/search/fulltext_postgres.php @@ -457,15 +457,13 @@ class fulltext_postgres extends \phpbb\search\base $sql_where_options .= $sql_match_where; $tmp_sql_match = array(); - foreach (explode(',', $sql_match) as $sql_match_column) - { - $tmp_sql_match[] = "to_tsvector ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', " . $sql_match_column . ") @@ to_tsquery ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', '" . $this->db->sql_escape($this->tsearch_query) . "')"; - } + $sql_match = str_replace(',', " || ' ' ||", $sql_match); + $tmp_sql_match = "to_tsvector ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', " . $sql_match . ") @@ to_tsquery ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', '" . $this->db->sql_escape($this->tsearch_query) . "')"; $this->db->sql_transaction('begin'); $sql_from = "FROM $sql_from$sql_sort_table" . POSTS_TABLE . " p"; - $sql_where = "WHERE (" . implode(' OR ', $tmp_sql_match) . ") + $sql_where = "WHERE (" . $tmp_sql_match . ") $sql_where_options"; $sql = "SELECT $sql_select $sql_from From 87688687e4eaa274ccfec10c43b6b91d050d05e5 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Sun, 12 Aug 2012 01:40:30 +0530 Subject: [PATCH 02/11] [ticket/11040] Add post_content index In addition to post_subject and post_title indexes add a post_content which is concatenation of both the columns. PHPBB3-11040 --- phpBB/phpbb/search/fulltext_postgres.php | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/phpBB/phpbb/search/fulltext_postgres.php b/phpBB/phpbb/search/fulltext_postgres.php index 7df61ee583..32519b1f14 100644 --- a/phpBB/phpbb/search/fulltext_postgres.php +++ b/phpBB/phpbb/search/fulltext_postgres.php @@ -796,6 +796,11 @@ class fulltext_postgres extends \phpbb\search\base $this->db->sql_query("CREATE INDEX " . POSTS_TABLE . "_" . $this->config['fulltext_postgres_ts_name'] . "_post_text ON " . POSTS_TABLE . " USING gin (to_tsvector ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', post_text))"); } + if (!isset($this->stats['post_content'])) + { + $this->db->sql_query("CREATE INDEX " . POSTS_TABLE . "_" . $this->config['fulltext_postgres_ts_name'] . "_post_content ON " . POSTS_TABLE . " USING gin (to_tsvector ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', post_subject || ' ' || post_text))"); + } + $this->db->sql_query('TRUNCATE TABLE ' . SEARCH_RESULTS_TABLE); return false; @@ -829,6 +834,11 @@ class fulltext_postgres extends \phpbb\search\base $this->db->sql_query('DROP INDEX ' . $this->stats['post_text']['relname']); } + if (isset($this->stats['post_content'])) + { + $this->db->sql_query('DROP INDEX ' . $this->stats['post_content']['relname']); + } + $this->db->sql_query('TRUNCATE TABLE ' . SEARCH_RESULTS_TABLE); return false; @@ -844,7 +854,7 @@ class fulltext_postgres extends \phpbb\search\base $this->get_stats(); } - return (isset($this->stats['post_text']) && isset($this->stats['post_subject'])) ? true : false; + return (isset($this->stats['post_text']) && isset($this->stats['post_subject']) && isset($this->stats['post_content'])) ? true : false; } /** @@ -894,6 +904,10 @@ class fulltext_postgres extends \phpbb\search\base { $this->stats['post_subject'] = $row; } + else if ($row['relname'] == POSTS_TABLE . '_' . $this->config['fulltext_postgres_ts_name'] . '_post_content' || $row['relname'] == POSTS_TABLE . '_post_content') + { + $this->stats['post_content'] = $row; + } } } $this->db->sql_freeresult($result); From 101ceed80c4ba3b24aaf92d1e72529c086237888 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Mon, 26 Aug 2013 15:09:01 +0530 Subject: [PATCH 03/11] [ticket/11040] Remove postgres extra indexes Remove post_text index as post_content index is sufficient to search post text. PHPBB3-11040 --- phpBB/phpbb/search/fulltext_postgres.php | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/phpBB/phpbb/search/fulltext_postgres.php b/phpBB/phpbb/search/fulltext_postgres.php index 32519b1f14..cb50ce69f0 100644 --- a/phpBB/phpbb/search/fulltext_postgres.php +++ b/phpBB/phpbb/search/fulltext_postgres.php @@ -791,11 +791,6 @@ class fulltext_postgres extends \phpbb\search\base $this->db->sql_query("CREATE INDEX " . POSTS_TABLE . "_" . $this->config['fulltext_postgres_ts_name'] . "_post_subject ON " . POSTS_TABLE . " USING gin (to_tsvector ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', post_subject))"); } - if (!isset($this->stats['post_text'])) - { - $this->db->sql_query("CREATE INDEX " . POSTS_TABLE . "_" . $this->config['fulltext_postgres_ts_name'] . "_post_text ON " . POSTS_TABLE . " USING gin (to_tsvector ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', post_text))"); - } - if (!isset($this->stats['post_content'])) { $this->db->sql_query("CREATE INDEX " . POSTS_TABLE . "_" . $this->config['fulltext_postgres_ts_name'] . "_post_content ON " . POSTS_TABLE . " USING gin (to_tsvector ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', post_subject || ' ' || post_text))"); @@ -829,11 +824,6 @@ class fulltext_postgres extends \phpbb\search\base $this->db->sql_query('DROP INDEX ' . $this->stats['post_subject']['relname']); } - if (isset($this->stats['post_text'])) - { - $this->db->sql_query('DROP INDEX ' . $this->stats['post_text']['relname']); - } - if (isset($this->stats['post_content'])) { $this->db->sql_query('DROP INDEX ' . $this->stats['post_content']['relname']); @@ -854,7 +844,7 @@ class fulltext_postgres extends \phpbb\search\base $this->get_stats(); } - return (isset($this->stats['post_text']) && isset($this->stats['post_subject']) && isset($this->stats['post_content'])) ? true : false; + return (isset($this->stats['post_subject']) && isset($this->stats['post_content'])) ? true : false; } /** @@ -896,11 +886,7 @@ class fulltext_postgres extends \phpbb\search\base // deal with older PostgreSQL versions which didn't use Index_type if (strpos($row['indexdef'], 'to_tsvector') !== false) { - if ($row['relname'] == POSTS_TABLE . '_' . $this->config['fulltext_postgres_ts_name'] . '_post_text' || $row['relname'] == POSTS_TABLE . '_post_text') - { - $this->stats['post_text'] = $row; - } - else if ($row['relname'] == POSTS_TABLE . '_' . $this->config['fulltext_postgres_ts_name'] . '_post_subject' || $row['relname'] == POSTS_TABLE . '_post_subject') + if ($row['relname'] == POSTS_TABLE . '_' . $this->config['fulltext_postgres_ts_name'] . '_post_subject' || $row['relname'] == POSTS_TABLE . '_post_subject') { $this->stats['post_subject'] = $row; } From 1094a2b577b79c7c456ce34d42fb5ce345297a41 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Fri, 8 Nov 2013 18:35:49 +0530 Subject: [PATCH 04/11] [ticket/11040] Add test cases for searching subject and post content together PHPBB3-11040 --- tests/functional/search/base.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index 28327da914..cec06e6a5c 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -12,11 +12,11 @@ */ abstract class phpbb_functional_search_base extends phpbb_functional_test_case { - protected function assert_search_found($keywords) + protected function assert_search_found($keywords, $posts_found, $words_highlighted) { $crawler = self::request('GET', 'search.php?keywords=' . $keywords); - $this->assertEquals(1, $crawler->filter('.postbody')->count()); - $this->assertEquals(3, $crawler->filter('.posthilit')->count()); + $this->assertEquals($posts_found, $crawler->filter('.postbody')->count()); + $this->assertEquals($words_highlighted, $crawler->filter('.posthilit')->count()); } protected function assert_search_not_found($keywords) @@ -32,6 +32,8 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $this->login(); $this->admin_login(); + $post = $this->create_topic(2, 'Test Topic 1 Subject', 'This is a test topic posted by the testing framework.'); + $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=settings&sid=' . $this->sid); $form = $crawler->selectButton('Submit')->form(); $values = $form->getValues(); @@ -55,7 +57,8 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case } $this->logout(); - $this->assert_search_found('phpbb3+installation'); + $this->assert_search_found('phpbb3+installation', 1, 3); + $this->assert_search_found('subject+framework', 1, 2); $this->assert_search_not_found('loremipsumdedo'); $this->login(); From 932e0eb17e9d1869eb4b1e76100c6848823780d0 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Fri, 8 Nov 2013 18:56:36 +0530 Subject: [PATCH 05/11] [ticket/11040] Swap post_text and post_subject for post_content index Provides better performance as per PostgreSQL Docs. PHPBB3-11040 --- phpBB/phpbb/search/fulltext_postgres.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpBB/phpbb/search/fulltext_postgres.php b/phpBB/phpbb/search/fulltext_postgres.php index cb50ce69f0..63caeffcc5 100644 --- a/phpBB/phpbb/search/fulltext_postgres.php +++ b/phpBB/phpbb/search/fulltext_postgres.php @@ -793,7 +793,7 @@ class fulltext_postgres extends \phpbb\search\base if (!isset($this->stats['post_content'])) { - $this->db->sql_query("CREATE INDEX " . POSTS_TABLE . "_" . $this->config['fulltext_postgres_ts_name'] . "_post_content ON " . POSTS_TABLE . " USING gin (to_tsvector ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', post_subject || ' ' || post_text))"); + $this->db->sql_query("CREATE INDEX " . POSTS_TABLE . "_" . $this->config['fulltext_postgres_ts_name'] . "_post_content ON " . POSTS_TABLE . " USING gin (to_tsvector ('" . $this->db->sql_escape($this->config['fulltext_postgres_ts_name']) . "', post_text || ' ' || post_subject))"); } $this->db->sql_query('TRUNCATE TABLE ' . SEARCH_RESULTS_TABLE); From b5310e95652f009e98d720f0db92fd1cf2551fa8 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Fri, 8 Nov 2013 23:11:05 +0530 Subject: [PATCH 06/11] [ticket/11040] Add methods to delete post and topic in functional tests PHPBB3-11040 --- .../phpbb_functional_test_case.php | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index e40efdec03..85ff47a1f8 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -981,6 +981,50 @@ class phpbb_functional_test_case extends phpbb_test_case ); } + /** + * Deletes a topic + * + * Be sure to login before creating + * + * @param int $topic_id + * @return null + */ + public function delete_topic($topic_id) + { + $crawler = self::request('GET', "viewtopic.php?t={$topic_id}&sid={$this->sid}"); + + $this->add_lang('posting'); + $form = $crawler->selectButton('Go')->eq(1)->form(); + $form['action']->select('delete_topic'); + $crawler = self::submit($form); + $this->assertContainsLang('DELETE_PERMANENTLY', $crawler->text()); + + $this->add_lang('mcp'); + $form = $crawler->selectButton('Yes')->form(); + $crawler = self::submit($form); + $this->assertContainsLang('TOPIC_DELETED_SUCCESS', $crawler->text()); + } + + /** + * Deletes a post + * + * Be sure to login before creating + * + * @param int $forum_id + * @param int $topic_id + * @return null + */ + public function delete_post($forum_id, $post_id) + { + $this->add_lang('posting'); + $crawler = self::request('GET', "posting.php?mode=delete&f={$forum_id}&p={$post_id}&sid={$this->sid}"); + $this->assertContainsLang('DELETE_PERMANENTLY', $crawler->text()); + + $form = $crawler->selectButton('Yes')->form(); + $crawler = self::submit($form); + $this->assertContainsLang('POST_DELETED', $crawler->text()); + } + /** * Returns the requested parameter from a URL * From 6524513f9d03f6afa42852b6efa3840ee8fda3e7 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Fri, 8 Nov 2013 23:12:06 +0530 Subject: [PATCH 07/11] [ticket/11040] Delete the functional test topic to avoid conflicts PHPBB3-11040 --- tests/functional/search/base.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index cec06e6a5c..979b7ed6bd 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -64,6 +64,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $this->login(); $this->admin_login(); $this->delete_search_index(); + $this->delete_topic($post['topic_id']); } protected function create_search_index() From 556c7d9218530721fe258446b1d80b555604ff89 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Fri, 8 Nov 2013 23:16:20 +0530 Subject: [PATCH 08/11] [ticket/11040] Add migration to drop postgres search indexes PHPBB3-11040 --- .../data/v310/postgres_fulltext_drop.php | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 phpBB/phpbb/db/migration/data/v310/postgres_fulltext_drop.php diff --git a/phpBB/phpbb/db/migration/data/v310/postgres_fulltext_drop.php b/phpBB/phpbb/db/migration/data/v310/postgres_fulltext_drop.php new file mode 100644 index 0000000000..7d4aea3c95 --- /dev/null +++ b/phpBB/phpbb/db/migration/data/v310/postgres_fulltext_drop.php @@ -0,0 +1,47 @@ +db->sql_layer, 'postgres') === false; + } + + static public function depends_on() + { + return array( + '\phpbb\db\migration\data\v310\dev', + ); + } + + public function update_schema() + { + /* + * Drop FULLTEXT indexes related to PostgreSQL fulltext search. + * Doing so is equivalent to dropping the search index from the ACP. + * Possibly time-consuming recreation of the search index (i.e. + * FULLTEXT indexes) is left as a task to the admin to not + * unnecessarily stall the upgrade process. The new search index will + * then require about 40% less table space (also see PHPBB3-11040). + */ + return array( + 'drop_keys' => array( + $this->table_prefix . 'posts' => array( + 'post_subject', + 'post_text', + 'post_content', + ), + ), + ); + } +} From 799c5759906245ee930d375ca58c041246f7f348 Mon Sep 17 00:00:00 2001 From: Dhruv Date: Sun, 9 Mar 2014 23:26:08 +0530 Subject: [PATCH 09/11] [ticket/11040] Use hard delete in delete_topic PHPBB3-11040 --- tests/test_framework/phpbb_functional_test_case.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_framework/phpbb_functional_test_case.php b/tests/test_framework/phpbb_functional_test_case.php index 85ff47a1f8..937192c58f 100644 --- a/tests/test_framework/phpbb_functional_test_case.php +++ b/tests/test_framework/phpbb_functional_test_case.php @@ -1001,6 +1001,7 @@ class phpbb_functional_test_case extends phpbb_test_case $this->add_lang('mcp'); $form = $crawler->selectButton('Yes')->form(); + $form['delete_permanent'] = 1; $crawler = self::submit($form); $this->assertContainsLang('TOPIC_DELETED_SUCCESS', $crawler->text()); } @@ -1021,6 +1022,7 @@ class phpbb_functional_test_case extends phpbb_test_case $this->assertContainsLang('DELETE_PERMANENTLY', $crawler->text()); $form = $crawler->selectButton('Yes')->form(); + $form['delete_permanent'] = 1; $crawler = self::submit($form); $this->assertContainsLang('POST_DELETED', $crawler->text()); } From 3d1d25a122399bb218451cc4ed63ac0f2d2a02bb Mon Sep 17 00:00:00 2001 From: Dhruv Date: Mon, 10 Mar 2014 01:02:57 +0530 Subject: [PATCH 10/11] [ticket/11040] Use unique text for the test post added PHPBB3-11040 --- tests/functional/search/base.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index 979b7ed6bd..d377181f4a 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -32,7 +32,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $this->login(); $this->admin_login(); - $post = $this->create_topic(2, 'Test Topic 1 Subject', 'This is a test topic posted by the testing framework.'); + $post = $this->create_topic(2, 'Test Topic 1 foosubject', 'This is a test topic posted by the barsearch testing framework.'); $crawler = self::request('GET', 'adm/index.php?i=acp_search&mode=settings&sid=' . $this->sid); $form = $crawler->selectButton('Submit')->form(); @@ -58,7 +58,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case $this->logout(); $this->assert_search_found('phpbb3+installation', 1, 3); - $this->assert_search_found('subject+framework', 1, 2); + $this->assert_search_found('foosubject+barsearch', 1, 2); $this->assert_search_not_found('loremipsumdedo'); $this->login(); From 6918921d53da75bf1b77dcbffff10e8dc574fbdb Mon Sep 17 00:00:00 2001 From: Dhruv Date: Mon, 10 Mar 2014 02:20:29 +0530 Subject: [PATCH 11/11] [ticket/11040] Topic is deleted if test is skipped PHPBB3-11040 --- tests/functional/search/base.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/search/base.php b/tests/functional/search/base.php index d377181f4a..0bd9bf01ab 100644 --- a/tests/functional/search/base.php +++ b/tests/functional/search/base.php @@ -51,6 +51,7 @@ abstract class phpbb_functional_search_base extends phpbb_functional_test_case // check if search backend is not supported if ($crawler->filter('.errorbox')->count() > 0) { + $this->delete_topic($post['topic_id']); $this->markTestSkipped("Search backend is not supported/running"); } $this->create_search_index();