[SPARK-20749][SQL] Built-in SQL Function Support - all variants of LEN[GTH] #18046

kiszk · 2017-05-20T23:14:01Z

What changes were proposed in this pull request?

This PR adds built-in SQL function BIT_LENGTH(), CHAR_LENGTH(), and OCTET_LENGTH() functions.

BIT_LENGTH() returns the bit length of the given string or binary expression.
CHAR_LENGTH() returns the length of the given string or binary expression. (i.e. equal to LENGTH())
OCTET_LENGTH() returns the byte length of the given string or binary expression.

How was this patch tested?

Added new test suites for these three functions

maropu · 2017-05-20T23:45:28Z

It seems you have the wrong JIRA number. Also, you need to add tests in SQLQueryTestSuite. Thanks.

SparkQA · 2017-05-21T01:35:52Z

Test build #77135 has finished for PR 18046 at commit 82ef305.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class BitLength(child: Expression) extends UnaryExpression with ImplicitCastInputTypes
case class OctetLength(child: Expression) extends UnaryExpression with ImplicitCastInputTypes

kiszk · 2017-05-21T06:21:08Z

@maropu Thank you for pointing them out. I addressed both.

SparkQA · 2017-05-21T06:22:38Z

Test build #77142 has started for PR 18046 at commit 5967d0f.

kiszk · 2017-05-21T07:09:51Z

Jenkins, retest this please

SparkQA · 2017-05-21T08:45:43Z

Test build #77143 has finished for PR 18046 at commit 5967d0f.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2017-05-21T09:20:04Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala

Is it 9 bits?

Good catch. updated.

viirya · 2017-05-21T09:44:03Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala

Is it common that length function is just an alias of char_length?

E.g., in mysql, length is an alias of octet_length.

Seems length is also an alias of char_length in Hive.

I think that length is an alias of octet_length in mysql. In postgreSql, length is alias an of char_length.

Based on the ANSI SQL, LENGTH function as a synonym for CHAR_LENGTH.

viirya · 2017-05-21T10:01:50Z

sql/core/src/test/resources/sql-tests/results/operators.sql.out

Seems you forget to update to operators.sql too?

Thanks, updated.

viirya · 2017-05-21T10:04:22Z

sql/core/src/test/resources/sql-tests/results/operators.sql.out

Should we use an example string which returns different results on character_length and octet_length?

These test suites handles the case. I have just used a simple case like other operations in the file.

SparkQA · 2017-05-21T11:32:35Z

Test build #77146 has started for PR 18046 at commit 400640a.

viirya · 2017-05-21T11:48:58Z

Can we also update the doc of Length? It is better to explicitly say it returns character length.

kiszk · 2017-05-21T12:41:35Z

@viirya I agree with you. However, I leave it for consistency of documents among versions.
I would like to hear @gatorsmile 's opinion.

kiszk · 2017-05-21T22:59:17Z

Jenkins, retest this please

SparkQA · 2017-05-21T23:52:01Z

Test build #77159 has started for PR 18046 at commit 400640a.

gatorsmile · 2017-05-22T17:44:00Z

@kiszk @viirya The document of length needs an update. Please always correct the function description if you find anything is incorrect. BTW, our current function description and test cases need more improvement. Please feel free to submit PRs to improve it. Thanks!

SparkQA · 2017-05-22T17:44:24Z

Test build #77190 has finished for PR 18046 at commit 9bc1976.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2017-05-22T17:48:29Z

Please run the following command to generate the result file:

SPARK_GENERATE_GOLDEN_FILES=1 build/sbt "sql/test-only *SQLQueryTestSuite"

SparkQA · 2017-05-23T00:21:34Z

Test build #77208 has finished for PR 18046 at commit 599fd31.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-05-23T06:25:32Z

Test build #77221 has finished for PR 18046 at commit e9acb63.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2017-05-23T07:29:04Z

@gatorsmile thank you for your suggestion.

kiszk · 2017-05-25T10:23:55Z

@gatorsmile Do I have to do additional things for this PR?

gatorsmile · 2017-05-28T00:11:56Z

@kiszk Could you resolve the conflict? Thanks!

SparkQA · 2017-05-28T04:57:31Z

Test build #77479 has started for PR 18046 at commit bf958c5.

kiszk · 2017-05-28T08:19:45Z

Jenkins, retest this please

SparkQA · 2017-05-28T10:41:11Z

Test build #77482 has finished for PR 18046 at commit bf958c5.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2017-05-28T10:45:04Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala

A function that returns the char length of the given string expression or number of bytes of the given binary expression.

