[SPARK-8772][SQL] Implement implicit type cast for expressions that define input types. #7175

rxin · 2015-07-02T02:40:01Z

No description provided.

rxin · 2015-07-02T02:40:46Z

This is based on #7174

AmplabJenkins · 2015-07-02T02:43:10Z

Merged build triggered.

AmplabJenkins · 2015-07-02T02:43:16Z

Merged build started.

SparkQA · 2015-07-02T02:44:35Z

Test build #36327 has started for PR 7175 at commit daa849b.

SparkQA · 2015-07-02T02:49:58Z

Test build #36327 has finished for PR 7175 at commit daa849b.

This patch fails to build.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- trait ExpectsInputTypes
- abstract class BinaryExpression extends Expression with trees.BinaryNode[Expression]
- abstract class BinaryOperator extends BinaryExpression
- abstract class BinaryArithmetic extends BinaryOperator
- case class Md5(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Sha1(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Crc32(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Not(child: Expression) extends UnaryExpression with Predicate with ExpectsInputTypes
- abstract class BinaryComparison extends BinaryOperator with Predicate
- trait StringRegexExpression extends ExpectsInputTypes
- trait CaseConversionExpression extends ExpectsInputTypes
- trait StringComparison extends ExpectsInputTypes
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes
- abstract class DataType extends AbstractDataType

AmplabJenkins · 2015-07-02T02:49:59Z

Merged build finished. Test FAILed.

AmplabJenkins · 2015-07-02T03:33:12Z

Merged build triggered.

AmplabJenkins · 2015-07-02T03:33:17Z

Merged build started.

SparkQA · 2015-07-02T03:34:32Z

Test build #36332 has started for PR 7175 at commit efbfa42.

…efines input types.

AmplabJenkins · 2015-07-02T04:28:11Z

Merged build triggered.

AmplabJenkins · 2015-07-02T04:28:18Z

Merged build started.

SparkQA · 2015-07-02T04:31:50Z

Test build #36336 has started for PR 7175 at commit f0ff97f.

chenghao-intel · 2015-07-02T05:24:52Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/AbstractDataType.scala

Most of the rule seems like to be:
DecimalType => DecimalType
FractionalType => DoubleType
LongType => LongType
IntegralType => IntegerType

SparkQA · 2015-07-02T05:39:03Z

Test build #36332 has finished for PR 7175 at commit efbfa42.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- trait ExpectsInputTypes
- abstract class BinaryExpression extends Expression with trees.BinaryNode[Expression]
- abstract class BinaryOperator extends BinaryExpression
- abstract class BinaryArithmetic extends BinaryOperator
- case class Md5(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Sha1(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Crc32(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Not(child: Expression) extends UnaryExpression with Predicate with ExpectsInputTypes
- abstract class BinaryComparison extends BinaryOperator with Predicate
- trait StringRegexExpression extends ExpectsInputTypes
- trait CaseConversionExpression extends ExpectsInputTypes
- trait StringComparison extends ExpectsInputTypes
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes
- protected[sql] abstract class AtomicType extends DataType
- abstract class NumericType extends AtomicType
- abstract class DataType extends AbstractDataType

AmplabJenkins · 2015-07-02T05:39:12Z

Merged build finished. Test FAILed.

yhuai · 2015-07-02T05:45:20Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/AbstractDataType.scala

When will we use this method?

SparkQA · 2015-07-02T05:58:19Z

Test build #36336 has finished for PR 7175 at commit f0ff97f.

This patch fails PySpark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- trait ExpectsInputTypes
- abstract class BinaryExpression extends Expression with trees.BinaryNode[Expression]
- abstract class BinaryOperator extends BinaryExpression
- abstract class BinaryArithmetic extends BinaryOperator
- case class Md5(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Sha1(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Crc32(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Not(child: Expression) extends UnaryExpression with Predicate with ExpectsInputTypes
- abstract class BinaryComparison extends BinaryOperator with Predicate
- trait StringRegexExpression extends ExpectsInputTypes
- trait CaseConversionExpression extends ExpectsInputTypes
- trait StringComparison extends ExpectsInputTypes
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes
- protected[sql] abstract class AtomicType extends DataType
- abstract class NumericType extends AtomicType
- abstract class DataType extends AbstractDataType

AmplabJenkins · 2015-07-02T05:58:28Z

Merged build finished. Test FAILed.

davies · 2015-07-02T05:59:01Z

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

Crc32 should be able to work with StringType, but StringType cannot be implicit casted BinaryType, right ?

i need to think about whether we should support implicit casts from string to binary. sql server does support that. hive doesn't, but hive chose to make a lot of the udfs work against both types.

I am not sure if we can always cast a string to binary correctly, as it produces different binary when specifying different encoder. It's actually the case accept multiple DataType for an expression.
And also for Length, which support both StringType and BinaryType.

We probably need another PR for this improvement.

I was thinking about having an AbstractDataType that's a TypeCollection, that expressions can put arbitrary types into it. Basically similar to the Seq[Any] idea, but with better type safety.

I think that's a good idea for this, but it probably make thing more complicated for auto casting. (Which data type should be cast to?)

+1 for StringType -> BinaryType (UTF8 will be used)

I mean we'd better leave the casting (StringType -> BinaryType) to be done within the UDF Crc32 itself, not via the generic auto casting rule. From the user perspective, the UDF Crc32 will support both StringType and BinaryType.

Oh, sorry, I just checked the code of Hive, it does convert the StringType => BinaryType (UTF8 bytes), just as the generic rule. @davies +1

AmplabJenkins · 2015-07-02T08:33:10Z

Merged build triggered.

rxin · 2015-07-02T08:33:14Z

I added an implicit type cast from String to Binary.

AmplabJenkins · 2015-07-02T08:33:16Z

Merged build started.

SparkQA · 2015-07-02T08:35:00Z

Test build #36375 has started for PR 7175 at commit 88080a2.

SparkQA · 2015-07-02T10:14:06Z

Test build #36375 has finished for PR 7175 at commit 88080a2.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
- case class Md5(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Sha1(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Crc32(child: Expression) extends UnaryExpression with ExpectsInputTypes
- case class Not(child: Expression) extends UnaryExpression with Predicate with ExpectsInputTypes
- trait StringRegexExpression extends ExpectsInputTypes
- trait CaseConversionExpression extends ExpectsInputTypes
- trait StringComparison extends ExpectsInputTypes
- case class StringLength(child: Expression) extends UnaryExpression with ExpectsInputTypes
- protected[sql] abstract class AtomicType extends DataType
- abstract class NumericType extends AtomicType
- abstract class DataType extends AbstractDataType

AmplabJenkins · 2015-07-02T10:14:56Z

Merged build finished. Test PASSed.

cloud-fan · 2015-07-02T12:09:10Z

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

according to hive and discussion in #6551,
should we only allow atomic type(except boolean and binary) to string?

cloud-fan · 2015-07-02T12:13:18Z

lgtm

cloud-fan · 2015-07-02T12:22:16Z

sql/catalyst/src/main/scala/org/apache/spark/sql/types/AbstractDataType.scala

Exist: why we add unapply for it? Is it same with Cast(child: NumericType, StringType)? It looks to me that we only need this object NumericType in ExpectsInputTypes when an expression need any kind of numeric input.
And, should we add object AtomicType too?

child is an expression.

sorry didn't see that...

btw this is old code. just got copied around.

rxin · 2015-07-02T21:16:01Z

@marmbrus i'm going to merge this one since it blocks @davies' pr. I will submit a follow up pr to support type collections for inputs. Please continue to review and I will address feedback together.

rxin changed the title ~~[SPARK-8772][SQL] Implement implicit type cast for expressions that defines input types.~~ [SPARK-8772][SQL] Implement implicit type cast for expressions that define input types. Jul 2, 2015

rxin added 2 commits July 1, 2015 21:23

[SPARK-8772][SQL] Implement implicit type cast for expressions that d…

c65e532

…efines input types.

Added missing file.

f0ff97f

rxin force-pushed the implicitCast branch from efbfa42 to f0ff97f Compare July 2, 2015 04:23

rxin mentioned this pull request Jul 2, 2015

[SPARK-8227][SQL]Add function unhex #7113

Closed

chenghao-intel reviewed Jul 2, 2015
View reviewed changes

yhuai reviewed Jul 2, 2015
View reviewed changes

sql/catalyst/src/main/scala/org/apache/spark/sql/types/AbstractDataType.scala Outdated

Copy link

Contributor

yhuai Jul 2, 2015

Choose a reason for hiding this comment

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

When will we use this method?

davies reviewed Jul 2, 2015
View reviewed changes

davies mentioned this pull request Jul 2, 2015

[SPARK-8784] [SQL] Add Python API for hex and unhex #7181

Closed

Clearer definition of implicit type cast.

88080a2

cloud-fan reviewed Jul 2, 2015
View reviewed changes

cloud-fan mentioned this pull request Jul 2, 2015

[SPARK-8759][SQL] add default eval to binary and unary expression according to default behavior of nullable #7157

Closed

asfgit closed this in 52508be Jul 2, 2015

[SPARK-8772][SQL] Implement implicit type cast for expressions that define input types. #7175

[SPARK-8772][SQL] Implement implicit type cast for expressions that define input types. #7175

Uh oh!

Conversation

rxin commented Jul 2, 2015

Uh oh!

rxin commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

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!

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!

AmplabJenkins commented Jul 2, 2015

Uh oh!

rxin commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

SparkQA commented Jul 2, 2015

Uh oh!

AmplabJenkins commented Jul 2, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Jul 2, 2015

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!