Add MySQL FULLTEXT support. #862

Merged
merged 1 commit into from Sep 21, 2012

Projects

None yet

4 participants

@bar bar Add MySQL FULLTEXT support.
Minor optimizations and testing added.
36c99a3
Contributor
bar commented Sep 20, 2012

IMO, some functions from lib/Cake/Model/Datasource/DboSource.php should be finally moved to lib/Cake/Model/Datasource/Database/Mysql.php and some abstract/stub functions should be created in their place.

To cite an example, DboSource::buildIndex() is only used by MySQL, every other driver has its own buildIndex(). Also, DboSource::buildIndex() signature has $table as second argument, which is not used anywhere.

Owner

This change looks good to me. I agree that the MySQLisms shouldn't be in the base class too. Perhaps a separate PR for that?

Contributor
bar commented Sep 20, 2012

Yes, a different PR should be cleaner :)

@lorenzo lorenzo commented on the diff Sep 20, 2012
lib/Cake/Model/Datasource/Database/Mysql.php
@@ -422,17 +422,21 @@ public function index($model) {
$table = $this->fullTableName($model);
$old = version_compare($this->getVersion(), '4.1', '<=');
if ($table) {
- $indices = $this->_execute('SHOW INDEX FROM ' . $table);
bar
bar Sep 20, 2012 Contributor

Me gustaría que esté en castellano haha... but because of consistency with every other $indexesname :P

lorenzo
lorenzo Sep 20, 2012 Owner

In English both words mean the same, just different spelling

bar
bar Sep 20, 2012 Contributor

Yes, it was a bad joke... I'm from Argentina :')
What I ment, was the variable name to be consistent with all the other $indexes inside lib/Cake/Model/Datasource/Database/Mysql.php, pure cosmetic surgery.

lorenzo
lorenzo Sep 20, 2012 Owner

That's ok :)

Owner
lorenzo commented Sep 20, 2012

Looks good to me

@markstory markstory merged commit f4a639c into cakephp:master Sep 21, 2012
Owner

I totally missed that this PR was targeted towards master. I'll be reverting the changes and re-merging onto 2.3.

@markstory markstory added a commit that referenced this pull request Sep 21, 2012
@bar @markstory bar + markstory Add MySQL FULLTEXT support.
Minor optimizations and testing added.
Merge pull request #862 from bar/mysql-fulltext

Fixes #262
aaefbf1
Owner

Cherry picked onto 2.3 in [aaefbf1]

Contributor
bar commented Sep 21, 2012

Sure @markstory, BTW it should not break backward compatibility.

Member
ADmad commented Sep 21, 2012

@bar Probably not, but its a new feature and those go to next minor release. Master branch is used for point releases which only has bugfixes.

Contributor
bar commented Sep 21, 2012

Thanks for the info @markstory !
I think, it should be nice to have that exact phrase in the "how to contribute" GIT page [1] , so we (developers) don't mess with the code the wrong way ;)

[1] https://github.com/cakephp/docs

Owner

@bar I've tried to clarify the docs in cakephp/docs@9b0af41.

Contributor
bar commented Sep 21, 2012

@markstory you rock!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment