[SPARK-6201] [SQL] promote string and do widen types for IN

[adrian-wang]

@huangjs
Acutally spark sql will first go through analysis period, in which we do widen types and promote strings, and then optimization, where constant IN will be converted into INSET.

So it turn out that we only need to fix this for IN.

[SparkQA]

Test build #28382 has finished for PR 4945 at commit 6c838c4.

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

[chenghao-intel]

Causes unmatched exception?

  case StringType => Cast(a, DoubleType)
  case x => x

[liancheng]

Same as above.

[SparkQA]

Test build #28420 has finished for PR 4945 at commit cd72593.

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

[adrian-wang]

@liancheng That's reasonable, not every string could be converted into numeric types.
Hive specified about this as
/**

• GenericUDFIn
  *
• Example usage:
• SELECT key FROM src WHERE key IN ("238", "1");
  *
• From MySQL page on IN(): To comply with the SQL standard, IN returns NULL
• not only if the expression on the left hand side is NULL, but also if no
• match is found in the list and one of the expressions in the list is NULL.
  *
• Also noteworthy: type conversion behavior is different from MySQL. With
• expr IN expr1, expr2... in MySQL, exprN will each be converted into the same
• type as expr. In the Hive implementation, all expr(N) will be converted into
• a common type for conversion consistency with other UDF's, and to prevent
• conversions from a big type to a small type (e.g. int to tinyint)
  */

[marmbrus]

What is the status here? I haven't looked closely but this seems reasonable to me. However, I'd like to see more hive comparison tests that specifically test edge cases (different sized numbers, strings that can't be converted to numbers, etc) to make sure we are compatible with what they are doing.

[liancheng]

The thing that makes me hesitant here is whether we should stick to Hive, because Hive's behavior is actually error prone and unintuitive. In Hive, IN is implemented as a UDF, and function argument type coercion rules apply here.

Take "1.00" IN (1.0, 2.0) as an example, "1.00", 1.0, and 2.0 are all arguments of GenericUDFIn. When doing type coercion, 1.0 and 2.0 is first converted to string "1.0" and "2.0", and then compared with "1.00", thus returns false.

Personally I think maybe we should just throw an exception if the left side of IN has different data types from the right side.

[marmbrus]

Any comments on @liancheng 's suggestion?

[adrian-wang]

If we don't like what hive does here, we can do it in MySQL way: Convert all expressions in the list to the type of left handle of IN. In fact the main differences here is that Spark SQL promotes strings to numeric when do type coercion, while Hive seems do the contrary.

[marmbrus]

The mysql way seems reasonable to me. @liancheng ?

[liancheng]

@adrian-wang @marmbrus Sorry for the late reply. Yeah, the MySQL way also seems reasonable to me.

In both Spark SQL and MySQL, IN is treated more like an operator, which has its own reasonable type coercion rules. However, in Hive, IN is defined as a UDF, which follows general UDF argument type coercion rules, but those rules doesn't make sense here.

[SparkQA]

Test build #30732 has finished for PR 4945 at commit 581fa1c.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.
• This patch does not change any dependencies.

[SparkQA]

Test build #30734 has finished for PR 4945 at commit 71e05cc.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.
• This patch does not change any dependencies.

[SparkQA]

Test build #31058 has started for PR 4945 at commit 71e05cc.

[rxin]

Thanks. I'm going to merge this.

[yhuai]

Merged in master and branch 1.4. Thanks!