viirya · 2017-05-28T10:47:24Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala

... that returns ...

viirya · 2017-05-28T10:48:00Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala

SparkQA · 2017-05-28T18:28:42Z

Test build #77488 has finished for PR 18046 at commit 8ca04ca.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2017-05-30T18:46:32Z

ping @gatorsmile

kiszk · 2017-06-03T18:44:47Z

ping @gatorsmile

SparkQA · 2017-06-12T11:40:59Z

Test build #77926 has finished for PR 18046 at commit e2bbc5a.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2017-06-12T12:06:31Z

Test build #77927 has finished for PR 18046 at commit 550a950.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

kiszk · 2017-06-12T15:28:09Z

ping @gatorsmile

SparkQA · 2017-06-15T05:32:34Z

Test build #78083 has started for PR 18046 at commit 1e8cbcd.

gatorsmile · 2017-06-15T22:50:56Z

retest this please

gatorsmile · 2017-06-15T22:51:08Z

@kiszk Sorry, I missed this ping.

SparkQA · 2017-06-16T01:17:04Z

Test build #78133 has finished for PR 18046 at commit 1e8cbcd.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

gatorsmile · 2017-06-16T06:05:40Z

LGTM

gatorsmile · 2017-06-16T06:07:17Z

Thanks! Merging to master.

kiszk changed the title ~~[SPARK-20746][SQL] Built-in SQL Function Support - all variants of LEN[GTH]~~ [SPARK-20749][SQL] Built-in SQL Function Support - all variants of LEN[GTH] May 21, 2017

viirya reviewed May 21, 2017

View reviewed changes

kiszk force-pushed the SPARK-20749 branch from 400640a to 9bc1976 Compare May 22, 2017 16:04

kiszk force-pushed the SPARK-20749 branch from e9acb63 to bf958c5 Compare May 28, 2017 04:54

viirya reviewed May 28, 2017

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala Outdated

Copy link

Member

viirya May 28, 2017

Choose a reason for hiding this comment

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

... that returns ...

viirya reviewed May 28, 2017

View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/stringExpressions.scala Outdated

Copy link

Member

viirya May 28, 2017

Choose a reason for hiding this comment

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

ditto

kiszk force-pushed the SPARK-20749 branch from 8ca04ca to e2bbc5a Compare June 12, 2017 09:14

kiszk added 10 commits June 15, 2017 14:02

initial commit

47b7821

fix test failures

fd34276

address review comment

f780268

fix test failures

7f7c194

update usage for length

422c0be

rebase master

7ee2f33

address review comments

dc2e43f

rebase master

b5acbde

fix failures

6b2b406

update output

1e8cbcd

kiszk force-pushed the SPARK-20749 branch from 550a950 to 1e8cbcd Compare June 15, 2017 05:30

asfgit closed this in 7a3e5dc Jun 16, 2017

[SPARK-20749][SQL] Built-in SQL Function Support - all variants of LEN[GTH] #18046

[SPARK-20749][SQL] Built-in SQL Function Support - all variants of LEN[GTH] #18046

Uh oh!

Conversation

kiszk commented May 20, 2017

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

maropu commented May 20, 2017

Uh oh!

SparkQA commented May 21, 2017

Uh oh!

kiszk commented May 21, 2017

Uh oh!

SparkQA commented May 21, 2017

Uh oh!

kiszk commented May 21, 2017

Uh oh!

SparkQA commented May 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viirya May 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

viirya May 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kiszk May 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kiszk May 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 21, 2017

Uh oh!

viirya commented May 21, 2017

Uh oh!

kiszk commented May 21, 2017

Uh oh!

kiszk commented May 21, 2017

Uh oh!

SparkQA commented May 21, 2017

Uh oh!

gatorsmile commented May 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented May 22, 2017

Uh oh!

gatorsmile commented May 22, 2017

Uh oh!

SparkQA commented May 23, 2017

Uh oh!

SparkQA commented May 23, 2017

Uh oh!

kiszk commented May 23, 2017

Uh oh!

kiszk commented May 25, 2017

Uh oh!

gatorsmile commented May 28, 2017

Uh oh!

SparkQA commented May 28, 2017

Uh oh!

kiszk commented May 28, 2017

viirya May 21, 2017 •

edited

Loading

viirya May 21, 2017 •

edited

Loading

kiszk May 21, 2017 •

edited

Loading

kiszk May 21, 2017 •

edited

Loading

gatorsmile commented May 22, 2017 •

edited

Loading

viirya May 28, 2017 •

edited

Loading