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

sql/expression: implement substring function #55

Merged
merged 1 commit into from
Feb 27, 2018

Conversation

erizocosmico
Copy link
Contributor

@erizocosmico erizocosmico commented Feb 26, 2018

Fixes #42

This PR implements the substring(str, start[, length]) function with the same rules as its homonym function in MySQL.
Since Go strings are UTF8, this function does not return a direct substring str[start:start+length], instead returns the substring of runes. That is, "á"[0:1] does not return a partial unicode glyph, but "á" itself.

Copy link
Contributor

@ajnavarro ajnavarro left a comment

Choose a reason for hiding this comment

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

Can you add a test using that function on engine_test.go please?

@erizocosmico
Copy link
Contributor Author

Done @ajnavarro

return nil, fmt.Errorf("substring: invalid length %d", length)
}

if startIdx < 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Take into account that:

  • The first position in string is 1.
  • If start_position is a positive number, then the SUBSTRING function starts from the beginning of the string.
  • If start_position is a negative number, then the SUBSTRING function starts from the end of the string and counts backwards. Negative values for start_position was introduced in MySQL 4.1.
  • If start_position is bigger than the string length, the result is NULL
  • If start_position is 0, the result is NULL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to the link you provided:
If start position is 0, the result is ""
if start_position is bigger than string length result is ""

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the UI is wrong. Check with: SELECT SUBSTRING("SQL Tutorial", 5, 3) AS ExtractString, NULL as test; the output of test appears to be "" but it is NULL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you do SELECT NULL is null returns 1, and if you do SELECT SUBSTRING("fooo bar", 0) is null it returns 0, so in this case is not null what it returns. From what I see in that UI, the only way SUBSTRING returns null is if the string itself is null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://dev.mysql.com/doc/refman/5.7/en/string-functions.html#function_substring

If len is less than 1, the result is the empty string.

startIdx := start.(int64)
runeCount := int64(len(text))

if length < 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

In mysql substring implementation length is optional

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, If length is bigger than the remained characters, no error happens

@ajnavarro
Copy link
Contributor

ajnavarro commented Feb 26, 2018

@erizocosmico You can check all corner cases here: https://www.w3schools.com/sql/trymysql.asp?filename=trysql_func_mysql_substring

Do you mind to add tests for them?

@erizocosmico
Copy link
Contributor Author

Yes, sure

Fixes src-d#42
This PR implements the substring(str, start[, length]) function with the same behaviour as its homonym MySQL function.
Since Go strings are UTF8, this function does not return a direct substring str[start:start+length], instead returns the substring of runes. That is, "á"[0:1] does not return a partial unicode glyph, but "á" itself.

Signed-off-by: Miguel Molina <miguel@erizocosmi.co>
@erizocosmico
Copy link
Contributor Author

Fixed @ajnavarro

@erizocosmico erizocosmico merged commit 2167aa0 into src-d:master Feb 27, 2018
@erizocosmico erizocosmico deleted the feature/substring branch February 27, 2018 11:57
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