Skip to content
This repository was archived by the owner on Jan 28, 2021. It is now read-only.

sql: function, added soundex #486

Merged
merged 3 commits into from
Oct 30, 2018
Merged

sql: function, added soundex #486

merged 3 commits into from
Oct 30, 2018

Conversation

bake
Copy link
Contributor

@bake bake commented Oct 21, 2018

This PR adds a soundex function as described in the MySQL Reference and implemented in mf_soundex.c in mysql/mysql-server.

@erizocosmico erizocosmico requested a review from a team October 22, 2018 07:44
if b.Len() == 0 {
return "", nil
}
for i := len([]rune(b.String())); i < 4; i++ {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 4 (because type rune = int32)?

Copy link
Contributor Author

@bake bake Oct 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though the reference doesn't say so, MySQL returns at least four characters, if one of them is a letter:

mysql> select soundex('');
+-------------+
| soundex('') |
+-------------+
|             |
+-------------+
1 row in set (0,00 sec)

mysql> select soundex('Q');
+--------------+
| soundex('Q') |
+--------------+
| Q000         |
+--------------+
1 row in set (0,00 sec)

mysql> select soundex('Quadratically');
+--------------------------+
| soundex('Quadratically') |
+--------------------------+
| Q36324                   |
+--------------------------+
1 row in set (0,00 sec)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure how close this should follow MySQL, since it is not a replacement. Always returning a string of 0 or 4 characters would be a possibility, too.

@bake
Copy link
Contributor Author

bake commented Oct 25, 2018

@kuba-- Should I change something?

@kuba--
Copy link
Contributor

kuba-- commented Oct 26, 2018

@bakerolls - could you rebase. Btw. I accepted this PR but to merge it we have to wait for a maintainer.

Signed-off-by: bake <[email protected]>

# Conflicts:
#	sql/expression/function/registry.go
@bake
Copy link
Contributor Author

bake commented Oct 26, 2018

Thank you!

@ajnavarro ajnavarro merged commit 82d62f9 into src-d:master Oct 30, 2018
@ajnavarro
Copy link
Contributor

@bakerolls thanks for the contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants