You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@spark.apache.org by Hyukjin Kwon <gu...@gmail.com> on 2017/11/14 14:54:32 UTC
[discuss][SQL] Partitioned column type inference proposal
Hi dev,
I would like to post a proposal about partitioned column type inference
(related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
configuration).
This thread focuses on the type coercion (finding the common type) in
partitioned columns, in particular, when the different form of data is
inserted for the partition column and then it is read back with the type
inference.
*Problem:*
val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
spark.read.load("/tmp/foo").printSchema()
val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
spark.read.load("/tmp/bar").printSchema()
It currently returns:
root
|-- i: integer (nullable = true)
|-- ts: date (nullable = true)
root
|-- i: integer (nullable = true)
|-- decimal: integer (nullable = true)
The type coercion looks less well designed yet and currently there are few
holes which is not quite ideal:
private val upCastingOrder: Seq[DataType] =
Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
...
literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
The current way does not deal with when the types are outside of the
upCastingOrder. It just returns the first type, as the type coerced one.
This has been being discussed in
https://github.com/apache/spark/pull/19389#discussion_r150426911, but I
would like to have more feedback from community as it possibly is a
breaking change.
For the current releases of Spark (2.2.0 <=), we support the types below
for partitioned column schema inference, given my investigation -
https://github.com/apache/spark/pull/19389#discussion_r150528207:
NullType
IntegerType
LongType
DoubleType,
*DecimalType(...)
DateType
TimestampType
StringType
*DecimalType only when it's bigger than LongType:
I believe this is something we should definitely fix.
*Proposal:*
I propose the change - https://github.com/apache/spark/pull/19389
Simply, it reuses the case 2 specified in
https://github.com/apache/spark/blob/6412ea1759d39a2380c572ec24cfd8ae4f2d81f7/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala#L40-L43
Please refer the chart I produced here -
https://github.com/apache/spark/pull/19389/files#r150528361. The current
proposal will brings the type coercion behaviour change in those cases
below:
Input typesOld output typeNew output type
[NullType, DecimalType(38,0)] StringType DecimalType(38,0)
[NullType, DateType] StringType DateType
[NullType, TimestampType] StringType TimestampType
[IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
[IntegerType, DateType] IntegerType StringType
[IntegerType, TimestampType] IntegerType StringType
[LongType, DecimalType(38,0)] LongType DecimalType(38,0)
[LongType, DateType] LongType StringType
[LongType, TimestampType] LongType StringType
[DoubleType, DateType] DoubleType StringType
[DoubleType, TimestampType] DoubleType StringType
[DecimalType(38,0), NullType] StringType DecimalType(38,0)
[DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
[DecimalType(38,0), LongType] LongType DecimalType(38,0)
[DecimalType(38,0), DateType] DecimalType(38,0) StringType
[DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
[DateType, NullType] StringType DateType
[DateType, IntegerType] IntegerType StringType
[DateType, LongType] LongType StringType
[DateType, DoubleType] DoubleType StringType
[DateType, DecimalType(38,0)] DateType StringType
[DateType, TimestampType] DateType TimestampType
[TimestampType, NullType] StringType TimestampType
[TimestampType, IntegerType] IntegerType StringType
[TimestampType, LongType] LongType StringType
[TimestampType, DoubleType] DoubleType StringType
[TimestampType, DecimalType(38,0)] TimestampType StringType
*Other possible suggestions**:*
Probably, we could also consider simply making the merged type to string
types when there are any type conflicts.
Otherwise, there could be more stricter rules. I want more opinion from the
community.
*Questions:*
- Does the *Proposal:* above looks good?
- If not, what would be the alternative?
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Wenchen Fan <cl...@gmail.com>.
> Don't most type coercion in programming languages convert Long to Double
That's true, but it's based on a fact that we clearly know one value is
long and one value is double.
For partition values, essentially they are all string type, we just try to
"guess" a better type. If we guess one value is long, and another value is
double, I think it's safer to give up the guess and fallback to string,
instead of using double type and suffer from precision loss.
On Tue, Nov 28, 2017 at 12:38 PM, Reynold Xin <rx...@databricks.com> wrote:
> I just read it again. Everything else seems reasonable, but Double + Long
> => Double seems a bit counterintuitive.
>
> Don't most type coercion in programming languages convert Long to Double
> (which would result in precision loss), than promoting them to string?
>
>
>
> On Mon, Nov 20, 2017 at 10:43 PM, Hyukjin Kwon <gu...@gmail.com>
> wrote:
>
>> Thanks all. I have updated my PR accordingly for now.
>>
>> 2017-11-16 2:01 GMT+09:00 Ryan Blue <rb...@netflix.com>:
>>
>>> Okay, thanks. That solves both problems, then.
>>>
>>> On Wed, Nov 15, 2017 at 8:51 AM, Wenchen Fan <cl...@gmail.com>
>>> wrote:
>>>
>>>> You can take a look at PartitioningUtils.inferPartitionColumnValue.
>>>> Like I mentioned before, decimal type here is used as big integer, if the
>>>> string is a floating point, Spark will try to parse it as double.
>>>>
>>>> On Wed, Nov 15, 2017 at 5:41 PM, Ryan Blue <rb...@netflix.com> wrote:
>>>>
>>>>> That looks good to me, but it doesn’t cover decimal scales. One
>>>>> obvious rule is to widen the scale to get a single type, if we don’t want
>>>>> to throw an exception.
>>>>>
>>>>> Also, if I follow this thread correctly, then the this is only used
>>>>> when inferring the type of a column from a String, right? In that case,
>>>>> when would Spark infer DoubleType instead of decimal?
>>>>>
>>>>> rb
>>>>>
>>>>>
>>>>> On Wed, Nov 15, 2017 at 4:42 AM, Hyukjin Kwon <gu...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Sorry, I meant:
>>>>>>
>>>>>> Before:
>>>>>>
>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>>> DateTypeTimestampTypeStringType
>>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>>> StringType StringType StringType
>>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>>>>> IntegerType IntegerType StringType
>>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>>> LongType StringType
>>>>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>>>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>> DoubleType DoubleType StringType
>>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>>> DateType DateType StringType
>>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>>> DoubleType TimestampType TimestampType StringType
>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>> StringType StringType StringType
>>>>>>
>>>>>> After:
>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>>> DateTypeTimestampTypeStringType
>>>>>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>>>>>> DateType TimestampType StringType
>>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>>> DoubleType StringType StringType StringType
>>>>>> *LongType* LongType LongType LongType DecimalType(38,0) StringType
>>>>>> StringType StringType StringType
>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>> DecimalType(38,0) DecimalType(38,0) StringType StringType StringType
>>>>>> StringType
>>>>>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>>>>>> StringType StringType StringType
>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>> DateType TimestampType StringType
>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>> StringType TimestampType TimestampType StringType
>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>> StringType StringType StringType
>>>>>>
>>>>>>
>>>>>> 2017-11-15 21:22 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>>>>
>>>>>>> If we do 1. and 2., I guess it will be:
>>>>>>>
>>>>>>>
>>>>>>> Before:
>>>>>>>
>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)
>>>>>>> DoubleTypeDateTypeTimestampTypeStringType
>>>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>>>> StringType StringType StringType
>>>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType
>>>>>>> DoubleType IntegerType IntegerType StringType
>>>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>>>> LongType StringType
>>>>>>> *DecimalType(38,0)* StringType IntegerType LongType
>>>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) DecimalType(38,0)
>>>>>>> StringType
>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>>> DoubleType DoubleType StringType
>>>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>>>> DateType DateType StringType
>>>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>>>> DoubleType TimestampType TimestampType StringType
>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>> StringType StringType StringType
>>>>>>>
>>>>>>> After:
>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)
>>>>>>> DoubleTypeDateTypeTimestampTypeStringType
>>>>>>> *NullType* NullType IntegerType LongType DecimalType(38,0)
>>>>>>> DoubleType DateType TimestampType StringType
>>>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>>>> DoubleType StringType StringType StringType
>>>>>>> *LongType* LongType LongType LongType StringType StringType
>>>>>>> StringType StringType StringType
>>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) StringType
>>>>>>> DecimalType(38,0) StringType StringType StringType StringType
>>>>>>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>>>>>>> StringType StringType StringType
>>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>>> DateType TimestampType StringType
>>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>>> StringType TimestampType TimestampType StringType
>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>> StringType StringType StringType
>>>>>>>
>>>>>>>
>>>>>>> Sounds fine to me and looks we can stay safer from precision loss
>>>>>>> concern.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 2017-11-15 19:15 GMT+09:00 Wenchen Fan <cl...@gmail.com>:
>>>>>>>
>>>>>>>> 1. shall we return NullType when merging 2 NullTypes?
>>>>>>>> 2. when merging DoubleType and LongType/DecimalType, we should
>>>>>>>> return StringType, otherwise precision lose may happen.
>>>>>>>>
>>>>>>>> One thing special for partition values type coercion is, we have a
>>>>>>>> safe fallback: the StringType. This means we can be very conservative on
>>>>>>>> type coercion, and reject any precision lose. While for normal type
>>>>>>>> coercion, sometimes type coercion is better than failing the query.
>>>>>>>>
>>>>>>>> On Wed, Nov 15, 2017 at 8:50 AM, Hyukjin Kwon <gu...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> I just reordered a little bit and coloured differences to check
>>>>>>>>> easily:
>>>>>>>>>
>>>>>>>>> Before:
>>>>>>>>>
>>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)
>>>>>>>>> DoubleTypeDateTypeTimestampTypeStringType
>>>>>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>>>>>> StringType StringType StringType
>>>>>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType
>>>>>>>>> DoubleType IntegerType IntegerType StringType
>>>>>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>>>>>> LongType StringType
>>>>>>>>> *DecimalType(38,0)* StringType IntegerType LongType
>>>>>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) DecimalType(38,0)
>>>>>>>>> StringType
>>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType
>>>>>>>>> DoubleType DoubleType DoubleType StringType
>>>>>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>>>>>> DateType DateType StringType
>>>>>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>>>>>> DoubleType TimestampType TimestampType StringType
>>>>>>>>> *StringType* StringType StringType StringType StringType
>>>>>>>>> StringType StringType StringType StringType
>>>>>>>>> After:
>>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)
>>>>>>>>> DoubleTypeDateTypeTimestampTypeStringType
>>>>>>>>> *NullType* StringType IntegerType LongType DecimalType(38,0)
>>>>>>>>> DoubleType DateType TimestampType StringType
>>>>>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>>>>>> DoubleType StringType StringType StringType
>>>>>>>>> *LongType* LongType LongType LongType DecimalType(38,0) DoubleType
>>>>>>>>> StringType StringType StringType
>>>>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>>>>> DecimalType(38,0) DecimalType(38,0) DoubleType StringType
>>>>>>>>> StringType StringType
>>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType
>>>>>>>>> DoubleType StringType StringType StringType
>>>>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>>>>> DateType TimestampType StringType
>>>>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>>>>> StringType TimestampType TimestampType StringType
>>>>>>>>> *StringType* StringType StringType StringType StringType
>>>>>>>>> StringType StringType StringType StringType
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>>>>>>>
>>>>>>>>>> Thanks all for feedback.
>>>>>>>>>>
>>>>>>>>>> > 1. when merging NullType with another type, the result should
>>>>>>>>>> always be that type.
>>>>>>>>>> > 2. when merging StringType with another type, the result should
>>>>>>>>>> always be StringType.
>>>>>>>>>> > 3. when merging integral types, the priority from high to low:
>>>>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>>>>> big integer when paring partition column values.
>>>>>>>>>> > 4. DoubleType can't be merged with other types, except
>>>>>>>>>> DoubleType itself.
>>>>>>>>>> > 5. when merging TimestampType with DateType, return
>>>>>>>>>> TimestampType.
>>>>>>>>>>
>>>>>>>>>> > Rather than a list, can we create a table? X-axis is data type,
>>>>>>>>>> and Y-axis is also data type, and the intersection explains what the
>>>>>>>>>> coerced type is?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Here, I produced a table as below:
>>>>>>>>>>
>>>>>>>>>> *Before*
>>>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleType
>>>>>>>>>> DecimalType(38,0)DateTypeTimestampTypeStringType
>>>>>>>>>> *NullType* StringType IntegerType LongType DoubleType StringType
>>>>>>>>>> StringType StringType StringType
>>>>>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>>>>>>> IntegerType IntegerType IntegerType StringType
>>>>>>>>>> *LongType* LongType LongType LongType DoubleType LongType
>>>>>>>>>> LongType LongType StringType
>>>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType
>>>>>>>>>> DoubleType DoubleType DoubleType StringType
>>>>>>>>>> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
>>>>>>>>>> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
>>>>>>>>>> *DateType* StringType IntegerType LongType DoubleType DateType
>>>>>>>>>> DateType DateType StringType
>>>>>>>>>> *TimestampType* StringType IntegerType LongType DoubleType
>>>>>>>>>> TimestampType TimestampType TimestampType StringType
>>>>>>>>>> *StringType* StringType StringType StringType StringType
>>>>>>>>>> StringType StringType StringType StringType
>>>>>>>>>>
>>>>>>>>>> *After*
>>>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleType
>>>>>>>>>> DecimalType(38,0)DateTypeTimestampTypeStringType
>>>>>>>>>> *NullType* StringType IntegerType LongType DoubleType
>>>>>>>>>> DecimalType(38,0) DateType TimestampType StringType
>>>>>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>>>>>>> DecimalType(38,0) StringType StringType StringType
>>>>>>>>>> *LongType* LongType LongType LongType DoubleType
>>>>>>>>>> DecimalType(38,0) StringType StringType StringType
>>>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType
>>>>>>>>>> DoubleType StringType StringType StringType
>>>>>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) StringType
>>>>>>>>>> StringType StringType
>>>>>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>>>>>> DateType TimestampType StringType
>>>>>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>>>>>> StringType TimestampType TimestampType StringType
>>>>>>>>>> *StringType* StringType StringType StringType StringType
>>>>>>>>>> StringType StringType StringType StringType
>>>>>>>>>>
>>>>>>>>>> Seems following Wenchen's comments (1. to 5.), and I also updated
>>>>>>>>>> PR description there with some codes I used.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> > Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>>>>> > Also, this shouldn't be isolated to partition column inference.
>>>>>>>>>> > We should make sure most of the type coercions are consistent
>>>>>>>>>> across different functionalities, with the caveat that we need to preserve
>>>>>>>>>> backward compatibility.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Sure, so, if I understood correctly, we preserve backward
>>>>>>>>>> compatibility for improvements, but not for bugs in general.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Probably, there are two things to be done now:
>>>>>>>>>>
>>>>>>>>>> - Deduplicates the type coercion logics, and fixes the obvious
>>>>>>>>>> bugs, that doesn't make sense, (e.g., decimal and timestamp ends up with
>>>>>>>>>> decimal).
>>>>>>>>>>
>>>>>>>>>> - Improves the deduplicated type coercion to follow other
>>>>>>>>>> systems like Hive and Postgres, or to make it sounds coherent by referring
>>>>>>>>>> them.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I think my PR here focuses on the former, because the current
>>>>>>>>>> partition column inference itself is already isolated
>>>>>>>>>> and I am trying to propose to put those type coercion into one
>>>>>>>>>> place in the TypeCoercion. We could also consider
>>>>>>>>>> some divergence as exceptions in the nature of this functionality
>>>>>>>>>> too, maybe .. ? (although I agree that most of the
>>>>>>>>>> type coercions are consistent across different functionalities in
>>>>>>>>>> general).
>>>>>>>>>>
>>>>>>>>>> If the deduplicated type coercion itself across functionalities
>>>>>>>>>> should be changed as an improvement, it should preserve
>>>>>>>>>> backward compatibility if I understood correctly and requires to
>>>>>>>>>> take a look for other systems.
>>>>>>>>>>
>>>>>>>>>> I am willing to do the latter soon; however, it probably takes a
>>>>>>>>>> quite while for me to investigate and propose a change.
>>>>>>>>>>
>>>>>>>>>> So, meanwhile, could we separately proceed this PR maybe or,
>>>>>>>>>> probably would there be something I missed?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>>>>>>>>>>
>>>>>>>>>>> Most of those thoughts from Wenchen make sense to me.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Rather than a list, can we create a table? X-axis is data type,
>>>>>>>>>>> and Y-axis is also data type, and the intersection explains what the
>>>>>>>>>>> coerced type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Also, this shouldn't be isolated to partition column inference.
>>>>>>>>>>> We should make sure most of the type coercions are consistent across
>>>>>>>>>>> different functionalities, with the caveat that we need to preserve
>>>>>>>>>>> backward compatibility.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <
>>>>>>>>>>> cloud0fan@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> My 2 cents:
>>>>>>>>>>>>
>>>>>>>>>>>> 1. when merging NullType with another type, the result should
>>>>>>>>>>>> always be that type.
>>>>>>>>>>>> 2. when merging StringType with another type, the result should
>>>>>>>>>>>> always be StringType.
>>>>>>>>>>>> 3. when merging integral types, the priority from high to low:
>>>>>>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>>>>>>> big integer when paring partition column values.
>>>>>>>>>>>> 4. DoubleType can't be merged with other types, except
>>>>>>>>>>>> DoubleType itself.
>>>>>>>>>>>> 5. when merging TimestampType with DateType, return
>>>>>>>>>>>> TimestampType.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <
>>>>>>>>>>>> gurwls223@gmail.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi dev,
>>>>>>>>>>>>>
>>>>>>>>>>>>> I would like to post a proposal about partitioned column type
>>>>>>>>>>>>> inference (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>>>>>>>>>>> configuration).
>>>>>>>>>>>>>
>>>>>>>>>>>>> This thread focuses on the type coercion (finding the common
>>>>>>>>>>>>> type) in partitioned columns, in particular, when the different form of
>>>>>>>>>>>>> data is inserted for the partition column and then it is read back with the
>>>>>>>>>>>>> type inference.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> *Problem:*
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>>>>>>>>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>>>>>>>>>>> spark.read.load("/tmp/foo").printSchema()
>>>>>>>>>>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>>>>>>>>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>>>>>>>>>>> spark.read.load("/tmp/bar").printSchema()
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> It currently returns:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> root
>>>>>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>>>>>> |-- ts: date (nullable = true)
>>>>>>>>>>>>>
>>>>>>>>>>>>> root
>>>>>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>>>>>> |-- decimal: integer (nullable = true)
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> The type coercion looks less well designed yet and currently
>>>>>>>>>>>>> there are few holes which is not quite ideal:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> private val upCastingOrder: Seq[DataType] =
>>>>>>>>>>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>>>>>>>>>>> ...
>>>>>>>>>>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> The current way does not deal with when the types are outside
>>>>>>>>>>>>> of the upCastingOrder. It just returns the first type, as the type coerced
>>>>>>>>>>>>> one.
>>>>>>>>>>>>>
>>>>>>>>>>>>> This has been being discussed in https://github.com/apache/s
>>>>>>>>>>>>> park/pull/19389#discussion_r150426911, but I would like to
>>>>>>>>>>>>> have more feedback from community as it possibly is a breaking change.
>>>>>>>>>>>>>
>>>>>>>>>>>>> For the current releases of Spark (2.2.0 <=), we support the
>>>>>>>>>>>>> types below for partitioned column schema inference, given my investigation
>>>>>>>>>>>>> - https://github.com/apache/spark/pull/19389#discussion_r150
>>>>>>>>>>>>> 528207:
>>>>>>>>>>>>>
>>>>>>>>>>>>> NullType
>>>>>>>>>>>>> IntegerType
>>>>>>>>>>>>> LongType
>>>>>>>>>>>>> DoubleType,
>>>>>>>>>>>>> *DecimalType(...)
>>>>>>>>>>>>> DateType
>>>>>>>>>>>>> TimestampType
>>>>>>>>>>>>> StringType
>>>>>>>>>>>>>
>>>>>>>>>>>>> *DecimalType only when it's bigger than LongType:
>>>>>>>>>>>>>
>>>>>>>>>>>>> I believe this is something we should definitely fix.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> *Proposal:*
>>>>>>>>>>>>>
>>>>>>>>>>>>> I propose the change - https://github.com/apache/sp
>>>>>>>>>>>>> ark/pull/19389
>>>>>>>>>>>>>
>>>>>>>>>>>>> Simply, it reuses the case 2 specified in
>>>>>>>>>>>>> https://github.com/apache/spark/blob/6412ea1759d39a2380c5
>>>>>>>>>>>>> 72ec24cfd8ae4f2d81f7/sql/catalyst/src/main/scala/org/apache/
>>>>>>>>>>>>> spark/sql/catalyst/analysis/TypeCoercion.scala#L40-L43
>>>>>>>>>>>>>
>>>>>>>>>>>>> Please refer the chart I produced here -
>>>>>>>>>>>>> https://github.com/apache/spark/pull/19389/files#r150528361.
>>>>>>>>>>>>> The current proposal will brings the type coercion behaviour change in
>>>>>>>>>>>>> those cases below:
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Input typesOld output typeNew output type
>>>>>>>>>>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>>>>>>>>>>> [NullType, DateType] StringType DateType
>>>>>>>>>>>>> [NullType, TimestampType] StringType TimestampType
>>>>>>>>>>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>>>>>>>>>>> [IntegerType, DateType] IntegerType StringType
>>>>>>>>>>>>> [IntegerType, TimestampType] IntegerType StringType
>>>>>>>>>>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>>>>>>>>>>> [LongType, DateType] LongType StringType
>>>>>>>>>>>>> [LongType, TimestampType] LongType StringType
>>>>>>>>>>>>> [DoubleType, DateType] DoubleType StringType
>>>>>>>>>>>>> [DoubleType, TimestampType] DoubleType StringType
>>>>>>>>>>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>>>>>>>>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>>>>>>>>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>>>>>>>>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>>>>>>>>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0)
>>>>>>>>>>>>> StringType
>>>>>>>>>>>>> [DateType, NullType] StringType DateType
>>>>>>>>>>>>> [DateType, IntegerType] IntegerType StringType
>>>>>>>>>>>>> [DateType, LongType] LongType StringType
>>>>>>>>>>>>> [DateType, DoubleType] DoubleType StringType
>>>>>>>>>>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>>>>>>>>>>> [DateType, TimestampType] DateType TimestampType
>>>>>>>>>>>>> [TimestampType, NullType] StringType TimestampType
>>>>>>>>>>>>> [TimestampType, IntegerType] IntegerType StringType
>>>>>>>>>>>>> [TimestampType, LongType] LongType StringType
>>>>>>>>>>>>> [TimestampType, DoubleType] DoubleType StringType
>>>>>>>>>>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> *Other possible suggestions**:*
>>>>>>>>>>>>>
>>>>>>>>>>>>> Probably, we could also consider simply making the merged type
>>>>>>>>>>>>> to string types when there are any type conflicts.
>>>>>>>>>>>>> Otherwise, there could be more stricter rules. I want more
>>>>>>>>>>>>> opinion from the community.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> *Questions:*
>>>>>>>>>>>>>
>>>>>>>>>>>>> - Does the *Proposal:* above looks good?
>>>>>>>>>>>>>
>>>>>>>>>>>>> - If not, what would be the alternative?
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Ryan Blue
>>>>> Software Engineer
>>>>> Netflix
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Ryan Blue
>>> Software Engineer
>>> Netflix
>>>
>>
>>
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Reynold Xin <rx...@databricks.com>.
I just read it again. Everything else seems reasonable, but Double + Long
=> Double seems a bit counterintuitive.
Don't most type coercion in programming languages convert Long to Double
(which would result in precision loss), than promoting them to string?
On Mon, Nov 20, 2017 at 10:43 PM, Hyukjin Kwon <gu...@gmail.com> wrote:
> Thanks all. I have updated my PR accordingly for now.
>
> 2017-11-16 2:01 GMT+09:00 Ryan Blue <rb...@netflix.com>:
>
>> Okay, thanks. That solves both problems, then.
>>
>> On Wed, Nov 15, 2017 at 8:51 AM, Wenchen Fan <cl...@gmail.com> wrote:
>>
>>> You can take a look at PartitioningUtils.inferPartitionColumnValue.
>>> Like I mentioned before, decimal type here is used as big integer, if the
>>> string is a floating point, Spark will try to parse it as double.
>>>
>>> On Wed, Nov 15, 2017 at 5:41 PM, Ryan Blue <rb...@netflix.com> wrote:
>>>
>>>> That looks good to me, but it doesn’t cover decimal scales. One obvious
>>>> rule is to widen the scale to get a single type, if we don’t want to throw
>>>> an exception.
>>>>
>>>> Also, if I follow this thread correctly, then the this is only used
>>>> when inferring the type of a column from a String, right? In that case,
>>>> when would Spark infer DoubleType instead of decimal?
>>>>
>>>> rb
>>>>
>>>>
>>>> On Wed, Nov 15, 2017 at 4:42 AM, Hyukjin Kwon <gu...@gmail.com>
>>>> wrote:
>>>>
>>>>> Sorry, I meant:
>>>>>
>>>>> Before:
>>>>>
>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>> DateTypeTimestampTypeStringType
>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>> StringType StringType StringType
>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>>>> IntegerType IntegerType StringType
>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>> LongType StringType
>>>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>> DoubleType DoubleType StringType
>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>> DateType DateType StringType
>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>> DoubleType TimestampType TimestampType StringType
>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>> StringType StringType StringType
>>>>>
>>>>> After:
>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>> DateTypeTimestampTypeStringType
>>>>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>>>>> DateType TimestampType StringType
>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>> DoubleType StringType StringType StringType
>>>>> *LongType* LongType LongType LongType DecimalType(38,0) StringType
>>>>> StringType StringType StringType
>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>> DecimalType(38,0) DecimalType(38,0) StringType StringType StringType
>>>>> StringType
>>>>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>>>>> StringType StringType StringType
>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>> DateType TimestampType StringType
>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>> StringType TimestampType TimestampType StringType
>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>> StringType StringType StringType
>>>>>
>>>>>
>>>>> 2017-11-15 21:22 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>>>
>>>>>> If we do 1. and 2., I guess it will be:
>>>>>>
>>>>>>
>>>>>> Before:
>>>>>>
>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>>> DateTypeTimestampTypeStringType
>>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>>> StringType StringType StringType
>>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>>>>> IntegerType IntegerType StringType
>>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>>> LongType StringType
>>>>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>>>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>> DoubleType DoubleType StringType
>>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>>> DateType DateType StringType
>>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>>> DoubleType TimestampType TimestampType StringType
>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>> StringType StringType StringType
>>>>>>
>>>>>> After:
>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>>> DateTypeTimestampTypeStringType
>>>>>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>>>>>> DateType TimestampType StringType
>>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>>> DoubleType StringType StringType StringType
>>>>>> *LongType* LongType LongType LongType StringType StringType
>>>>>> StringType StringType StringType
>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) StringType
>>>>>> DecimalType(38,0) StringType StringType StringType StringType
>>>>>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>>>>>> StringType StringType StringType
>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>> DateType TimestampType StringType
>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>> StringType TimestampType TimestampType StringType
>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>> StringType StringType StringType
>>>>>>
>>>>>>
>>>>>> Sounds fine to me and looks we can stay safer from precision loss
>>>>>> concern.
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2017-11-15 19:15 GMT+09:00 Wenchen Fan <cl...@gmail.com>:
>>>>>>
>>>>>>> 1. shall we return NullType when merging 2 NullTypes?
>>>>>>> 2. when merging DoubleType and LongType/DecimalType, we should
>>>>>>> return StringType, otherwise precision lose may happen.
>>>>>>>
>>>>>>> One thing special for partition values type coercion is, we have a
>>>>>>> safe fallback: the StringType. This means we can be very conservative on
>>>>>>> type coercion, and reject any precision lose. While for normal type
>>>>>>> coercion, sometimes type coercion is better than failing the query.
>>>>>>>
>>>>>>> On Wed, Nov 15, 2017 at 8:50 AM, Hyukjin Kwon <gu...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> I just reordered a little bit and coloured differences to check
>>>>>>>> easily:
>>>>>>>>
>>>>>>>> Before:
>>>>>>>>
>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)
>>>>>>>> DoubleTypeDateTypeTimestampTypeStringType
>>>>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>>>>> StringType StringType StringType
>>>>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType
>>>>>>>> DoubleType IntegerType IntegerType StringType
>>>>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>>>>> LongType StringType
>>>>>>>> *DecimalType(38,0)* StringType IntegerType LongType
>>>>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) DecimalType(38,0)
>>>>>>>> StringType
>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>>>> DoubleType DoubleType StringType
>>>>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>>>>> DateType DateType StringType
>>>>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>>>>> DoubleType TimestampType TimestampType StringType
>>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>>> StringType StringType StringType
>>>>>>>> After:
>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)
>>>>>>>> DoubleTypeDateTypeTimestampTypeStringType
>>>>>>>> *NullType* StringType IntegerType LongType DecimalType(38,0)
>>>>>>>> DoubleType DateType TimestampType StringType
>>>>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>>>>> DoubleType StringType StringType StringType
>>>>>>>> *LongType* LongType LongType LongType DecimalType(38,0) DoubleType
>>>>>>>> StringType StringType StringType
>>>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>>>> DecimalType(38,0) DecimalType(38,0) DoubleType StringType
>>>>>>>> StringType StringType
>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>>>> StringType StringType StringType
>>>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>>>> DateType TimestampType StringType
>>>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>>>> StringType TimestampType TimestampType StringType
>>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>>> StringType StringType StringType
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> 2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>>>>>>
>>>>>>>>> Thanks all for feedback.
>>>>>>>>>
>>>>>>>>> > 1. when merging NullType with another type, the result should
>>>>>>>>> always be that type.
>>>>>>>>> > 2. when merging StringType with another type, the result should
>>>>>>>>> always be StringType.
>>>>>>>>> > 3. when merging integral types, the priority from high to low:
>>>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>>>> big integer when paring partition column values.
>>>>>>>>> > 4. DoubleType can't be merged with other types, except
>>>>>>>>> DoubleType itself.
>>>>>>>>> > 5. when merging TimestampType with DateType, return
>>>>>>>>> TimestampType.
>>>>>>>>>
>>>>>>>>> > Rather than a list, can we create a table? X-axis is data type,
>>>>>>>>> and Y-axis is also data type, and the intersection explains what the
>>>>>>>>> coerced type is?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Here, I produced a table as below:
>>>>>>>>>
>>>>>>>>> *Before*
>>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleType
>>>>>>>>> DecimalType(38,0)DateTypeTimestampTypeStringType
>>>>>>>>> *NullType* StringType IntegerType LongType DoubleType StringType
>>>>>>>>> StringType StringType StringType
>>>>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>>>>>> IntegerType IntegerType IntegerType StringType
>>>>>>>>> *LongType* LongType LongType LongType DoubleType LongType LongType
>>>>>>>>> LongType StringType
>>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType
>>>>>>>>> DoubleType DoubleType DoubleType StringType
>>>>>>>>> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
>>>>>>>>> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
>>>>>>>>> *DateType* StringType IntegerType LongType DoubleType DateType
>>>>>>>>> DateType DateType StringType
>>>>>>>>> *TimestampType* StringType IntegerType LongType DoubleType
>>>>>>>>> TimestampType TimestampType TimestampType StringType
>>>>>>>>> *StringType* StringType StringType StringType StringType
>>>>>>>>> StringType StringType StringType StringType
>>>>>>>>>
>>>>>>>>> *After*
>>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleType
>>>>>>>>> DecimalType(38,0)DateTypeTimestampTypeStringType
>>>>>>>>> *NullType* StringType IntegerType LongType DoubleType
>>>>>>>>> DecimalType(38,0) DateType TimestampType StringType
>>>>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>>>>>> DecimalType(38,0) StringType StringType StringType
>>>>>>>>> *LongType* LongType LongType LongType DoubleType DecimalType(38,0)
>>>>>>>>> StringType StringType StringType
>>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType
>>>>>>>>> DoubleType StringType StringType StringType
>>>>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) StringType
>>>>>>>>> StringType StringType
>>>>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>>>>> DateType TimestampType StringType
>>>>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>>>>> StringType TimestampType TimestampType StringType
>>>>>>>>> *StringType* StringType StringType StringType StringType
>>>>>>>>> StringType StringType StringType StringType
>>>>>>>>>
>>>>>>>>> Seems following Wenchen's comments (1. to 5.), and I also updated
>>>>>>>>> PR description there with some codes I used.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> > Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>>>> > Also, this shouldn't be isolated to partition column inference.
>>>>>>>>> > We should make sure most of the type coercions are consistent
>>>>>>>>> across different functionalities, with the caveat that we need to preserve
>>>>>>>>> backward compatibility.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Sure, so, if I understood correctly, we preserve backward
>>>>>>>>> compatibility for improvements, but not for bugs in general.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Probably, there are two things to be done now:
>>>>>>>>>
>>>>>>>>> - Deduplicates the type coercion logics, and fixes the obvious
>>>>>>>>> bugs, that doesn't make sense, (e.g., decimal and timestamp ends up with
>>>>>>>>> decimal).
>>>>>>>>>
>>>>>>>>> - Improves the deduplicated type coercion to follow other
>>>>>>>>> systems like Hive and Postgres, or to make it sounds coherent by referring
>>>>>>>>> them.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I think my PR here focuses on the former, because the current
>>>>>>>>> partition column inference itself is already isolated
>>>>>>>>> and I am trying to propose to put those type coercion into one
>>>>>>>>> place in the TypeCoercion. We could also consider
>>>>>>>>> some divergence as exceptions in the nature of this functionality
>>>>>>>>> too, maybe .. ? (although I agree that most of the
>>>>>>>>> type coercions are consistent across different functionalities in
>>>>>>>>> general).
>>>>>>>>>
>>>>>>>>> If the deduplicated type coercion itself across functionalities
>>>>>>>>> should be changed as an improvement, it should preserve
>>>>>>>>> backward compatibility if I understood correctly and requires to
>>>>>>>>> take a look for other systems.
>>>>>>>>>
>>>>>>>>> I am willing to do the latter soon; however, it probably takes a
>>>>>>>>> quite while for me to investigate and propose a change.
>>>>>>>>>
>>>>>>>>> So, meanwhile, could we separately proceed this PR maybe or,
>>>>>>>>> probably would there be something I missed?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>>>>>>>>>
>>>>>>>>>> Most of those thoughts from Wenchen make sense to me.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Rather than a list, can we create a table? X-axis is data type,
>>>>>>>>>> and Y-axis is also data type, and the intersection explains what the
>>>>>>>>>> coerced type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Also, this shouldn't be isolated to partition column inference.
>>>>>>>>>> We should make sure most of the type coercions are consistent across
>>>>>>>>>> different functionalities, with the caveat that we need to preserve
>>>>>>>>>> backward compatibility.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cloud0fan@gmail.com
>>>>>>>>>> > wrote:
>>>>>>>>>>
>>>>>>>>>>> My 2 cents:
>>>>>>>>>>>
>>>>>>>>>>> 1. when merging NullType with another type, the result should
>>>>>>>>>>> always be that type.
>>>>>>>>>>> 2. when merging StringType with another type, the result should
>>>>>>>>>>> always be StringType.
>>>>>>>>>>> 3. when merging integral types, the priority from high to low:
>>>>>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>>>>>> big integer when paring partition column values.
>>>>>>>>>>> 4. DoubleType can't be merged with other types, except
>>>>>>>>>>> DoubleType itself.
>>>>>>>>>>> 5. when merging TimestampType with DateType, return
>>>>>>>>>>> TimestampType.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <
>>>>>>>>>>> gurwls223@gmail.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi dev,
>>>>>>>>>>>>
>>>>>>>>>>>> I would like to post a proposal about partitioned column type
>>>>>>>>>>>> inference (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>>>>>>>>>> configuration).
>>>>>>>>>>>>
>>>>>>>>>>>> This thread focuses on the type coercion (finding the common
>>>>>>>>>>>> type) in partitioned columns, in particular, when the different form of
>>>>>>>>>>>> data is inserted for the partition column and then it is read back with the
>>>>>>>>>>>> type inference.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> *Problem:*
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>>>>>>>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>>>>>>>>>> spark.read.load("/tmp/foo").printSchema()
>>>>>>>>>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>>>>>>>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>>>>>>>>>> spark.read.load("/tmp/bar").printSchema()
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> It currently returns:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> root
>>>>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>>>>> |-- ts: date (nullable = true)
>>>>>>>>>>>>
>>>>>>>>>>>> root
>>>>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>>>>> |-- decimal: integer (nullable = true)
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> The type coercion looks less well designed yet and currently
>>>>>>>>>>>> there are few holes which is not quite ideal:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> private val upCastingOrder: Seq[DataType] =
>>>>>>>>>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>>>>>>>>>> ...
>>>>>>>>>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> The current way does not deal with when the types are outside
>>>>>>>>>>>> of the upCastingOrder. It just returns the first type, as the type coerced
>>>>>>>>>>>> one.
>>>>>>>>>>>>
>>>>>>>>>>>> This has been being discussed in https://github.com/apache/s
>>>>>>>>>>>> park/pull/19389#discussion_r150426911, but I would like to
>>>>>>>>>>>> have more feedback from community as it possibly is a breaking change.
>>>>>>>>>>>>
>>>>>>>>>>>> For the current releases of Spark (2.2.0 <=), we support the
>>>>>>>>>>>> types below for partitioned column schema inference, given my investigation
>>>>>>>>>>>> - https://github.com/apache/spark/pull/19389#discussion_r150
>>>>>>>>>>>> 528207:
>>>>>>>>>>>>
>>>>>>>>>>>> NullType
>>>>>>>>>>>> IntegerType
>>>>>>>>>>>> LongType
>>>>>>>>>>>> DoubleType,
>>>>>>>>>>>> *DecimalType(...)
>>>>>>>>>>>> DateType
>>>>>>>>>>>> TimestampType
>>>>>>>>>>>> StringType
>>>>>>>>>>>>
>>>>>>>>>>>> *DecimalType only when it's bigger than LongType:
>>>>>>>>>>>>
>>>>>>>>>>>> I believe this is something we should definitely fix.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> *Proposal:*
>>>>>>>>>>>>
>>>>>>>>>>>> I propose the change - https://github.com/apache/sp
>>>>>>>>>>>> ark/pull/19389
>>>>>>>>>>>>
>>>>>>>>>>>> Simply, it reuses the case 2 specified in
>>>>>>>>>>>> https://github.com/apache/spark/blob/6412ea1759d39a2380c5
>>>>>>>>>>>> 72ec24cfd8ae4f2d81f7/sql/catalyst/src/main/scala/org/apache/
>>>>>>>>>>>> spark/sql/catalyst/analysis/TypeCoercion.scala#L40-L43
>>>>>>>>>>>>
>>>>>>>>>>>> Please refer the chart I produced here -
>>>>>>>>>>>> https://github.com/apache/spark/pull/19389/files#r150528361.
>>>>>>>>>>>> The current proposal will brings the type coercion behaviour change in
>>>>>>>>>>>> those cases below:
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Input typesOld output typeNew output type
>>>>>>>>>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>>>>>>>>>> [NullType, DateType] StringType DateType
>>>>>>>>>>>> [NullType, TimestampType] StringType TimestampType
>>>>>>>>>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>>>>>>>>>> [IntegerType, DateType] IntegerType StringType
>>>>>>>>>>>> [IntegerType, TimestampType] IntegerType StringType
>>>>>>>>>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>>>>>>>>>> [LongType, DateType] LongType StringType
>>>>>>>>>>>> [LongType, TimestampType] LongType StringType
>>>>>>>>>>>> [DoubleType, DateType] DoubleType StringType
>>>>>>>>>>>> [DoubleType, TimestampType] DoubleType StringType
>>>>>>>>>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>>>>>>>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>>>>>>>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>>>>>>>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>>>>>>>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>>>>>>>>>>> [DateType, NullType] StringType DateType
>>>>>>>>>>>> [DateType, IntegerType] IntegerType StringType
>>>>>>>>>>>> [DateType, LongType] LongType StringType
>>>>>>>>>>>> [DateType, DoubleType] DoubleType StringType
>>>>>>>>>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>>>>>>>>>> [DateType, TimestampType] DateType TimestampType
>>>>>>>>>>>> [TimestampType, NullType] StringType TimestampType
>>>>>>>>>>>> [TimestampType, IntegerType] IntegerType StringType
>>>>>>>>>>>> [TimestampType, LongType] LongType StringType
>>>>>>>>>>>> [TimestampType, DoubleType] DoubleType StringType
>>>>>>>>>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> *Other possible suggestions**:*
>>>>>>>>>>>>
>>>>>>>>>>>> Probably, we could also consider simply making the merged type
>>>>>>>>>>>> to string types when there are any type conflicts.
>>>>>>>>>>>> Otherwise, there could be more stricter rules. I want more
>>>>>>>>>>>> opinion from the community.
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> *Questions:*
>>>>>>>>>>>>
>>>>>>>>>>>> - Does the *Proposal:* above looks good?
>>>>>>>>>>>>
>>>>>>>>>>>> - If not, what would be the alternative?
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Ryan Blue
>>>> Software Engineer
>>>> Netflix
>>>>
>>>
>>>
>>
>>
>> --
>> Ryan Blue
>> Software Engineer
>> Netflix
>>
>
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Hyukjin Kwon <gu...@gmail.com>.
Thanks all. I have updated my PR accordingly for now.
2017-11-16 2:01 GMT+09:00 Ryan Blue <rb...@netflix.com>:
> Okay, thanks. That solves both problems, then.
>
> On Wed, Nov 15, 2017 at 8:51 AM, Wenchen Fan <cl...@gmail.com> wrote:
>
>> You can take a look at PartitioningUtils.inferPartitionColumnValue. Like
>> I mentioned before, decimal type here is used as big integer, if the string
>> is a floating point, Spark will try to parse it as double.
>>
>> On Wed, Nov 15, 2017 at 5:41 PM, Ryan Blue <rb...@netflix.com> wrote:
>>
>>> That looks good to me, but it doesn’t cover decimal scales. One obvious
>>> rule is to widen the scale to get a single type, if we don’t want to throw
>>> an exception.
>>>
>>> Also, if I follow this thread correctly, then the this is only used when
>>> inferring the type of a column from a String, right? In that case, when
>>> would Spark infer DoubleType instead of decimal?
>>>
>>> rb
>>>
>>>
>>> On Wed, Nov 15, 2017 at 4:42 AM, Hyukjin Kwon <gu...@gmail.com>
>>> wrote:
>>>
>>>> Sorry, I meant:
>>>>
>>>> Before:
>>>>
>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>> DateTypeTimestampTypeStringType
>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>> StringType StringType StringType
>>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>>> IntegerType IntegerType StringType
>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>> LongType StringType
>>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>> DoubleType DoubleType StringType
>>>> *DateType* StringType IntegerType LongType DateType DoubleType DateType
>>>> DateType StringType
>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>> DoubleType TimestampType TimestampType StringType
>>>> *StringType* StringType StringType StringType StringType StringType
>>>> StringType StringType StringType
>>>>
>>>> After:
>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>> DateTypeTimestampTypeStringType
>>>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>>>> DateType TimestampType StringType
>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>> DoubleType StringType StringType StringType
>>>> *LongType* LongType LongType LongType DecimalType(38,0) StringType
>>>> StringType StringType StringType
>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>> DecimalType(38,0) DecimalType(38,0) StringType StringType StringType
>>>> StringType
>>>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>>>> StringType StringType StringType
>>>> *DateType* DateType StringType StringType StringType StringType
>>>> DateType TimestampType StringType
>>>> *TimestampType* TimestampType StringType StringType StringType
>>>> StringType TimestampType TimestampType StringType
>>>> *StringType* StringType StringType StringType StringType StringType
>>>> StringType StringType StringType
>>>>
>>>>
>>>> 2017-11-15 21:22 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>>
>>>>> If we do 1. and 2., I guess it will be:
>>>>>
>>>>>
>>>>> Before:
>>>>>
>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>> DateTypeTimestampTypeStringType
>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>> StringType StringType StringType
>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>>>> IntegerType IntegerType StringType
>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>> LongType StringType
>>>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>> DoubleType DoubleType StringType
>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>> DateType DateType StringType
>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>> DoubleType TimestampType TimestampType StringType
>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>> StringType StringType StringType
>>>>>
>>>>> After:
>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>> DateTypeTimestampTypeStringType
>>>>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>>>>> DateType TimestampType StringType
>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>> DoubleType StringType StringType StringType
>>>>> *LongType* LongType LongType LongType StringType StringType StringType
>>>>> StringType StringType
>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) StringType
>>>>> DecimalType(38,0) StringType StringType StringType StringType
>>>>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>>>>> StringType StringType StringType
>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>> DateType TimestampType StringType
>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>> StringType TimestampType TimestampType StringType
>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>> StringType StringType StringType
>>>>>
>>>>>
>>>>> Sounds fine to me and looks we can stay safer from precision loss
>>>>> concern.
>>>>>
>>>>>
>>>>>
>>>>> 2017-11-15 19:15 GMT+09:00 Wenchen Fan <cl...@gmail.com>:
>>>>>
>>>>>> 1. shall we return NullType when merging 2 NullTypes?
>>>>>> 2. when merging DoubleType and LongType/DecimalType, we should return
>>>>>> StringType, otherwise precision lose may happen.
>>>>>>
>>>>>> One thing special for partition values type coercion is, we have a
>>>>>> safe fallback: the StringType. This means we can be very conservative on
>>>>>> type coercion, and reject any precision lose. While for normal type
>>>>>> coercion, sometimes type coercion is better than failing the query.
>>>>>>
>>>>>> On Wed, Nov 15, 2017 at 8:50 AM, Hyukjin Kwon <gu...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> I just reordered a little bit and coloured differences to check
>>>>>>> easily:
>>>>>>>
>>>>>>> Before:
>>>>>>>
>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)
>>>>>>> DoubleTypeDateTypeTimestampTypeStringType
>>>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>>>> StringType StringType StringType
>>>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType
>>>>>>> DoubleType IntegerType IntegerType StringType
>>>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>>>> LongType StringType
>>>>>>> *DecimalType(38,0)* StringType IntegerType LongType
>>>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) DecimalType(38,0)
>>>>>>> StringType
>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>>> DoubleType DoubleType StringType
>>>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>>>> DateType DateType StringType
>>>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>>>> DoubleType TimestampType TimestampType StringType
>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>> StringType StringType StringType
>>>>>>> After:
>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)
>>>>>>> DoubleTypeDateTypeTimestampTypeStringType
>>>>>>> *NullType* StringType IntegerType LongType DecimalType(38,0)
>>>>>>> DoubleType DateType TimestampType StringType
>>>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>>>> DoubleType StringType StringType StringType
>>>>>>> *LongType* LongType LongType LongType DecimalType(38,0) DoubleType
>>>>>>> StringType StringType StringType
>>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>>> DecimalType(38,0) DecimalType(38,0) DoubleType StringType StringType
>>>>>>> StringType
>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>>> StringType StringType StringType
>>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>>> DateType TimestampType StringType
>>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>>> StringType TimestampType TimestampType StringType
>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>> StringType StringType StringType
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>>>>>
>>>>>>>> Thanks all for feedback.
>>>>>>>>
>>>>>>>> > 1. when merging NullType with another type, the result should
>>>>>>>> always be that type.
>>>>>>>> > 2. when merging StringType with another type, the result should
>>>>>>>> always be StringType.
>>>>>>>> > 3. when merging integral types, the priority from high to low:
>>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>>> big integer when paring partition column values.
>>>>>>>> > 4. DoubleType can't be merged with other types, except DoubleType
>>>>>>>> itself.
>>>>>>>> > 5. when merging TimestampType with DateType, return TimestampType.
>>>>>>>>
>>>>>>>> > Rather than a list, can we create a table? X-axis is data type,
>>>>>>>> and Y-axis is also data type, and the intersection explains what the
>>>>>>>> coerced type is?
>>>>>>>>
>>>>>>>>
>>>>>>>> Here, I produced a table as below:
>>>>>>>>
>>>>>>>> *Before*
>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleType
>>>>>>>> DecimalType(38,0)DateTypeTimestampTypeStringType
>>>>>>>> *NullType* StringType IntegerType LongType DoubleType StringType
>>>>>>>> StringType StringType StringType
>>>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>>>>> IntegerType IntegerType IntegerType StringType
>>>>>>>> *LongType* LongType LongType LongType DoubleType LongType LongType
>>>>>>>> LongType StringType
>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>>>> DoubleType DoubleType StringType
>>>>>>>> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
>>>>>>>> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
>>>>>>>> *DateType* StringType IntegerType LongType DoubleType DateType
>>>>>>>> DateType DateType StringType
>>>>>>>> *TimestampType* StringType IntegerType LongType DoubleType
>>>>>>>> TimestampType TimestampType TimestampType StringType
>>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>>> StringType StringType StringType
>>>>>>>>
>>>>>>>> *After*
>>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleType
>>>>>>>> DecimalType(38,0)DateTypeTimestampTypeStringType
>>>>>>>> *NullType* StringType IntegerType LongType DoubleType
>>>>>>>> DecimalType(38,0) DateType TimestampType StringType
>>>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>>>>> DecimalType(38,0) StringType StringType StringType
>>>>>>>> *LongType* LongType LongType LongType DoubleType DecimalType(38,0)
>>>>>>>> StringType StringType StringType
>>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>>>> StringType StringType StringType
>>>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) StringType
>>>>>>>> StringType StringType
>>>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>>>> DateType TimestampType StringType
>>>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>>>> StringType TimestampType TimestampType StringType
>>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>>> StringType StringType StringType
>>>>>>>>
>>>>>>>> Seems following Wenchen's comments (1. to 5.), and I also updated
>>>>>>>> PR description there with some codes I used.
>>>>>>>>
>>>>>>>>
>>>>>>>> > Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>>> > Also, this shouldn't be isolated to partition column inference.
>>>>>>>> > We should make sure most of the type coercions are consistent
>>>>>>>> across different functionalities, with the caveat that we need to preserve
>>>>>>>> backward compatibility.
>>>>>>>>
>>>>>>>>
>>>>>>>> Sure, so, if I understood correctly, we preserve backward
>>>>>>>> compatibility for improvements, but not for bugs in general.
>>>>>>>>
>>>>>>>>
>>>>>>>> Probably, there are two things to be done now:
>>>>>>>>
>>>>>>>> - Deduplicates the type coercion logics, and fixes the obvious
>>>>>>>> bugs, that doesn't make sense, (e.g., decimal and timestamp ends up with
>>>>>>>> decimal).
>>>>>>>>
>>>>>>>> - Improves the deduplicated type coercion to follow other systems
>>>>>>>> like Hive and Postgres, or to make it sounds coherent by referring them.
>>>>>>>>
>>>>>>>>
>>>>>>>> I think my PR here focuses on the former, because the current
>>>>>>>> partition column inference itself is already isolated
>>>>>>>> and I am trying to propose to put those type coercion into one
>>>>>>>> place in the TypeCoercion. We could also consider
>>>>>>>> some divergence as exceptions in the nature of this functionality
>>>>>>>> too, maybe .. ? (although I agree that most of the
>>>>>>>> type coercions are consistent across different functionalities in
>>>>>>>> general).
>>>>>>>>
>>>>>>>> If the deduplicated type coercion itself across functionalities
>>>>>>>> should be changed as an improvement, it should preserve
>>>>>>>> backward compatibility if I understood correctly and requires to
>>>>>>>> take a look for other systems.
>>>>>>>>
>>>>>>>> I am willing to do the latter soon; however, it probably takes a
>>>>>>>> quite while for me to investigate and propose a change.
>>>>>>>>
>>>>>>>> So, meanwhile, could we separately proceed this PR maybe or,
>>>>>>>> probably would there be something I missed?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>>>>>>>>
>>>>>>>>> Most of those thoughts from Wenchen make sense to me.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Rather than a list, can we create a table? X-axis is data type,
>>>>>>>>> and Y-axis is also data type, and the intersection explains what the
>>>>>>>>> coerced type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Also, this shouldn't be isolated to partition column inference. We
>>>>>>>>> should make sure most of the type coercions are consistent across different
>>>>>>>>> functionalities, with the caveat that we need to preserve backward
>>>>>>>>> compatibility.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> My 2 cents:
>>>>>>>>>>
>>>>>>>>>> 1. when merging NullType with another type, the result should
>>>>>>>>>> always be that type.
>>>>>>>>>> 2. when merging StringType with another type, the result should
>>>>>>>>>> always be StringType.
>>>>>>>>>> 3. when merging integral types, the priority from high to low:
>>>>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>>>>> big integer when paring partition column values.
>>>>>>>>>> 4. DoubleType can't be merged with other types, except DoubleType
>>>>>>>>>> itself.
>>>>>>>>>> 5. when merging TimestampType with DateType, return TimestampType.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <
>>>>>>>>>> gurwls223@gmail.com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi dev,
>>>>>>>>>>>
>>>>>>>>>>> I would like to post a proposal about partitioned column type
>>>>>>>>>>> inference (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>>>>>>>>> configuration).
>>>>>>>>>>>
>>>>>>>>>>> This thread focuses on the type coercion (finding the common
>>>>>>>>>>> type) in partitioned columns, in particular, when the different form of
>>>>>>>>>>> data is inserted for the partition column and then it is read back with the
>>>>>>>>>>> type inference.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> *Problem:*
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>>>>>>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>>>>>>>>> spark.read.load("/tmp/foo").printSchema()
>>>>>>>>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>>>>>>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>>>>>>>>> spark.read.load("/tmp/bar").printSchema()
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> It currently returns:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> root
>>>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>>>> |-- ts: date (nullable = true)
>>>>>>>>>>>
>>>>>>>>>>> root
>>>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>>>> |-- decimal: integer (nullable = true)
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> The type coercion looks less well designed yet and currently
>>>>>>>>>>> there are few holes which is not quite ideal:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> private val upCastingOrder: Seq[DataType] =
>>>>>>>>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>>>>>>>>> ...
>>>>>>>>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> The current way does not deal with when the types are outside of
>>>>>>>>>>> the upCastingOrder. It just returns the first type, as the type coerced one.
>>>>>>>>>>>
>>>>>>>>>>> This has been being discussed in https://github.com/apache/s
>>>>>>>>>>> park/pull/19389#discussion_r150426911, but I would like to have
>>>>>>>>>>> more feedback from community as it possibly is a breaking change.
>>>>>>>>>>>
>>>>>>>>>>> For the current releases of Spark (2.2.0 <=), we support the
>>>>>>>>>>> types below for partitioned column schema inference, given my investigation
>>>>>>>>>>> - https://github.com/apache/spark/pull/19389#discussion_r150
>>>>>>>>>>> 528207:
>>>>>>>>>>>
>>>>>>>>>>> NullType
>>>>>>>>>>> IntegerType
>>>>>>>>>>> LongType
>>>>>>>>>>> DoubleType,
>>>>>>>>>>> *DecimalType(...)
>>>>>>>>>>> DateType
>>>>>>>>>>> TimestampType
>>>>>>>>>>> StringType
>>>>>>>>>>>
>>>>>>>>>>> *DecimalType only when it's bigger than LongType:
>>>>>>>>>>>
>>>>>>>>>>> I believe this is something we should definitely fix.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> *Proposal:*
>>>>>>>>>>>
>>>>>>>>>>> I propose the change - https://github.com/apache/sp
>>>>>>>>>>> ark/pull/19389
>>>>>>>>>>>
>>>>>>>>>>> Simply, it reuses the case 2 specified in
>>>>>>>>>>> https://github.com/apache/spark/blob/6412ea1759d39a2380c5
>>>>>>>>>>> 72ec24cfd8ae4f2d81f7/sql/catalyst/src/main/scala/org/apache/
>>>>>>>>>>> spark/sql/catalyst/analysis/TypeCoercion.scala#L40-L43
>>>>>>>>>>>
>>>>>>>>>>> Please refer the chart I produced here -
>>>>>>>>>>> https://github.com/apache/spark/pull/19389/files#r150528361.
>>>>>>>>>>> The current proposal will brings the type coercion behaviour change in
>>>>>>>>>>> those cases below:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Input typesOld output typeNew output type
>>>>>>>>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>>>>>>>>> [NullType, DateType] StringType DateType
>>>>>>>>>>> [NullType, TimestampType] StringType TimestampType
>>>>>>>>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>>>>>>>>> [IntegerType, DateType] IntegerType StringType
>>>>>>>>>>> [IntegerType, TimestampType] IntegerType StringType
>>>>>>>>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>>>>>>>>> [LongType, DateType] LongType StringType
>>>>>>>>>>> [LongType, TimestampType] LongType StringType
>>>>>>>>>>> [DoubleType, DateType] DoubleType StringType
>>>>>>>>>>> [DoubleType, TimestampType] DoubleType StringType
>>>>>>>>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>>>>>>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>>>>>>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>>>>>>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>>>>>>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>>>>>>>>>> [DateType, NullType] StringType DateType
>>>>>>>>>>> [DateType, IntegerType] IntegerType StringType
>>>>>>>>>>> [DateType, LongType] LongType StringType
>>>>>>>>>>> [DateType, DoubleType] DoubleType StringType
>>>>>>>>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>>>>>>>>> [DateType, TimestampType] DateType TimestampType
>>>>>>>>>>> [TimestampType, NullType] StringType TimestampType
>>>>>>>>>>> [TimestampType, IntegerType] IntegerType StringType
>>>>>>>>>>> [TimestampType, LongType] LongType StringType
>>>>>>>>>>> [TimestampType, DoubleType] DoubleType StringType
>>>>>>>>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> *Other possible suggestions**:*
>>>>>>>>>>>
>>>>>>>>>>> Probably, we could also consider simply making the merged type
>>>>>>>>>>> to string types when there are any type conflicts.
>>>>>>>>>>> Otherwise, there could be more stricter rules. I want more
>>>>>>>>>>> opinion from the community.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> *Questions:*
>>>>>>>>>>>
>>>>>>>>>>> - Does the *Proposal:* above looks good?
>>>>>>>>>>>
>>>>>>>>>>> - If not, what would be the alternative?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> Ryan Blue
>>> Software Engineer
>>> Netflix
>>>
>>
>>
>
>
> --
> Ryan Blue
> Software Engineer
> Netflix
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Ryan Blue <rb...@netflix.com.INVALID>.
Okay, thanks. That solves both problems, then.
On Wed, Nov 15, 2017 at 8:51 AM, Wenchen Fan <cl...@gmail.com> wrote:
> You can take a look at PartitioningUtils.inferPartitionColumnValue. Like
> I mentioned before, decimal type here is used as big integer, if the string
> is a floating point, Spark will try to parse it as double.
>
> On Wed, Nov 15, 2017 at 5:41 PM, Ryan Blue <rb...@netflix.com> wrote:
>
>> That looks good to me, but it doesn’t cover decimal scales. One obvious
>> rule is to widen the scale to get a single type, if we don’t want to throw
>> an exception.
>>
>> Also, if I follow this thread correctly, then the this is only used when
>> inferring the type of a column from a String, right? In that case, when
>> would Spark infer DoubleType instead of decimal?
>>
>> rb
>>
>>
>> On Wed, Nov 15, 2017 at 4:42 AM, Hyukjin Kwon <gu...@gmail.com>
>> wrote:
>>
>>> Sorry, I meant:
>>>
>>> Before:
>>>
>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>> DateTypeTimestampTypeStringType
>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>> StringType StringType StringType
>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>> IntegerType IntegerType StringType
>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>> LongType StringType
>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>> DoubleType DoubleType StringType
>>> *DateType* StringType IntegerType LongType DateType DoubleType DateType
>>> DateType StringType
>>> *TimestampType* StringType IntegerType LongType TimestampType DoubleType
>>> TimestampType TimestampType StringType
>>> *StringType* StringType StringType StringType StringType StringType
>>> StringType StringType StringType
>>>
>>> After:
>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>> DateTypeTimestampTypeStringType
>>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>>> DateType TimestampType StringType
>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>> DoubleType StringType StringType StringType
>>> *LongType* LongType LongType LongType DecimalType(38,0) StringType
>>> StringType StringType StringType
>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>> DecimalType(38,0) DecimalType(38,0) StringType StringType StringType
>>> StringType
>>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>>> StringType StringType StringType
>>> *DateType* DateType StringType StringType StringType StringType DateType
>>> TimestampType StringType
>>> *TimestampType* TimestampType StringType StringType StringType
>>> StringType TimestampType TimestampType StringType
>>> *StringType* StringType StringType StringType StringType StringType
>>> StringType StringType StringType
>>>
>>>
>>> 2017-11-15 21:22 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>
>>>> If we do 1. and 2., I guess it will be:
>>>>
>>>>
>>>> Before:
>>>>
>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>> DateTypeTimestampTypeStringType
>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>> StringType StringType StringType
>>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>>> IntegerType IntegerType StringType
>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>> LongType StringType
>>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>> DoubleType DoubleType StringType
>>>> *DateType* StringType IntegerType LongType DateType DoubleType DateType
>>>> DateType StringType
>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>> DoubleType TimestampType TimestampType StringType
>>>> *StringType* StringType StringType StringType StringType StringType
>>>> StringType StringType StringType
>>>>
>>>> After:
>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>> DateTypeTimestampTypeStringType
>>>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>>>> DateType TimestampType StringType
>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>> DoubleType StringType StringType StringType
>>>> *LongType* LongType LongType LongType StringType StringType StringType
>>>> StringType StringType
>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) StringType
>>>> DecimalType(38,0) StringType StringType StringType StringType
>>>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>>>> StringType StringType StringType
>>>> *DateType* DateType StringType StringType StringType StringType
>>>> DateType TimestampType StringType
>>>> *TimestampType* TimestampType StringType StringType StringType
>>>> StringType TimestampType TimestampType StringType
>>>> *StringType* StringType StringType StringType StringType StringType
>>>> StringType StringType StringType
>>>>
>>>>
>>>> Sounds fine to me and looks we can stay safer from precision loss
>>>> concern.
>>>>
>>>>
>>>>
>>>> 2017-11-15 19:15 GMT+09:00 Wenchen Fan <cl...@gmail.com>:
>>>>
>>>>> 1. shall we return NullType when merging 2 NullTypes?
>>>>> 2. when merging DoubleType and LongType/DecimalType, we should return
>>>>> StringType, otherwise precision lose may happen.
>>>>>
>>>>> One thing special for partition values type coercion is, we have a
>>>>> safe fallback: the StringType. This means we can be very conservative on
>>>>> type coercion, and reject any precision lose. While for normal type
>>>>> coercion, sometimes type coercion is better than failing the query.
>>>>>
>>>>> On Wed, Nov 15, 2017 at 8:50 AM, Hyukjin Kwon <gu...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> I just reordered a little bit and coloured differences to check
>>>>>> easily:
>>>>>>
>>>>>> Before:
>>>>>>
>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>>> DateTypeTimestampTypeStringType
>>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>>> StringType StringType StringType
>>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>>>>> IntegerType IntegerType StringType
>>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>>> LongType StringType
>>>>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>>>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>> DoubleType DoubleType StringType
>>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>>> DateType DateType StringType
>>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>>> DoubleType TimestampType TimestampType StringType
>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>> StringType StringType StringType
>>>>>> After:
>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>>> DateTypeTimestampTypeStringType
>>>>>> *NullType* StringType IntegerType LongType DecimalType(38,0)
>>>>>> DoubleType DateType TimestampType StringType
>>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>>> DoubleType StringType StringType StringType
>>>>>> *LongType* LongType LongType LongType DecimalType(38,0) DoubleType
>>>>>> StringType StringType StringType
>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>> DecimalType(38,0) DecimalType(38,0) DoubleType StringType StringType
>>>>>> StringType
>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>> StringType StringType StringType
>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>> DateType TimestampType StringType
>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>> StringType TimestampType TimestampType StringType
>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>> StringType StringType StringType
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>>>>
>>>>>>> Thanks all for feedback.
>>>>>>>
>>>>>>> > 1. when merging NullType with another type, the result should
>>>>>>> always be that type.
>>>>>>> > 2. when merging StringType with another type, the result should
>>>>>>> always be StringType.
>>>>>>> > 3. when merging integral types, the priority from high to low:
>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>> big integer when paring partition column values.
>>>>>>> > 4. DoubleType can't be merged with other types, except DoubleType
>>>>>>> itself.
>>>>>>> > 5. when merging TimestampType with DateType, return TimestampType.
>>>>>>>
>>>>>>> > Rather than a list, can we create a table? X-axis is data type,
>>>>>>> and Y-axis is also data type, and the intersection explains what the
>>>>>>> coerced type is?
>>>>>>>
>>>>>>>
>>>>>>> Here, I produced a table as below:
>>>>>>>
>>>>>>> *Before*
>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleType
>>>>>>> DecimalType(38,0)DateTypeTimestampTypeStringType
>>>>>>> *NullType* StringType IntegerType LongType DoubleType StringType
>>>>>>> StringType StringType StringType
>>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>>>> IntegerType IntegerType IntegerType StringType
>>>>>>> *LongType* LongType LongType LongType DoubleType LongType LongType
>>>>>>> LongType StringType
>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>>> DoubleType DoubleType StringType
>>>>>>> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
>>>>>>> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
>>>>>>> *DateType* StringType IntegerType LongType DoubleType DateType
>>>>>>> DateType DateType StringType
>>>>>>> *TimestampType* StringType IntegerType LongType DoubleType
>>>>>>> TimestampType TimestampType TimestampType StringType
>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>> StringType StringType StringType
>>>>>>>
>>>>>>> *After*
>>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleType
>>>>>>> DecimalType(38,0)DateTypeTimestampTypeStringType
>>>>>>> *NullType* StringType IntegerType LongType DoubleType
>>>>>>> DecimalType(38,0) DateType TimestampType StringType
>>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>>>> DecimalType(38,0) StringType StringType StringType
>>>>>>> *LongType* LongType LongType LongType DoubleType DecimalType(38,0)
>>>>>>> StringType StringType StringType
>>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>>> StringType StringType StringType
>>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) StringType StringType
>>>>>>> StringType
>>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>>> DateType TimestampType StringType
>>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>>> StringType TimestampType TimestampType StringType
>>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>>> StringType StringType StringType
>>>>>>>
>>>>>>> Seems following Wenchen's comments (1. to 5.), and I also updated PR
>>>>>>> description there with some codes I used.
>>>>>>>
>>>>>>>
>>>>>>> > Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>> > Also, this shouldn't be isolated to partition column inference.
>>>>>>> > We should make sure most of the type coercions are consistent
>>>>>>> across different functionalities, with the caveat that we need to preserve
>>>>>>> backward compatibility.
>>>>>>>
>>>>>>>
>>>>>>> Sure, so, if I understood correctly, we preserve backward
>>>>>>> compatibility for improvements, but not for bugs in general.
>>>>>>>
>>>>>>>
>>>>>>> Probably, there are two things to be done now:
>>>>>>>
>>>>>>> - Deduplicates the type coercion logics, and fixes the obvious
>>>>>>> bugs, that doesn't make sense, (e.g., decimal and timestamp ends up with
>>>>>>> decimal).
>>>>>>>
>>>>>>> - Improves the deduplicated type coercion to follow other systems
>>>>>>> like Hive and Postgres, or to make it sounds coherent by referring them.
>>>>>>>
>>>>>>>
>>>>>>> I think my PR here focuses on the former, because the current
>>>>>>> partition column inference itself is already isolated
>>>>>>> and I am trying to propose to put those type coercion into one place
>>>>>>> in the TypeCoercion. We could also consider
>>>>>>> some divergence as exceptions in the nature of this functionality
>>>>>>> too, maybe .. ? (although I agree that most of the
>>>>>>> type coercions are consistent across different functionalities in
>>>>>>> general).
>>>>>>>
>>>>>>> If the deduplicated type coercion itself across functionalities
>>>>>>> should be changed as an improvement, it should preserve
>>>>>>> backward compatibility if I understood correctly and requires to
>>>>>>> take a look for other systems.
>>>>>>>
>>>>>>> I am willing to do the latter soon; however, it probably takes a
>>>>>>> quite while for me to investigate and propose a change.
>>>>>>>
>>>>>>> So, meanwhile, could we separately proceed this PR maybe or,
>>>>>>> probably would there be something I missed?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>>>>>>>
>>>>>>>> Most of those thoughts from Wenchen make sense to me.
>>>>>>>>
>>>>>>>>
>>>>>>>> Rather than a list, can we create a table? X-axis is data type, and
>>>>>>>> Y-axis is also data type, and the intersection explains what the coerced
>>>>>>>> type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>>>
>>>>>>>>
>>>>>>>> Also, this shouldn't be isolated to partition column inference. We
>>>>>>>> should make sure most of the type coercions are consistent across different
>>>>>>>> functionalities, with the caveat that we need to preserve backward
>>>>>>>> compatibility.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> My 2 cents:
>>>>>>>>>
>>>>>>>>> 1. when merging NullType with another type, the result should
>>>>>>>>> always be that type.
>>>>>>>>> 2. when merging StringType with another type, the result should
>>>>>>>>> always be StringType.
>>>>>>>>> 3. when merging integral types, the priority from high to low:
>>>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>>>> big integer when paring partition column values.
>>>>>>>>> 4. DoubleType can't be merged with other types, except DoubleType
>>>>>>>>> itself.
>>>>>>>>> 5. when merging TimestampType with DateType, return TimestampType.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gurwls223@gmail.com
>>>>>>>>> > wrote:
>>>>>>>>>
>>>>>>>>>> Hi dev,
>>>>>>>>>>
>>>>>>>>>> I would like to post a proposal about partitioned column type
>>>>>>>>>> inference (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>>>>>>>> configuration).
>>>>>>>>>>
>>>>>>>>>> This thread focuses on the type coercion (finding the common
>>>>>>>>>> type) in partitioned columns, in particular, when the different form of
>>>>>>>>>> data is inserted for the partition column and then it is read back with the
>>>>>>>>>> type inference.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> *Problem:*
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>>>>>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>>>>>>>> spark.read.load("/tmp/foo").printSchema()
>>>>>>>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>>>>>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>>>>>>>> spark.read.load("/tmp/bar").printSchema()
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> It currently returns:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> root
>>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>>> |-- ts: date (nullable = true)
>>>>>>>>>>
>>>>>>>>>> root
>>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>>> |-- decimal: integer (nullable = true)
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The type coercion looks less well designed yet and currently
>>>>>>>>>> there are few holes which is not quite ideal:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> private val upCastingOrder: Seq[DataType] =
>>>>>>>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>>>>>>>> ...
>>>>>>>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> The current way does not deal with when the types are outside of
>>>>>>>>>> the upCastingOrder. It just returns the first type, as the type coerced one.
>>>>>>>>>>
>>>>>>>>>> This has been being discussed in https://github.com/apache/s
>>>>>>>>>> park/pull/19389#discussion_r150426911, but I would like to have
>>>>>>>>>> more feedback from community as it possibly is a breaking change.
>>>>>>>>>>
>>>>>>>>>> For the current releases of Spark (2.2.0 <=), we support the
>>>>>>>>>> types below for partitioned column schema inference, given my investigation
>>>>>>>>>> - https://github.com/apache/spark/pull/19389#discussion_r150
>>>>>>>>>> 528207:
>>>>>>>>>>
>>>>>>>>>> NullType
>>>>>>>>>> IntegerType
>>>>>>>>>> LongType
>>>>>>>>>> DoubleType,
>>>>>>>>>> *DecimalType(...)
>>>>>>>>>> DateType
>>>>>>>>>> TimestampType
>>>>>>>>>> StringType
>>>>>>>>>>
>>>>>>>>>> *DecimalType only when it's bigger than LongType:
>>>>>>>>>>
>>>>>>>>>> I believe this is something we should definitely fix.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> *Proposal:*
>>>>>>>>>>
>>>>>>>>>> I propose the change - https://github.com/apache/spark/pull/19389
>>>>>>>>>>
>>>>>>>>>> Simply, it reuses the case 2 specified in
>>>>>>>>>> https://github.com/apache/spark/blob/6412ea1759d39a2380c5
>>>>>>>>>> 72ec24cfd8ae4f2d81f7/sql/catalyst/src/main/scala/org/apache/
>>>>>>>>>> spark/sql/catalyst/analysis/TypeCoercion.scala#L40-L43
>>>>>>>>>>
>>>>>>>>>> Please refer the chart I produced here -
>>>>>>>>>> https://github.com/apache/spark/pull/19389/files#r150528361. The
>>>>>>>>>> current proposal will brings the type coercion behaviour change in those
>>>>>>>>>> cases below:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Input typesOld output typeNew output type
>>>>>>>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>>>>>>>> [NullType, DateType] StringType DateType
>>>>>>>>>> [NullType, TimestampType] StringType TimestampType
>>>>>>>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>>>>>>>> [IntegerType, DateType] IntegerType StringType
>>>>>>>>>> [IntegerType, TimestampType] IntegerType StringType
>>>>>>>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>>>>>>>> [LongType, DateType] LongType StringType
>>>>>>>>>> [LongType, TimestampType] LongType StringType
>>>>>>>>>> [DoubleType, DateType] DoubleType StringType
>>>>>>>>>> [DoubleType, TimestampType] DoubleType StringType
>>>>>>>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>>>>>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>>>>>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>>>>>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>>>>>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>>>>>>>>> [DateType, NullType] StringType DateType
>>>>>>>>>> [DateType, IntegerType] IntegerType StringType
>>>>>>>>>> [DateType, LongType] LongType StringType
>>>>>>>>>> [DateType, DoubleType] DoubleType StringType
>>>>>>>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>>>>>>>> [DateType, TimestampType] DateType TimestampType
>>>>>>>>>> [TimestampType, NullType] StringType TimestampType
>>>>>>>>>> [TimestampType, IntegerType] IntegerType StringType
>>>>>>>>>> [TimestampType, LongType] LongType StringType
>>>>>>>>>> [TimestampType, DoubleType] DoubleType StringType
>>>>>>>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> *Other possible suggestions**:*
>>>>>>>>>>
>>>>>>>>>> Probably, we could also consider simply making the merged type to
>>>>>>>>>> string types when there are any type conflicts.
>>>>>>>>>> Otherwise, there could be more stricter rules. I want more
>>>>>>>>>> opinion from the community.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> *Questions:*
>>>>>>>>>>
>>>>>>>>>> - Does the *Proposal:* above looks good?
>>>>>>>>>>
>>>>>>>>>> - If not, what would be the alternative?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>>
>> --
>> Ryan Blue
>> Software Engineer
>> Netflix
>>
>
>
--
Ryan Blue
Software Engineer
Netflix
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Wenchen Fan <cl...@gmail.com>.
You can take a look at PartitioningUtils.inferPartitionColumnValue. Like I
mentioned before, decimal type here is used as big integer, if the string
is a floating point, Spark will try to parse it as double.
On Wed, Nov 15, 2017 at 5:41 PM, Ryan Blue <rb...@netflix.com> wrote:
> That looks good to me, but it doesn’t cover decimal scales. One obvious
> rule is to widen the scale to get a single type, if we don’t want to throw
> an exception.
>
> Also, if I follow this thread correctly, then the this is only used when
> inferring the type of a column from a String, right? In that case, when
> would Spark infer DoubleType instead of decimal?
>
> rb
>
>
> On Wed, Nov 15, 2017 at 4:42 AM, Hyukjin Kwon <gu...@gmail.com> wrote:
>
>> Sorry, I meant:
>>
>> Before:
>>
>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>> DateTypeTimestampTypeStringType
>> *NullType* StringType IntegerType LongType StringType DoubleType
>> StringType StringType StringType
>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>> IntegerType IntegerType StringType
>> *LongType* LongType LongType LongType LongType DoubleType LongType
>> LongType StringType
>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>> DoubleType DoubleType StringType
>> *DateType* StringType IntegerType LongType DateType DoubleType DateType
>> DateType StringType
>> *TimestampType* StringType IntegerType LongType TimestampType DoubleType
>> TimestampType TimestampType StringType
>> *StringType* StringType StringType StringType StringType StringType
>> StringType StringType StringType
>>
>> After:
>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>> DateTypeTimestampTypeStringType
>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>> DateType TimestampType StringType
>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>> DoubleType StringType StringType StringType
>> *LongType* LongType LongType LongType DecimalType(38,0) StringType
>> StringType StringType StringType
>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) DecimalType(38,0)
>> DecimalType(38,0) StringType StringType StringType StringType
>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>> StringType StringType StringType
>> *DateType* DateType StringType StringType StringType StringType DateType
>> TimestampType StringType
>> *TimestampType* TimestampType StringType StringType StringType StringType
>> TimestampType TimestampType StringType
>> *StringType* StringType StringType StringType StringType StringType
>> StringType StringType StringType
>>
>>
>> 2017-11-15 21:22 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>
>>> If we do 1. and 2., I guess it will be:
>>>
>>>
>>> Before:
>>>
>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>> DateTypeTimestampTypeStringType
>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>> StringType StringType StringType
>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>> IntegerType IntegerType StringType
>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>> LongType StringType
>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>> DoubleType DoubleType StringType
>>> *DateType* StringType IntegerType LongType DateType DoubleType DateType
>>> DateType StringType
>>> *TimestampType* StringType IntegerType LongType TimestampType DoubleType
>>> TimestampType TimestampType StringType
>>> *StringType* StringType StringType StringType StringType StringType
>>> StringType StringType StringType
>>>
>>> After:
>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>> DateTypeTimestampTypeStringType
>>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>>> DateType TimestampType StringType
>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>> DoubleType StringType StringType StringType
>>> *LongType* LongType LongType LongType StringType StringType StringType
>>> StringType StringType
>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) StringType
>>> DecimalType(38,0) StringType StringType StringType StringType
>>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>>> StringType StringType StringType
>>> *DateType* DateType StringType StringType StringType StringType DateType
>>> TimestampType StringType
>>> *TimestampType* TimestampType StringType StringType StringType
>>> StringType TimestampType TimestampType StringType
>>> *StringType* StringType StringType StringType StringType StringType
>>> StringType StringType StringType
>>>
>>>
>>> Sounds fine to me and looks we can stay safer from precision loss
>>> concern.
>>>
>>>
>>>
>>> 2017-11-15 19:15 GMT+09:00 Wenchen Fan <cl...@gmail.com>:
>>>
>>>> 1. shall we return NullType when merging 2 NullTypes?
>>>> 2. when merging DoubleType and LongType/DecimalType, we should return
>>>> StringType, otherwise precision lose may happen.
>>>>
>>>> One thing special for partition values type coercion is, we have a safe
>>>> fallback: the StringType. This means we can be very conservative on type
>>>> coercion, and reject any precision lose. While for normal type coercion,
>>>> sometimes type coercion is better than failing the query.
>>>>
>>>> On Wed, Nov 15, 2017 at 8:50 AM, Hyukjin Kwon <gu...@gmail.com>
>>>> wrote:
>>>>
>>>>> I just reordered a little bit and coloured differences to check easily:
>>>>>
>>>>> Before:
>>>>>
>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>> DateTypeTimestampTypeStringType
>>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>>> StringType StringType StringType
>>>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>>>> IntegerType IntegerType StringType
>>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>>> LongType StringType
>>>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>> DoubleType DoubleType StringType
>>>>> *DateType* StringType IntegerType LongType DateType DoubleType
>>>>> DateType DateType StringType
>>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>>> DoubleType TimestampType TimestampType StringType
>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>> StringType StringType StringType
>>>>> After:
>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>>> DateTypeTimestampTypeStringType
>>>>> *NullType* StringType IntegerType LongType DecimalType(38,0)
>>>>> DoubleType DateType TimestampType StringType
>>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>>> DoubleType StringType StringType StringType
>>>>> *LongType* LongType LongType LongType DecimalType(38,0) DoubleType
>>>>> StringType StringType StringType
>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>> DecimalType(38,0) DecimalType(38,0) DoubleType StringType StringType
>>>>> StringType
>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>> StringType StringType StringType
>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>> DateType TimestampType StringType
>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>> StringType TimestampType TimestampType StringType
>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>> StringType StringType StringType
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> 2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>>>
>>>>>> Thanks all for feedback.
>>>>>>
>>>>>> > 1. when merging NullType with another type, the result should
>>>>>> always be that type.
>>>>>> > 2. when merging StringType with another type, the result should
>>>>>> always be StringType.
>>>>>> > 3. when merging integral types, the priority from high to low:
>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>> big integer when paring partition column values.
>>>>>> > 4. DoubleType can't be merged with other types, except DoubleType
>>>>>> itself.
>>>>>> > 5. when merging TimestampType with DateType, return TimestampType.
>>>>>>
>>>>>> > Rather than a list, can we create a table? X-axis is data type, and
>>>>>> Y-axis is also data type, and the intersection explains what the coerced
>>>>>> type is?
>>>>>>
>>>>>>
>>>>>> Here, I produced a table as below:
>>>>>>
>>>>>> *Before*
>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>>>>>> DateTypeTimestampTypeStringType
>>>>>> *NullType* StringType IntegerType LongType DoubleType StringType
>>>>>> StringType StringType StringType
>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType IntegerType
>>>>>> IntegerType IntegerType StringType
>>>>>> *LongType* LongType LongType LongType DoubleType LongType LongType
>>>>>> LongType StringType
>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>> DoubleType DoubleType StringType
>>>>>> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
>>>>>> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
>>>>>> *DateType* StringType IntegerType LongType DoubleType DateType
>>>>>> DateType DateType StringType
>>>>>> *TimestampType* StringType IntegerType LongType DoubleType
>>>>>> TimestampType TimestampType TimestampType StringType
>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>> StringType StringType StringType
>>>>>>
>>>>>> *After*
>>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>>>>>> DateTypeTimestampTypeStringType
>>>>>> *NullType* StringType IntegerType LongType DoubleType
>>>>>> DecimalType(38,0) DateType TimestampType StringType
>>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>>> DecimalType(38,0) StringType StringType StringType
>>>>>> *LongType* LongType LongType LongType DoubleType DecimalType(38,0)
>>>>>> StringType StringType StringType
>>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>>> StringType StringType StringType
>>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) StringType StringType
>>>>>> StringType
>>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>>> DateType TimestampType StringType
>>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>>> StringType TimestampType TimestampType StringType
>>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>>> StringType StringType StringType
>>>>>>
>>>>>> Seems following Wenchen's comments (1. to 5.), and I also updated PR
>>>>>> description there with some codes I used.
>>>>>>
>>>>>>
>>>>>> > Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>> > Also, this shouldn't be isolated to partition column inference.
>>>>>> > We should make sure most of the type coercions are consistent
>>>>>> across different functionalities, with the caveat that we need to preserve
>>>>>> backward compatibility.
>>>>>>
>>>>>>
>>>>>> Sure, so, if I understood correctly, we preserve backward
>>>>>> compatibility for improvements, but not for bugs in general.
>>>>>>
>>>>>>
>>>>>> Probably, there are two things to be done now:
>>>>>>
>>>>>> - Deduplicates the type coercion logics, and fixes the obvious
>>>>>> bugs, that doesn't make sense, (e.g., decimal and timestamp ends up with
>>>>>> decimal).
>>>>>>
>>>>>> - Improves the deduplicated type coercion to follow other systems
>>>>>> like Hive and Postgres, or to make it sounds coherent by referring them.
>>>>>>
>>>>>>
>>>>>> I think my PR here focuses on the former, because the current
>>>>>> partition column inference itself is already isolated
>>>>>> and I am trying to propose to put those type coercion into one place
>>>>>> in the TypeCoercion. We could also consider
>>>>>> some divergence as exceptions in the nature of this functionality
>>>>>> too, maybe .. ? (although I agree that most of the
>>>>>> type coercions are consistent across different functionalities in
>>>>>> general).
>>>>>>
>>>>>> If the deduplicated type coercion itself across functionalities
>>>>>> should be changed as an improvement, it should preserve
>>>>>> backward compatibility if I understood correctly and requires to take
>>>>>> a look for other systems.
>>>>>>
>>>>>> I am willing to do the latter soon; however, it probably takes a
>>>>>> quite while for me to investigate and propose a change.
>>>>>>
>>>>>> So, meanwhile, could we separately proceed this PR maybe or, probably
>>>>>> would there be something I missed?
>>>>>>
>>>>>>
>>>>>>
>>>>>> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>>>>>>
>>>>>>> Most of those thoughts from Wenchen make sense to me.
>>>>>>>
>>>>>>>
>>>>>>> Rather than a list, can we create a table? X-axis is data type, and
>>>>>>> Y-axis is also data type, and the intersection explains what the coerced
>>>>>>> type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>>
>>>>>>>
>>>>>>> Also, this shouldn't be isolated to partition column inference. We
>>>>>>> should make sure most of the type coercions are consistent across different
>>>>>>> functionalities, with the caveat that we need to preserve backward
>>>>>>> compatibility.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> My 2 cents:
>>>>>>>>
>>>>>>>> 1. when merging NullType with another type, the result should
>>>>>>>> always be that type.
>>>>>>>> 2. when merging StringType with another type, the result should
>>>>>>>> always be StringType.
>>>>>>>> 3. when merging integral types, the priority from high to low:
>>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>>> big integer when paring partition column values.
>>>>>>>> 4. DoubleType can't be merged with other types, except DoubleType
>>>>>>>> itself.
>>>>>>>> 5. when merging TimestampType with DateType, return TimestampType.
>>>>>>>>
>>>>>>>>
>>>>>>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gu...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> Hi dev,
>>>>>>>>>
>>>>>>>>> I would like to post a proposal about partitioned column type
>>>>>>>>> inference (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>>>>>>> configuration).
>>>>>>>>>
>>>>>>>>> This thread focuses on the type coercion (finding the common
>>>>>>>>> type) in partitioned columns, in particular, when the different form of
>>>>>>>>> data is inserted for the partition column and then it is read back with the
>>>>>>>>> type inference.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> *Problem:*
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>>>>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>>>>>>> spark.read.load("/tmp/foo").printSchema()
>>>>>>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>>>>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>>>>>>> spark.read.load("/tmp/bar").printSchema()
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> It currently returns:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> root
>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>> |-- ts: date (nullable = true)
>>>>>>>>>
>>>>>>>>> root
>>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>>> |-- decimal: integer (nullable = true)
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The type coercion looks less well designed yet and currently there
>>>>>>>>> are few holes which is not quite ideal:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> private val upCastingOrder: Seq[DataType] =
>>>>>>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>>>>>>> ...
>>>>>>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> The current way does not deal with when the types are outside of
>>>>>>>>> the upCastingOrder. It just returns the first type, as the type coerced one.
>>>>>>>>>
>>>>>>>>> This has been being discussed in https://github.com/apache/s
>>>>>>>>> park/pull/19389#discussion_r150426911, but I would like to have
>>>>>>>>> more feedback from community as it possibly is a breaking change.
>>>>>>>>>
>>>>>>>>> For the current releases of Spark (2.2.0 <=), we support the types
>>>>>>>>> below for partitioned column schema inference, given my investigation -
>>>>>>>>> https://github.com/apache/spark/pull/19389#discussion_r150528207:
>>>>>>>>>
>>>>>>>>> NullType
>>>>>>>>> IntegerType
>>>>>>>>> LongType
>>>>>>>>> DoubleType,
>>>>>>>>> *DecimalType(...)
>>>>>>>>> DateType
>>>>>>>>> TimestampType
>>>>>>>>> StringType
>>>>>>>>>
>>>>>>>>> *DecimalType only when it's bigger than LongType:
>>>>>>>>>
>>>>>>>>> I believe this is something we should definitely fix.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> *Proposal:*
>>>>>>>>>
>>>>>>>>> I propose the change - https://github.com/apache/spark/pull/19389
>>>>>>>>>
>>>>>>>>> Simply, it reuses the case 2 specified in
>>>>>>>>> https://github.com/apache/spark/blob/6412ea1759d39a2380c5
>>>>>>>>> 72ec24cfd8ae4f2d81f7/sql/catalyst/src/main/scala/org/apache/
>>>>>>>>> spark/sql/catalyst/analysis/TypeCoercion.scala#L40-L43
>>>>>>>>>
>>>>>>>>> Please refer the chart I produced here -
>>>>>>>>> https://github.com/apache/spark/pull/19389/files#r150528361. The
>>>>>>>>> current proposal will brings the type coercion behaviour change in those
>>>>>>>>> cases below:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Input typesOld output typeNew output type
>>>>>>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>>>>>>> [NullType, DateType] StringType DateType
>>>>>>>>> [NullType, TimestampType] StringType TimestampType
>>>>>>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>>>>>>> [IntegerType, DateType] IntegerType StringType
>>>>>>>>> [IntegerType, TimestampType] IntegerType StringType
>>>>>>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>>>>>>> [LongType, DateType] LongType StringType
>>>>>>>>> [LongType, TimestampType] LongType StringType
>>>>>>>>> [DoubleType, DateType] DoubleType StringType
>>>>>>>>> [DoubleType, TimestampType] DoubleType StringType
>>>>>>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>>>>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>>>>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>>>>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>>>>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>>>>>>>> [DateType, NullType] StringType DateType
>>>>>>>>> [DateType, IntegerType] IntegerType StringType
>>>>>>>>> [DateType, LongType] LongType StringType
>>>>>>>>> [DateType, DoubleType] DoubleType StringType
>>>>>>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>>>>>>> [DateType, TimestampType] DateType TimestampType
>>>>>>>>> [TimestampType, NullType] StringType TimestampType
>>>>>>>>> [TimestampType, IntegerType] IntegerType StringType
>>>>>>>>> [TimestampType, LongType] LongType StringType
>>>>>>>>> [TimestampType, DoubleType] DoubleType StringType
>>>>>>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> *Other possible suggestions**:*
>>>>>>>>>
>>>>>>>>> Probably, we could also consider simply making the merged type to
>>>>>>>>> string types when there are any type conflicts.
>>>>>>>>> Otherwise, there could be more stricter rules. I want more opinion
>>>>>>>>> from the community.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> *Questions:*
>>>>>>>>>
>>>>>>>>> - Does the *Proposal:* above looks good?
>>>>>>>>>
>>>>>>>>> - If not, what would be the alternative?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
>
> --
> Ryan Blue
> Software Engineer
> Netflix
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Ryan Blue <rb...@netflix.com.INVALID>.
That looks good to me, but it doesn’t cover decimal scales. One obvious
rule is to widen the scale to get a single type, if we don’t want to throw
an exception.
Also, if I follow this thread correctly, then the this is only used when
inferring the type of a column from a String, right? In that case, when
would Spark infer DoubleType instead of decimal?
rb
On Wed, Nov 15, 2017 at 4:42 AM, Hyukjin Kwon <gu...@gmail.com> wrote:
> Sorry, I meant:
>
> Before:
>
> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
> DateTypeTimestampTypeStringType
> *NullType* StringType IntegerType LongType StringType DoubleType
> StringType StringType StringType
> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
> IntegerType IntegerType StringType
> *LongType* LongType LongType LongType LongType DoubleType LongType
> LongType StringType
> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
> DoubleType DoubleType StringType
> *DateType* StringType IntegerType LongType DateType DoubleType DateType
> DateType StringType
> *TimestampType* StringType IntegerType LongType TimestampType DoubleType
> TimestampType TimestampType StringType
> *StringType* StringType StringType StringType StringType StringType
> StringType StringType StringType
>
> After:
> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
> DateTypeTimestampTypeStringType
> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
> DateType TimestampType StringType
> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
> DoubleType StringType StringType StringType
> *LongType* LongType LongType LongType DecimalType(38,0) StringType
> StringType StringType StringType
> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) DecimalType(38,0)
> DecimalType(38,0) StringType StringType StringType StringType
> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
> StringType StringType StringType
> *DateType* DateType StringType StringType StringType StringType DateType
> TimestampType StringType
> *TimestampType* TimestampType StringType StringType StringType StringType
> TimestampType TimestampType StringType
> *StringType* StringType StringType StringType StringType StringType
> StringType StringType StringType
>
>
> 2017-11-15 21:22 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>
>> If we do 1. and 2., I guess it will be:
>>
>>
>> Before:
>>
>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>> DateTypeTimestampTypeStringType
>> *NullType* StringType IntegerType LongType StringType DoubleType
>> StringType StringType StringType
>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>> IntegerType IntegerType StringType
>> *LongType* LongType LongType LongType LongType DoubleType LongType
>> LongType StringType
>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>> DoubleType DoubleType StringType
>> *DateType* StringType IntegerType LongType DateType DoubleType DateType
>> DateType StringType
>> *TimestampType* StringType IntegerType LongType TimestampType DoubleType
>> TimestampType TimestampType StringType
>> *StringType* StringType StringType StringType StringType StringType
>> StringType StringType StringType
>>
>> After:
>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>> DateTypeTimestampTypeStringType
>> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
>> DateType TimestampType StringType
>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>> DoubleType StringType StringType StringType
>> *LongType* LongType LongType LongType StringType StringType StringType
>> StringType StringType
>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) StringType
>> DecimalType(38,0) StringType StringType StringType StringType
>> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
>> StringType StringType StringType
>> *DateType* DateType StringType StringType StringType StringType DateType
>> TimestampType StringType
>> *TimestampType* TimestampType StringType StringType StringType StringType
>> TimestampType TimestampType StringType
>> *StringType* StringType StringType StringType StringType StringType
>> StringType StringType StringType
>>
>>
>> Sounds fine to me and looks we can stay safer from precision loss concern.
>>
>>
>>
>> 2017-11-15 19:15 GMT+09:00 Wenchen Fan <cl...@gmail.com>:
>>
>>> 1. shall we return NullType when merging 2 NullTypes?
>>> 2. when merging DoubleType and LongType/DecimalType, we should return
>>> StringType, otherwise precision lose may happen.
>>>
>>> One thing special for partition values type coercion is, we have a safe
>>> fallback: the StringType. This means we can be very conservative on type
>>> coercion, and reject any precision lose. While for normal type coercion,
>>> sometimes type coercion is better than failing the query.
>>>
>>> On Wed, Nov 15, 2017 at 8:50 AM, Hyukjin Kwon <gu...@gmail.com>
>>> wrote:
>>>
>>>> I just reordered a little bit and coloured differences to check easily:
>>>>
>>>> Before:
>>>>
>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>> DateTypeTimestampTypeStringType
>>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>>> StringType StringType StringType
>>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>>> IntegerType IntegerType StringType
>>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>>> LongType StringType
>>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>> DoubleType DoubleType StringType
>>>> *DateType* StringType IntegerType LongType DateType DoubleType DateType
>>>> DateType StringType
>>>> *TimestampType* StringType IntegerType LongType TimestampType
>>>> DoubleType TimestampType TimestampType StringType
>>>> *StringType* StringType StringType StringType StringType StringType
>>>> StringType StringType StringType
>>>> After:
>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>>> DateTypeTimestampTypeStringType
>>>> *NullType* StringType IntegerType LongType DecimalType(38,0) DoubleType
>>>> DateType TimestampType StringType
>>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>>> DoubleType StringType StringType StringType
>>>> *LongType* LongType LongType LongType DecimalType(38,0) DoubleType
>>>> StringType StringType StringType
>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>> DecimalType(38,0) DecimalType(38,0) DoubleType StringType StringType
>>>> StringType
>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>> StringType StringType StringType
>>>> *DateType* DateType StringType StringType StringType StringType
>>>> DateType TimestampType StringType
>>>> *TimestampType* TimestampType StringType StringType StringType
>>>> StringType TimestampType TimestampType StringType
>>>> *StringType* StringType StringType StringType StringType StringType
>>>> StringType StringType StringType
>>>>
>>>>
>>>>
>>>>
>>>> 2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>>
>>>>> Thanks all for feedback.
>>>>>
>>>>> > 1. when merging NullType with another type, the result should always
>>>>> be that type.
>>>>> > 2. when merging StringType with another type, the result should
>>>>> always be StringType.
>>>>> > 3. when merging integral types, the priority from high to low:
>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>> big integer when paring partition column values.
>>>>> > 4. DoubleType can't be merged with other types, except DoubleType
>>>>> itself.
>>>>> > 5. when merging TimestampType with DateType, return TimestampType.
>>>>>
>>>>> > Rather than a list, can we create a table? X-axis is data type, and
>>>>> Y-axis is also data type, and the intersection explains what the coerced
>>>>> type is?
>>>>>
>>>>>
>>>>> Here, I produced a table as below:
>>>>>
>>>>> *Before*
>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>>>>> DateTypeTimestampTypeStringType
>>>>> *NullType* StringType IntegerType LongType DoubleType StringType
>>>>> StringType StringType StringType
>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType IntegerType
>>>>> IntegerType IntegerType StringType
>>>>> *LongType* LongType LongType LongType DoubleType LongType LongType
>>>>> LongType StringType
>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>> DoubleType DoubleType StringType
>>>>> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
>>>>> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
>>>>> *DateType* StringType IntegerType LongType DoubleType DateType
>>>>> DateType DateType StringType
>>>>> *TimestampType* StringType IntegerType LongType DoubleType
>>>>> TimestampType TimestampType TimestampType StringType
>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>> StringType StringType StringType
>>>>>
>>>>> *After*
>>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>>>>> DateTypeTimestampTypeStringType
>>>>> *NullType* StringType IntegerType LongType DoubleType
>>>>> DecimalType(38,0) DateType TimestampType StringType
>>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>>> DecimalType(38,0) StringType StringType StringType
>>>>> *LongType* LongType LongType LongType DoubleType DecimalType(38,0)
>>>>> StringType StringType StringType
>>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>>> StringType StringType StringType
>>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>>> DecimalType(38,0) DoubleType DecimalType(38,0) StringType StringType
>>>>> StringType
>>>>> *DateType* DateType StringType StringType StringType StringType
>>>>> DateType TimestampType StringType
>>>>> *TimestampType* TimestampType StringType StringType StringType
>>>>> StringType TimestampType TimestampType StringType
>>>>> *StringType* StringType StringType StringType StringType StringType
>>>>> StringType StringType StringType
>>>>>
>>>>> Seems following Wenchen's comments (1. to 5.), and I also updated PR
>>>>> description there with some codes I used.
>>>>>
>>>>>
>>>>> > Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>> > Also, this shouldn't be isolated to partition column inference.
>>>>> > We should make sure most of the type coercions are consistent across
>>>>> different functionalities, with the caveat that we need to preserve
>>>>> backward compatibility.
>>>>>
>>>>>
>>>>> Sure, so, if I understood correctly, we preserve backward
>>>>> compatibility for improvements, but not for bugs in general.
>>>>>
>>>>>
>>>>> Probably, there are two things to be done now:
>>>>>
>>>>> - Deduplicates the type coercion logics, and fixes the obvious bugs,
>>>>> that doesn't make sense, (e.g., decimal and timestamp ends up with decimal).
>>>>>
>>>>> - Improves the deduplicated type coercion to follow other systems
>>>>> like Hive and Postgres, or to make it sounds coherent by referring them.
>>>>>
>>>>>
>>>>> I think my PR here focuses on the former, because the current
>>>>> partition column inference itself is already isolated
>>>>> and I am trying to propose to put those type coercion into one place
>>>>> in the TypeCoercion. We could also consider
>>>>> some divergence as exceptions in the nature of this functionality too,
>>>>> maybe .. ? (although I agree that most of the
>>>>> type coercions are consistent across different functionalities in
>>>>> general).
>>>>>
>>>>> If the deduplicated type coercion itself across functionalities should
>>>>> be changed as an improvement, it should preserve
>>>>> backward compatibility if I understood correctly and requires to take
>>>>> a look for other systems.
>>>>>
>>>>> I am willing to do the latter soon; however, it probably takes a quite
>>>>> while for me to investigate and propose a change.
>>>>>
>>>>> So, meanwhile, could we separately proceed this PR maybe or, probably
>>>>> would there be something I missed?
>>>>>
>>>>>
>>>>>
>>>>> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>>>>>
>>>>>> Most of those thoughts from Wenchen make sense to me.
>>>>>>
>>>>>>
>>>>>> Rather than a list, can we create a table? X-axis is data type, and
>>>>>> Y-axis is also data type, and the intersection explains what the coerced
>>>>>> type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>>
>>>>>>
>>>>>> Also, this shouldn't be isolated to partition column inference. We
>>>>>> should make sure most of the type coercions are consistent across different
>>>>>> functionalities, with the caveat that we need to preserve backward
>>>>>> compatibility.
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> My 2 cents:
>>>>>>>
>>>>>>> 1. when merging NullType with another type, the result should always
>>>>>>> be that type.
>>>>>>> 2. when merging StringType with another type, the result should
>>>>>>> always be StringType.
>>>>>>> 3. when merging integral types, the priority from high to low:
>>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>>> big integer when paring partition column values.
>>>>>>> 4. DoubleType can't be merged with other types, except DoubleType
>>>>>>> itself.
>>>>>>> 5. when merging TimestampType with DateType, return TimestampType.
>>>>>>>
>>>>>>>
>>>>>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gu...@gmail.com>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi dev,
>>>>>>>>
>>>>>>>> I would like to post a proposal about partitioned column type
>>>>>>>> inference (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>>>>>> configuration).
>>>>>>>>
>>>>>>>> This thread focuses on the type coercion (finding the common
>>>>>>>> type) in partitioned columns, in particular, when the different form of
>>>>>>>> data is inserted for the partition column and then it is read back with the
>>>>>>>> type inference.
>>>>>>>>
>>>>>>>>
>>>>>>>> *Problem:*
>>>>>>>>
>>>>>>>>
>>>>>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>>>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>>>>>> spark.read.load("/tmp/foo").printSchema()
>>>>>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>>>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>>>>>> spark.read.load("/tmp/bar").printSchema()
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> It currently returns:
>>>>>>>>
>>>>>>>>
>>>>>>>> root
>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>> |-- ts: date (nullable = true)
>>>>>>>>
>>>>>>>> root
>>>>>>>> |-- i: integer (nullable = true)
>>>>>>>> |-- decimal: integer (nullable = true)
>>>>>>>>
>>>>>>>>
>>>>>>>> The type coercion looks less well designed yet and currently there
>>>>>>>> are few holes which is not quite ideal:
>>>>>>>>
>>>>>>>>
>>>>>>>> private val upCastingOrder: Seq[DataType] =
>>>>>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>>>>>> ...
>>>>>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> The current way does not deal with when the types are outside of
>>>>>>>> the upCastingOrder. It just returns the first type, as the type coerced one.
>>>>>>>>
>>>>>>>> This has been being discussed in https://github.com/apache/s
>>>>>>>> park/pull/19389#discussion_r150426911, but I would like to have
>>>>>>>> more feedback from community as it possibly is a breaking change.
>>>>>>>>
>>>>>>>> For the current releases of Spark (2.2.0 <=), we support the types
>>>>>>>> below for partitioned column schema inference, given my investigation -
>>>>>>>> https://github.com/apache/spark/pull/19389#discussion_r150528207:
>>>>>>>>
>>>>>>>> NullType
>>>>>>>> IntegerType
>>>>>>>> LongType
>>>>>>>> DoubleType,
>>>>>>>> *DecimalType(...)
>>>>>>>> DateType
>>>>>>>> TimestampType
>>>>>>>> StringType
>>>>>>>>
>>>>>>>> *DecimalType only when it's bigger than LongType:
>>>>>>>>
>>>>>>>> I believe this is something we should definitely fix.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> *Proposal:*
>>>>>>>>
>>>>>>>> I propose the change - https://github.com/apache/spark/pull/19389
>>>>>>>>
>>>>>>>> Simply, it reuses the case 2 specified in
>>>>>>>> https://github.com/apache/spark/blob/6412ea1759d39a2380c5
>>>>>>>> 72ec24cfd8ae4f2d81f7/sql/catalyst/src/main/scala/org/apache/
>>>>>>>> spark/sql/catalyst/analysis/TypeCoercion.scala#L40-L43
>>>>>>>>
>>>>>>>> Please refer the chart I produced here -
>>>>>>>> https://github.com/apache/spark/pull/19389/files#r150528361. The
>>>>>>>> current proposal will brings the type coercion behaviour change in those
>>>>>>>> cases below:
>>>>>>>>
>>>>>>>>
>>>>>>>> Input typesOld output typeNew output type
>>>>>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>>>>>> [NullType, DateType] StringType DateType
>>>>>>>> [NullType, TimestampType] StringType TimestampType
>>>>>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>>>>>> [IntegerType, DateType] IntegerType StringType
>>>>>>>> [IntegerType, TimestampType] IntegerType StringType
>>>>>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>>>>>> [LongType, DateType] LongType StringType
>>>>>>>> [LongType, TimestampType] LongType StringType
>>>>>>>> [DoubleType, DateType] DoubleType StringType
>>>>>>>> [DoubleType, TimestampType] DoubleType StringType
>>>>>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>>>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>>>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>>>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>>>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>>>>>>> [DateType, NullType] StringType DateType
>>>>>>>> [DateType, IntegerType] IntegerType StringType
>>>>>>>> [DateType, LongType] LongType StringType
>>>>>>>> [DateType, DoubleType] DoubleType StringType
>>>>>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>>>>>> [DateType, TimestampType] DateType TimestampType
>>>>>>>> [TimestampType, NullType] StringType TimestampType
>>>>>>>> [TimestampType, IntegerType] IntegerType StringType
>>>>>>>> [TimestampType, LongType] LongType StringType
>>>>>>>> [TimestampType, DoubleType] DoubleType StringType
>>>>>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> *Other possible suggestions**:*
>>>>>>>>
>>>>>>>> Probably, we could also consider simply making the merged type to
>>>>>>>> string types when there are any type conflicts.
>>>>>>>> Otherwise, there could be more stricter rules. I want more opinion
>>>>>>>> from the community.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> *Questions:*
>>>>>>>>
>>>>>>>> - Does the *Proposal:* above looks good?
>>>>>>>>
>>>>>>>> - If not, what would be the alternative?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
--
Ryan Blue
Software Engineer
Netflix
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Hyukjin Kwon <gu...@gmail.com>.
Sorry, I meant:
Before:
InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
DateTypeTimestampTypeStringType
*NullType* StringType IntegerType LongType StringType DoubleType StringType
StringType StringType
*IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
IntegerType IntegerType StringType
*LongType* LongType LongType LongType LongType DoubleType LongType LongType
StringType
*DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
DoubleType DecimalType(38,0) DecimalType(38,0) StringType
*DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
DoubleType DoubleType StringType
*DateType* StringType IntegerType LongType DateType DoubleType DateType
DateType StringType
*TimestampType* StringType IntegerType LongType TimestampType DoubleType
TimestampType TimestampType StringType
*StringType* StringType StringType StringType StringType StringType
StringType StringType StringType
After:
InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
DateTypeTimestampTypeStringType
*NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
DateType TimestampType StringType
*IntegerType* IntegerType IntegerType LongType DecimalType(38,0) DoubleType
StringType StringType StringType
*LongType* LongType LongType LongType DecimalType(38,0) StringType
StringType StringType StringType
*DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) DecimalType(38,0)
DecimalType(38,0) StringType StringType StringType StringType
*DoubleType* DoubleType DoubleType StringType StringType DoubleType
StringType StringType StringType
*DateType* DateType StringType StringType StringType StringType DateType
TimestampType StringType
*TimestampType* TimestampType StringType StringType StringType StringType
TimestampType TimestampType StringType
*StringType* StringType StringType StringType StringType StringType
StringType StringType StringType
2017-11-15 21:22 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
> If we do 1. and 2., I guess it will be:
>
>
> Before:
>
> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
> DateTypeTimestampTypeStringType
> *NullType* StringType IntegerType LongType StringType DoubleType
> StringType StringType StringType
> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
> IntegerType IntegerType StringType
> *LongType* LongType LongType LongType LongType DoubleType LongType
> LongType StringType
> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
> DoubleType DoubleType StringType
> *DateType* StringType IntegerType LongType DateType DoubleType DateType
> DateType StringType
> *TimestampType* StringType IntegerType LongType TimestampType DoubleType
> TimestampType TimestampType StringType
> *StringType* StringType StringType StringType StringType StringType
> StringType StringType StringType
>
> After:
> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
> DateTypeTimestampTypeStringType
> *NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
> DateType TimestampType StringType
> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
> DoubleType StringType StringType StringType
> *LongType* LongType LongType LongType StringType StringType StringType
> StringType StringType
> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) StringType
> DecimalType(38,0) StringType StringType StringType StringType
> *DoubleType* DoubleType DoubleType StringType StringType DoubleType
> StringType StringType StringType
> *DateType* DateType StringType StringType StringType StringType DateType
> TimestampType StringType
> *TimestampType* TimestampType StringType StringType StringType StringType
> TimestampType TimestampType StringType
> *StringType* StringType StringType StringType StringType StringType
> StringType StringType StringType
>
>
> Sounds fine to me and looks we can stay safer from precision loss concern.
>
>
>
> 2017-11-15 19:15 GMT+09:00 Wenchen Fan <cl...@gmail.com>:
>
>> 1. shall we return NullType when merging 2 NullTypes?
>> 2. when merging DoubleType and LongType/DecimalType, we should return
>> StringType, otherwise precision lose may happen.
>>
>> One thing special for partition values type coercion is, we have a safe
>> fallback: the StringType. This means we can be very conservative on type
>> coercion, and reject any precision lose. While for normal type coercion,
>> sometimes type coercion is better than failing the query.
>>
>> On Wed, Nov 15, 2017 at 8:50 AM, Hyukjin Kwon <gu...@gmail.com>
>> wrote:
>>
>>> I just reordered a little bit and coloured differences to check easily:
>>>
>>> Before:
>>>
>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>> DateTypeTimestampTypeStringType
>>> *NullType* StringType IntegerType LongType StringType DoubleType
>>> StringType StringType StringType
>>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>>> IntegerType IntegerType StringType
>>> *LongType* LongType LongType LongType LongType DoubleType LongType
>>> LongType StringType
>>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>> DoubleType DoubleType StringType
>>> *DateType* StringType IntegerType LongType DateType DoubleType DateType
>>> DateType StringType
>>> *TimestampType* StringType IntegerType LongType TimestampType DoubleType
>>> TimestampType TimestampType StringType
>>> *StringType* StringType StringType StringType StringType StringType
>>> StringType StringType StringType
>>> After:
>>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>>> DateTypeTimestampTypeStringType
>>> *NullType* StringType IntegerType LongType DecimalType(38,0) DoubleType
>>> DateType TimestampType StringType
>>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>>> DoubleType StringType StringType StringType
>>> *LongType* LongType LongType LongType DecimalType(38,0) DoubleType
>>> StringType StringType StringType
>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>> DecimalType(38,0) DecimalType(38,0) DoubleType StringType StringType
>>> StringType
>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>> StringType StringType StringType
>>> *DateType* DateType StringType StringType StringType StringType DateType
>>> TimestampType StringType
>>> *TimestampType* TimestampType StringType StringType StringType
>>> StringType TimestampType TimestampType StringType
>>> *StringType* StringType StringType StringType StringType StringType
>>> StringType StringType StringType
>>>
>>>
>>>
>>>
>>> 2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>>
>>>> Thanks all for feedback.
>>>>
>>>> > 1. when merging NullType with another type, the result should always
>>>> be that type.
>>>> > 2. when merging StringType with another type, the result should
>>>> always be StringType.
>>>> > 3. when merging integral types, the priority from high to low:
>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>> big integer when paring partition column values.
>>>> > 4. DoubleType can't be merged with other types, except DoubleType
>>>> itself.
>>>> > 5. when merging TimestampType with DateType, return TimestampType.
>>>>
>>>> > Rather than a list, can we create a table? X-axis is data type, and
>>>> Y-axis is also data type, and the intersection explains what the coerced
>>>> type is?
>>>>
>>>>
>>>> Here, I produced a table as below:
>>>>
>>>> *Before*
>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>>>> DateTypeTimestampTypeStringType
>>>> *NullType* StringType IntegerType LongType DoubleType StringType
>>>> StringType StringType StringType
>>>> *IntegerType* IntegerType IntegerType LongType DoubleType IntegerType
>>>> IntegerType IntegerType StringType
>>>> *LongType* LongType LongType LongType DoubleType LongType LongType
>>>> LongType StringType
>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>> DoubleType DoubleType StringType
>>>> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
>>>> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
>>>> *DateType* StringType IntegerType LongType DoubleType DateType DateType
>>>> DateType StringType
>>>> *TimestampType* StringType IntegerType LongType DoubleType
>>>> TimestampType TimestampType TimestampType StringType
>>>> *StringType* StringType StringType StringType StringType StringType
>>>> StringType StringType StringType
>>>>
>>>> *After*
>>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>>>> DateTypeTimestampTypeStringType
>>>> *NullType* StringType IntegerType LongType DoubleType DecimalType(38,0)
>>>> DateType TimestampType StringType
>>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>>> DecimalType(38,0) StringType StringType StringType
>>>> *LongType* LongType LongType LongType DoubleType DecimalType(38,0)
>>>> StringType StringType StringType
>>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>>> StringType StringType StringType
>>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>>> DecimalType(38,0) DoubleType DecimalType(38,0) StringType StringType
>>>> StringType
>>>> *DateType* DateType StringType StringType StringType StringType
>>>> DateType TimestampType StringType
>>>> *TimestampType* TimestampType StringType StringType StringType
>>>> StringType TimestampType TimestampType StringType
>>>> *StringType* StringType StringType StringType StringType StringType
>>>> StringType StringType StringType
>>>>
>>>> Seems following Wenchen's comments (1. to 5.), and I also updated PR
>>>> description there with some codes I used.
>>>>
>>>>
>>>> > Can we also look at what Hive, standard SQL (Postgres?) do?
>>>> > Also, this shouldn't be isolated to partition column inference.
>>>> > We should make sure most of the type coercions are consistent across
>>>> different functionalities, with the caveat that we need to preserve
>>>> backward compatibility.
>>>>
>>>>
>>>> Sure, so, if I understood correctly, we preserve backward compatibility
>>>> for improvements, but not for bugs in general.
>>>>
>>>>
>>>> Probably, there are two things to be done now:
>>>>
>>>> - Deduplicates the type coercion logics, and fixes the obvious bugs,
>>>> that doesn't make sense, (e.g., decimal and timestamp ends up with decimal).
>>>>
>>>> - Improves the deduplicated type coercion to follow other systems
>>>> like Hive and Postgres, or to make it sounds coherent by referring them.
>>>>
>>>>
>>>> I think my PR here focuses on the former, because the current partition
>>>> column inference itself is already isolated
>>>> and I am trying to propose to put those type coercion into one place in
>>>> the TypeCoercion. We could also consider
>>>> some divergence as exceptions in the nature of this functionality too,
>>>> maybe .. ? (although I agree that most of the
>>>> type coercions are consistent across different functionalities in
>>>> general).
>>>>
>>>> If the deduplicated type coercion itself across functionalities should
>>>> be changed as an improvement, it should preserve
>>>> backward compatibility if I understood correctly and requires to take a
>>>> look for other systems.
>>>>
>>>> I am willing to do the latter soon; however, it probably takes a quite
>>>> while for me to investigate and propose a change.
>>>>
>>>> So, meanwhile, could we separately proceed this PR maybe or, probably
>>>> would there be something I missed?
>>>>
>>>>
>>>>
>>>> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>>>>
>>>>> Most of those thoughts from Wenchen make sense to me.
>>>>>
>>>>>
>>>>> Rather than a list, can we create a table? X-axis is data type, and
>>>>> Y-axis is also data type, and the intersection explains what the coerced
>>>>> type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>>
>>>>>
>>>>> Also, this shouldn't be isolated to partition column inference. We
>>>>> should make sure most of the type coercions are consistent across different
>>>>> functionalities, with the caveat that we need to preserve backward
>>>>> compatibility.
>>>>>
>>>>>
>>>>>
>>>>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> My 2 cents:
>>>>>>
>>>>>> 1. when merging NullType with another type, the result should always
>>>>>> be that type.
>>>>>> 2. when merging StringType with another type, the result should
>>>>>> always be StringType.
>>>>>> 3. when merging integral types, the priority from high to low:
>>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>>> big integer when paring partition column values.
>>>>>> 4. DoubleType can't be merged with other types, except DoubleType
>>>>>> itself.
>>>>>> 5. when merging TimestampType with DateType, return TimestampType.
>>>>>>
>>>>>>
>>>>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gu...@gmail.com>
>>>>>> wrote:
>>>>>>
>>>>>>> Hi dev,
>>>>>>>
>>>>>>> I would like to post a proposal about partitioned column type
>>>>>>> inference (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>>>>> configuration).
>>>>>>>
>>>>>>> This thread focuses on the type coercion (finding the common
>>>>>>> type) in partitioned columns, in particular, when the different form of
>>>>>>> data is inserted for the partition column and then it is read back with the
>>>>>>> type inference.
>>>>>>>
>>>>>>>
>>>>>>> *Problem:*
>>>>>>>
>>>>>>>
>>>>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>>>>> spark.read.load("/tmp/foo").printSchema()
>>>>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>>>>> spark.read.load("/tmp/bar").printSchema()
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> It currently returns:
>>>>>>>
>>>>>>>
>>>>>>> root
>>>>>>> |-- i: integer (nullable = true)
>>>>>>> |-- ts: date (nullable = true)
>>>>>>>
>>>>>>> root
>>>>>>> |-- i: integer (nullable = true)
>>>>>>> |-- decimal: integer (nullable = true)
>>>>>>>
>>>>>>>
>>>>>>> The type coercion looks less well designed yet and currently there
>>>>>>> are few holes which is not quite ideal:
>>>>>>>
>>>>>>>
>>>>>>> private val upCastingOrder: Seq[DataType] =
>>>>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>>>>> ...
>>>>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> The current way does not deal with when the types are outside of the
>>>>>>> upCastingOrder. It just returns the first type, as the type coerced one.
>>>>>>>
>>>>>>> This has been being discussed in https://github.com/apache/s
>>>>>>> park/pull/19389#discussion_r150426911, but I would like to have
>>>>>>> more feedback from community as it possibly is a breaking change.
>>>>>>>
>>>>>>> For the current releases of Spark (2.2.0 <=), we support the types
>>>>>>> below for partitioned column schema inference, given my investigation -
>>>>>>> https://github.com/apache/spark/pull/19389#discussion_r150528207:
>>>>>>>
>>>>>>> NullType
>>>>>>> IntegerType
>>>>>>> LongType
>>>>>>> DoubleType,
>>>>>>> *DecimalType(...)
>>>>>>> DateType
>>>>>>> TimestampType
>>>>>>> StringType
>>>>>>>
>>>>>>> *DecimalType only when it's bigger than LongType:
>>>>>>>
>>>>>>> I believe this is something we should definitely fix.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> *Proposal:*
>>>>>>>
>>>>>>> I propose the change - https://github.com/apache/spark/pull/19389
>>>>>>>
>>>>>>> Simply, it reuses the case 2 specified in
>>>>>>> https://github.com/apache/spark/blob/6412ea1759d39a2380c5
>>>>>>> 72ec24cfd8ae4f2d81f7/sql/catalyst/src/main/scala/org/apache/
>>>>>>> spark/sql/catalyst/analysis/TypeCoercion.scala#L40-L43
>>>>>>>
>>>>>>> Please refer the chart I produced here -
>>>>>>> https://github.com/apache/spark/pull/19389/files#r150528361. The
>>>>>>> current proposal will brings the type coercion behaviour change in those
>>>>>>> cases below:
>>>>>>>
>>>>>>>
>>>>>>> Input typesOld output typeNew output type
>>>>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>>>>> [NullType, DateType] StringType DateType
>>>>>>> [NullType, TimestampType] StringType TimestampType
>>>>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>>>>> [IntegerType, DateType] IntegerType StringType
>>>>>>> [IntegerType, TimestampType] IntegerType StringType
>>>>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>>>>> [LongType, DateType] LongType StringType
>>>>>>> [LongType, TimestampType] LongType StringType
>>>>>>> [DoubleType, DateType] DoubleType StringType
>>>>>>> [DoubleType, TimestampType] DoubleType StringType
>>>>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>>>>>> [DateType, NullType] StringType DateType
>>>>>>> [DateType, IntegerType] IntegerType StringType
>>>>>>> [DateType, LongType] LongType StringType
>>>>>>> [DateType, DoubleType] DoubleType StringType
>>>>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>>>>> [DateType, TimestampType] DateType TimestampType
>>>>>>> [TimestampType, NullType] StringType TimestampType
>>>>>>> [TimestampType, IntegerType] IntegerType StringType
>>>>>>> [TimestampType, LongType] LongType StringType
>>>>>>> [TimestampType, DoubleType] DoubleType StringType
>>>>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> *Other possible suggestions**:*
>>>>>>>
>>>>>>> Probably, we could also consider simply making the merged type to
>>>>>>> string types when there are any type conflicts.
>>>>>>> Otherwise, there could be more stricter rules. I want more opinion
>>>>>>> from the community.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> *Questions:*
>>>>>>>
>>>>>>> - Does the *Proposal:* above looks good?
>>>>>>>
>>>>>>> - If not, what would be the alternative?
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Hyukjin Kwon <gu...@gmail.com>.
If we do 1. and 2., I guess it will be:
Before:
InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
DateTypeTimestampTypeStringType
*NullType* StringType IntegerType LongType StringType DoubleType StringType
StringType StringType
*IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
IntegerType IntegerType StringType
*LongType* LongType LongType LongType LongType DoubleType LongType LongType
StringType
*DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
DoubleType DecimalType(38,0) DecimalType(38,0) StringType
*DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
DoubleType DoubleType StringType
*DateType* StringType IntegerType LongType DateType DoubleType DateType
DateType StringType
*TimestampType* StringType IntegerType LongType TimestampType DoubleType
TimestampType TimestampType StringType
*StringType* StringType StringType StringType StringType StringType
StringType StringType StringType
After:
InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
DateTypeTimestampTypeStringType
*NullType* NullType IntegerType LongType DecimalType(38,0) DoubleType
DateType TimestampType StringType
*IntegerType* IntegerType IntegerType LongType DecimalType(38,0) DoubleType
StringType StringType StringType
*LongType* LongType LongType LongType StringType StringType StringType
StringType StringType
*DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) StringType
DecimalType(38,0) StringType StringType StringType StringType
*DoubleType* DoubleType DoubleType StringType StringType DoubleType
StringType StringType StringType
*DateType* DateType StringType StringType StringType StringType DateType
TimestampType StringType
*TimestampType* TimestampType StringType StringType StringType StringType
TimestampType TimestampType StringType
*StringType* StringType StringType StringType StringType StringType
StringType StringType StringType
Sounds fine to me and looks we can stay safer from precision loss concern.
2017-11-15 19:15 GMT+09:00 Wenchen Fan <cl...@gmail.com>:
> 1. shall we return NullType when merging 2 NullTypes?
> 2. when merging DoubleType and LongType/DecimalType, we should return
> StringType, otherwise precision lose may happen.
>
> One thing special for partition values type coercion is, we have a safe
> fallback: the StringType. This means we can be very conservative on type
> coercion, and reject any precision lose. While for normal type coercion,
> sometimes type coercion is better than failing the query.
>
> On Wed, Nov 15, 2017 at 8:50 AM, Hyukjin Kwon <gu...@gmail.com> wrote:
>
>> I just reordered a little bit and coloured differences to check easily:
>>
>> Before:
>>
>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>> DateTypeTimestampTypeStringType
>> *NullType* StringType IntegerType LongType StringType DoubleType
>> StringType StringType StringType
>> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
>> IntegerType IntegerType StringType
>> *LongType* LongType LongType LongType LongType DoubleType LongType
>> LongType StringType
>> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
>> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>> DoubleType DoubleType StringType
>> *DateType* StringType IntegerType LongType DateType DoubleType DateType
>> DateType StringType
>> *TimestampType* StringType IntegerType LongType TimestampType DoubleType
>> TimestampType TimestampType StringType
>> *StringType* StringType StringType StringType StringType StringType
>> StringType StringType StringType
>> After:
>> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
>> DateTypeTimestampTypeStringType
>> *NullType* StringType IntegerType LongType DecimalType(38,0) DoubleType
>> DateType TimestampType StringType
>> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
>> DoubleType StringType StringType StringType
>> *LongType* LongType LongType LongType DecimalType(38,0) DoubleType
>> StringType StringType StringType
>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) DecimalType(38,0)
>> DecimalType(38,0) DoubleType StringType StringType StringType
>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>> StringType StringType StringType
>> *DateType* DateType StringType StringType StringType StringType DateType
>> TimestampType StringType
>> *TimestampType* TimestampType StringType StringType StringType StringType
>> TimestampType TimestampType StringType
>> *StringType* StringType StringType StringType StringType StringType
>> StringType StringType StringType
>>
>>
>>
>>
>> 2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>>
>>> Thanks all for feedback.
>>>
>>> > 1. when merging NullType with another type, the result should always
>>> be that type.
>>> > 2. when merging StringType with another type, the result should always
>>> be StringType.
>>> > 3. when merging integral types, the priority from high to low:
>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>> big integer when paring partition column values.
>>> > 4. DoubleType can't be merged with other types, except DoubleType
>>> itself.
>>> > 5. when merging TimestampType with DateType, return TimestampType.
>>>
>>> > Rather than a list, can we create a table? X-axis is data type, and
>>> Y-axis is also data type, and the intersection explains what the coerced
>>> type is?
>>>
>>>
>>> Here, I produced a table as below:
>>>
>>> *Before*
>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>>> DateTypeTimestampTypeStringType
>>> *NullType* StringType IntegerType LongType DoubleType StringType
>>> StringType StringType StringType
>>> *IntegerType* IntegerType IntegerType LongType DoubleType IntegerType
>>> IntegerType IntegerType StringType
>>> *LongType* LongType LongType LongType DoubleType LongType LongType
>>> LongType StringType
>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>> DoubleType DoubleType StringType
>>> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
>>> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
>>> *DateType* StringType IntegerType LongType DoubleType DateType DateType
>>> DateType StringType
>>> *TimestampType* StringType IntegerType LongType DoubleType TimestampType
>>> TimestampType TimestampType StringType
>>> *StringType* StringType StringType StringType StringType StringType
>>> StringType StringType StringType
>>>
>>> *After*
>>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>>> DateTypeTimestampTypeStringType
>>> *NullType* StringType IntegerType LongType DoubleType DecimalType(38,0)
>>> DateType TimestampType StringType
>>> *IntegerType* IntegerType IntegerType LongType DoubleType
>>> DecimalType(38,0) StringType StringType StringType
>>> *LongType* LongType LongType LongType DoubleType DecimalType(38,0)
>>> StringType StringType StringType
>>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>>> StringType StringType StringType
>>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0)
>>> DecimalType(38,0) DoubleType DecimalType(38,0) StringType StringType
>>> StringType
>>> *DateType* DateType StringType StringType StringType StringType DateType
>>> TimestampType StringType
>>> *TimestampType* TimestampType StringType StringType StringType
>>> StringType TimestampType TimestampType StringType
>>> *StringType* StringType StringType StringType StringType StringType
>>> StringType StringType StringType
>>>
>>> Seems following Wenchen's comments (1. to 5.), and I also updated PR
>>> description there with some codes I used.
>>>
>>>
>>> > Can we also look at what Hive, standard SQL (Postgres?) do?
>>> > Also, this shouldn't be isolated to partition column inference.
>>> > We should make sure most of the type coercions are consistent across
>>> different functionalities, with the caveat that we need to preserve
>>> backward compatibility.
>>>
>>>
>>> Sure, so, if I understood correctly, we preserve backward compatibility
>>> for improvements, but not for bugs in general.
>>>
>>>
>>> Probably, there are two things to be done now:
>>>
>>> - Deduplicates the type coercion logics, and fixes the obvious bugs,
>>> that doesn't make sense, (e.g., decimal and timestamp ends up with decimal).
>>>
>>> - Improves the deduplicated type coercion to follow other systems like
>>> Hive and Postgres, or to make it sounds coherent by referring them.
>>>
>>>
>>> I think my PR here focuses on the former, because the current partition
>>> column inference itself is already isolated
>>> and I am trying to propose to put those type coercion into one place in
>>> the TypeCoercion. We could also consider
>>> some divergence as exceptions in the nature of this functionality too,
>>> maybe .. ? (although I agree that most of the
>>> type coercions are consistent across different functionalities in
>>> general).
>>>
>>> If the deduplicated type coercion itself across functionalities should
>>> be changed as an improvement, it should preserve
>>> backward compatibility if I understood correctly and requires to take a
>>> look for other systems.
>>>
>>> I am willing to do the latter soon; however, it probably takes a quite
>>> while for me to investigate and propose a change.
>>>
>>> So, meanwhile, could we separately proceed this PR maybe or, probably
>>> would there be something I missed?
>>>
>>>
>>>
>>> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>>>
>>>> Most of those thoughts from Wenchen make sense to me.
>>>>
>>>>
>>>> Rather than a list, can we create a table? X-axis is data type, and
>>>> Y-axis is also data type, and the intersection explains what the coerced
>>>> type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>>>
>>>>
>>>> Also, this shouldn't be isolated to partition column inference. We
>>>> should make sure most of the type coercions are consistent across different
>>>> functionalities, with the caveat that we need to preserve backward
>>>> compatibility.
>>>>
>>>>
>>>>
>>>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com>
>>>> wrote:
>>>>
>>>>> My 2 cents:
>>>>>
>>>>> 1. when merging NullType with another type, the result should always
>>>>> be that type.
>>>>> 2. when merging StringType with another type, the result should always
>>>>> be StringType.
>>>>> 3. when merging integral types, the priority from high to low:
>>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>>> big integer when paring partition column values.
>>>>> 4. DoubleType can't be merged with other types, except DoubleType
>>>>> itself.
>>>>> 5. when merging TimestampType with DateType, return TimestampType.
>>>>>
>>>>>
>>>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gu...@gmail.com>
>>>>> wrote:
>>>>>
>>>>>> Hi dev,
>>>>>>
>>>>>> I would like to post a proposal about partitioned column type
>>>>>> inference (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>>>> configuration).
>>>>>>
>>>>>> This thread focuses on the type coercion (finding the common type) in
>>>>>> partitioned columns, in particular, when the different form of data is
>>>>>> inserted for the partition column and then it is read back with the type
>>>>>> inference.
>>>>>>
>>>>>>
>>>>>> *Problem:*
>>>>>>
>>>>>>
>>>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>>>> spark.read.load("/tmp/foo").printSchema()
>>>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>>>> spark.read.load("/tmp/bar").printSchema()
>>>>>>
>>>>>>
>>>>>>
>>>>>> It currently returns:
>>>>>>
>>>>>>
>>>>>> root
>>>>>> |-- i: integer (nullable = true)
>>>>>> |-- ts: date (nullable = true)
>>>>>>
>>>>>> root
>>>>>> |-- i: integer (nullable = true)
>>>>>> |-- decimal: integer (nullable = true)
>>>>>>
>>>>>>
>>>>>> The type coercion looks less well designed yet and currently there
>>>>>> are few holes which is not quite ideal:
>>>>>>
>>>>>>
>>>>>> private val upCastingOrder: Seq[DataType] =
>>>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>>>> ...
>>>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>>>
>>>>>>
>>>>>>
>>>>>> The current way does not deal with when the types are outside of the
>>>>>> upCastingOrder. It just returns the first type, as the type coerced one.
>>>>>>
>>>>>> This has been being discussed in https://github.com/apache/s
>>>>>> park/pull/19389#discussion_r150426911, but I would like to have more
>>>>>> feedback from community as it possibly is a breaking change.
>>>>>>
>>>>>> For the current releases of Spark (2.2.0 <=), we support the types
>>>>>> below for partitioned column schema inference, given my investigation -
>>>>>> https://github.com/apache/spark/pull/19389#discussion_r150528207:
>>>>>>
>>>>>> NullType
>>>>>> IntegerType
>>>>>> LongType
>>>>>> DoubleType,
>>>>>> *DecimalType(...)
>>>>>> DateType
>>>>>> TimestampType
>>>>>> StringType
>>>>>>
>>>>>> *DecimalType only when it's bigger than LongType:
>>>>>>
>>>>>> I believe this is something we should definitely fix.
>>>>>>
>>>>>>
>>>>>>
>>>>>> *Proposal:*
>>>>>>
>>>>>> I propose the change - https://github.com/apache/spark/pull/19389
>>>>>>
>>>>>> Simply, it reuses the case 2 specified in https://github.com/apache/s
>>>>>> park/blob/6412ea1759d39a2380c572ec24cfd8ae4f2d81f7/sql/catal
>>>>>> yst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Ty
>>>>>> peCoercion.scala#L40-L43
>>>>>>
>>>>>> Please refer the chart I produced here - https://github.com/apache/sp
>>>>>> ark/pull/19389/files#r150528361. The current proposal will brings
>>>>>> the type coercion behaviour change in those cases below:
>>>>>>
>>>>>>
>>>>>> Input typesOld output typeNew output type
>>>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>>>> [NullType, DateType] StringType DateType
>>>>>> [NullType, TimestampType] StringType TimestampType
>>>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>>>> [IntegerType, DateType] IntegerType StringType
>>>>>> [IntegerType, TimestampType] IntegerType StringType
>>>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>>>> [LongType, DateType] LongType StringType
>>>>>> [LongType, TimestampType] LongType StringType
>>>>>> [DoubleType, DateType] DoubleType StringType
>>>>>> [DoubleType, TimestampType] DoubleType StringType
>>>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>>>>> [DateType, NullType] StringType DateType
>>>>>> [DateType, IntegerType] IntegerType StringType
>>>>>> [DateType, LongType] LongType StringType
>>>>>> [DateType, DoubleType] DoubleType StringType
>>>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>>>> [DateType, TimestampType] DateType TimestampType
>>>>>> [TimestampType, NullType] StringType TimestampType
>>>>>> [TimestampType, IntegerType] IntegerType StringType
>>>>>> [TimestampType, LongType] LongType StringType
>>>>>> [TimestampType, DoubleType] DoubleType StringType
>>>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>>>
>>>>>>
>>>>>>
>>>>>> *Other possible suggestions**:*
>>>>>>
>>>>>> Probably, we could also consider simply making the merged type to
>>>>>> string types when there are any type conflicts.
>>>>>> Otherwise, there could be more stricter rules. I want more opinion
>>>>>> from the community.
>>>>>>
>>>>>>
>>>>>>
>>>>>> *Questions:*
>>>>>>
>>>>>> - Does the *Proposal:* above looks good?
>>>>>>
>>>>>> - If not, what would be the alternative?
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Wenchen Fan <cl...@gmail.com>.
1. shall we return NullType when merging 2 NullTypes?
2. when merging DoubleType and LongType/DecimalType, we should return
StringType, otherwise precision lose may happen.
One thing special for partition values type coercion is, we have a safe
fallback: the StringType. This means we can be very conservative on type
coercion, and reject any precision lose. While for normal type coercion,
sometimes type coercion is better than failing the query.
On Wed, Nov 15, 2017 at 8:50 AM, Hyukjin Kwon <gu...@gmail.com> wrote:
> I just reordered a little bit and coloured differences to check easily:
>
> Before:
>
> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
> DateTypeTimestampTypeStringType
> *NullType* StringType IntegerType LongType StringType DoubleType
> StringType StringType StringType
> *IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
> IntegerType IntegerType StringType
> *LongType* LongType LongType LongType LongType DoubleType LongType
> LongType StringType
> *DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
> DoubleType DecimalType(38,0) DecimalType(38,0) StringType
> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
> DoubleType DoubleType StringType
> *DateType* StringType IntegerType LongType DateType DoubleType DateType
> DateType StringType
> *TimestampType* StringType IntegerType LongType TimestampType DoubleType
> TimestampType TimestampType StringType
> *StringType* StringType StringType StringType StringType StringType
> StringType StringType StringType
> After:
> InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
> DateTypeTimestampTypeStringType
> *NullType* StringType IntegerType LongType DecimalType(38,0) DoubleType
> DateType TimestampType StringType
> *IntegerType* IntegerType IntegerType LongType DecimalType(38,0)
> DoubleType StringType StringType StringType
> *LongType* LongType LongType LongType DecimalType(38,0) DoubleType
> StringType StringType StringType
> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) DecimalType(38,0)
> DecimalType(38,0) DoubleType StringType StringType StringType
> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
> StringType StringType StringType
> *DateType* DateType StringType StringType StringType StringType DateType
> TimestampType StringType
> *TimestampType* TimestampType StringType StringType StringType StringType
> TimestampType TimestampType StringType
> *StringType* StringType StringType StringType StringType StringType
> StringType StringType StringType
>
>
>
>
> 2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
>
>> Thanks all for feedback.
>>
>> > 1. when merging NullType with another type, the result should always be
>> that type.
>> > 2. when merging StringType with another type, the result should always
>> be StringType.
>> > 3. when merging integral types, the priority from high to low:
>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>> big integer when paring partition column values.
>> > 4. DoubleType can't be merged with other types, except DoubleType
>> itself.
>> > 5. when merging TimestampType with DateType, return TimestampType.
>>
>> > Rather than a list, can we create a table? X-axis is data type, and
>> Y-axis is also data type, and the intersection explains what the coerced
>> type is?
>>
>>
>> Here, I produced a table as below:
>>
>> *Before*
>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>> DateTypeTimestampTypeStringType
>> *NullType* StringType IntegerType LongType DoubleType StringType
>> StringType StringType StringType
>> *IntegerType* IntegerType IntegerType LongType DoubleType IntegerType
>> IntegerType IntegerType StringType
>> *LongType* LongType LongType LongType DoubleType LongType LongType
>> LongType StringType
>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>> DoubleType DoubleType StringType
>> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
>> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
>> *DateType* StringType IntegerType LongType DoubleType DateType DateType
>> DateType StringType
>> *TimestampType* StringType IntegerType LongType DoubleType TimestampType
>> TimestampType TimestampType StringType
>> *StringType* StringType StringType StringType StringType StringType
>> StringType StringType StringType
>>
>> *After*
>> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
>> DateTypeTimestampTypeStringType
>> *NullType* StringType IntegerType LongType DoubleType DecimalType(38,0)
>> DateType TimestampType StringType
>> *IntegerType* IntegerType IntegerType LongType DoubleType
>> DecimalType(38,0) StringType StringType StringType
>> *LongType* LongType LongType LongType DoubleType DecimalType(38,0)
>> StringType StringType StringType
>> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
>> StringType StringType StringType
>> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) DecimalType(38,0)
>> DoubleType DecimalType(38,0) StringType StringType StringType
>> *DateType* DateType StringType StringType StringType StringType DateType
>> TimestampType StringType
>> *TimestampType* TimestampType StringType StringType StringType StringType
>> TimestampType TimestampType StringType
>> *StringType* StringType StringType StringType StringType StringType
>> StringType StringType StringType
>>
>> Seems following Wenchen's comments (1. to 5.), and I also updated PR
>> description there with some codes I used.
>>
>>
>> > Can we also look at what Hive, standard SQL (Postgres?) do?
>> > Also, this shouldn't be isolated to partition column inference.
>> > We should make sure most of the type coercions are consistent across
>> different functionalities, with the caveat that we need to preserve
>> backward compatibility.
>>
>>
>> Sure, so, if I understood correctly, we preserve backward compatibility
>> for improvements, but not for bugs in general.
>>
>>
>> Probably, there are two things to be done now:
>>
>> - Deduplicates the type coercion logics, and fixes the obvious bugs,
>> that doesn't make sense, (e.g., decimal and timestamp ends up with decimal).
>>
>> - Improves the deduplicated type coercion to follow other systems like
>> Hive and Postgres, or to make it sounds coherent by referring them.
>>
>>
>> I think my PR here focuses on the former, because the current partition
>> column inference itself is already isolated
>> and I am trying to propose to put those type coercion into one place in
>> the TypeCoercion. We could also consider
>> some divergence as exceptions in the nature of this functionality too,
>> maybe .. ? (although I agree that most of the
>> type coercions are consistent across different functionalities in
>> general).
>>
>> If the deduplicated type coercion itself across functionalities should be
>> changed as an improvement, it should preserve
>> backward compatibility if I understood correctly and requires to take a
>> look for other systems.
>>
>> I am willing to do the latter soon; however, it probably takes a quite
>> while for me to investigate and propose a change.
>>
>> So, meanwhile, could we separately proceed this PR maybe or, probably
>> would there be something I missed?
>>
>>
>>
>> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>>
>>> Most of those thoughts from Wenchen make sense to me.
>>>
>>>
>>> Rather than a list, can we create a table? X-axis is data type, and
>>> Y-axis is also data type, and the intersection explains what the coerced
>>> type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>>
>>>
>>> Also, this shouldn't be isolated to partition column inference. We
>>> should make sure most of the type coercions are consistent across different
>>> functionalities, with the caveat that we need to preserve backward
>>> compatibility.
>>>
>>>
>>>
>>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com>
>>> wrote:
>>>
>>>> My 2 cents:
>>>>
>>>> 1. when merging NullType with another type, the result should always be
>>>> that type.
>>>> 2. when merging StringType with another type, the result should always
>>>> be StringType.
>>>> 3. when merging integral types, the priority from high to low:
>>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>>> big integer when paring partition column values.
>>>> 4. DoubleType can't be merged with other types, except DoubleType
>>>> itself.
>>>> 5. when merging TimestampType with DateType, return TimestampType.
>>>>
>>>>
>>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gu...@gmail.com>
>>>> wrote:
>>>>
>>>>> Hi dev,
>>>>>
>>>>> I would like to post a proposal about partitioned column type
>>>>> inference (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>>> configuration).
>>>>>
>>>>> This thread focuses on the type coercion (finding the common type) in
>>>>> partitioned columns, in particular, when the different form of data is
>>>>> inserted for the partition column and then it is read back with the type
>>>>> inference.
>>>>>
>>>>>
>>>>> *Problem:*
>>>>>
>>>>>
>>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>>> spark.read.load("/tmp/foo").printSchema()
>>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>>> spark.read.load("/tmp/bar").printSchema()
>>>>>
>>>>>
>>>>>
>>>>> It currently returns:
>>>>>
>>>>>
>>>>> root
>>>>> |-- i: integer (nullable = true)
>>>>> |-- ts: date (nullable = true)
>>>>>
>>>>> root
>>>>> |-- i: integer (nullable = true)
>>>>> |-- decimal: integer (nullable = true)
>>>>>
>>>>>
>>>>> The type coercion looks less well designed yet and currently there are
>>>>> few holes which is not quite ideal:
>>>>>
>>>>>
>>>>> private val upCastingOrder: Seq[DataType] =
>>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>>> ...
>>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>>
>>>>>
>>>>>
>>>>> The current way does not deal with when the types are outside of the
>>>>> upCastingOrder. It just returns the first type, as the type coerced one.
>>>>>
>>>>> This has been being discussed in https://github.com/apache/s
>>>>> park/pull/19389#discussion_r150426911, but I would like to have more
>>>>> feedback from community as it possibly is a breaking change.
>>>>>
>>>>> For the current releases of Spark (2.2.0 <=), we support the types
>>>>> below for partitioned column schema inference, given my investigation -
>>>>> https://github.com/apache/spark/pull/19389#discussion_r150528207:
>>>>>
>>>>> NullType
>>>>> IntegerType
>>>>> LongType
>>>>> DoubleType,
>>>>> *DecimalType(...)
>>>>> DateType
>>>>> TimestampType
>>>>> StringType
>>>>>
>>>>> *DecimalType only when it's bigger than LongType:
>>>>>
>>>>> I believe this is something we should definitely fix.
>>>>>
>>>>>
>>>>>
>>>>> *Proposal:*
>>>>>
>>>>> I propose the change - https://github.com/apache/spark/pull/19389
>>>>>
>>>>> Simply, it reuses the case 2 specified in https://github.com/apache/s
>>>>> park/blob/6412ea1759d39a2380c572ec24cfd8ae4f2d81f7/sql/catal
>>>>> yst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Ty
>>>>> peCoercion.scala#L40-L43
>>>>>
>>>>> Please refer the chart I produced here - https://github.com/apache/sp
>>>>> ark/pull/19389/files#r150528361. The current proposal will brings the
>>>>> type coercion behaviour change in those cases below:
>>>>>
>>>>>
>>>>> Input typesOld output typeNew output type
>>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>>> [NullType, DateType] StringType DateType
>>>>> [NullType, TimestampType] StringType TimestampType
>>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>>> [IntegerType, DateType] IntegerType StringType
>>>>> [IntegerType, TimestampType] IntegerType StringType
>>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>>> [LongType, DateType] LongType StringType
>>>>> [LongType, TimestampType] LongType StringType
>>>>> [DoubleType, DateType] DoubleType StringType
>>>>> [DoubleType, TimestampType] DoubleType StringType
>>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>>>> [DateType, NullType] StringType DateType
>>>>> [DateType, IntegerType] IntegerType StringType
>>>>> [DateType, LongType] LongType StringType
>>>>> [DateType, DoubleType] DoubleType StringType
>>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>>> [DateType, TimestampType] DateType TimestampType
>>>>> [TimestampType, NullType] StringType TimestampType
>>>>> [TimestampType, IntegerType] IntegerType StringType
>>>>> [TimestampType, LongType] LongType StringType
>>>>> [TimestampType, DoubleType] DoubleType StringType
>>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>>
>>>>>
>>>>>
>>>>> *Other possible suggestions**:*
>>>>>
>>>>> Probably, we could also consider simply making the merged type to
>>>>> string types when there are any type conflicts.
>>>>> Otherwise, there could be more stricter rules. I want more opinion
>>>>> from the community.
>>>>>
>>>>>
>>>>>
>>>>> *Questions:*
>>>>>
>>>>> - Does the *Proposal:* above looks good?
>>>>>
>>>>> - If not, what would be the alternative?
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Hyukjin Kwon <gu...@gmail.com>.
I just reordered a little bit and coloured differences to check easily:
Before:
InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
DateTypeTimestampTypeStringType
*NullType* StringType IntegerType LongType StringType DoubleType StringType
StringType StringType
*IntegerType* IntegerType IntegerType LongType IntegerType DoubleType
IntegerType IntegerType StringType
*LongType* LongType LongType LongType LongType DoubleType LongType LongType
StringType
*DecimalType(38,0)* StringType IntegerType LongType DecimalType(38,0)
DoubleType DecimalType(38,0) DecimalType(38,0) StringType
*DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
DoubleType DoubleType StringType
*DateType* StringType IntegerType LongType DateType DoubleType DateType
DateType StringType
*TimestampType* StringType IntegerType LongType TimestampType DoubleType
TimestampType TimestampType StringType
*StringType* StringType StringType StringType StringType StringType
StringType StringType StringType
After:
InputA \ InputBNullTypeIntegerTypeLongTypeDecimalType(38,0)DoubleType
DateTypeTimestampTypeStringType
*NullType* StringType IntegerType LongType DecimalType(38,0) DoubleType
DateType TimestampType StringType
*IntegerType* IntegerType IntegerType LongType DecimalType(38,0) DoubleType
StringType StringType StringType
*LongType* LongType LongType LongType DecimalType(38,0) DoubleType
StringType StringType StringType
*DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) DecimalType(38,0)
DecimalType(38,0) DoubleType StringType StringType StringType
*DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
StringType StringType StringType
*DateType* DateType StringType StringType StringType StringType DateType
TimestampType StringType
*TimestampType* TimestampType StringType StringType StringType StringType
TimestampType TimestampType StringType
*StringType* StringType StringType StringType StringType StringType
StringType StringType StringType
2017-11-15 11:15 GMT+09:00 Hyukjin Kwon <gu...@gmail.com>:
> Thanks all for feedback.
>
> > 1. when merging NullType with another type, the result should always be
> that type.
> > 2. when merging StringType with another type, the result should always
> be StringType.
> > 3. when merging integral types, the priority from high to low:
> DecimalType, LongType, IntegerType. This is because DecimalType is used as
> big integer when paring partition column values.
> > 4. DoubleType can't be merged with other types, except DoubleType itself.
> > 5. when merging TimestampType with DateType, return TimestampType.
>
> > Rather than a list, can we create a table? X-axis is data type, and
> Y-axis is also data type, and the intersection explains what the coerced
> type is?
>
>
> Here, I produced a table as below:
>
> *Before*
> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
> DateTypeTimestampTypeStringType
> *NullType* StringType IntegerType LongType DoubleType StringType
> StringType StringType StringType
> *IntegerType* IntegerType IntegerType LongType DoubleType IntegerType
> IntegerType IntegerType StringType
> *LongType* LongType LongType LongType DoubleType LongType LongType
> LongType StringType
> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
> DoubleType DoubleType StringType
> *DecimalType(38,0)* StringType IntegerType LongType DoubleType
> DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
> *DateType* StringType IntegerType LongType DoubleType DateType DateType
> DateType StringType
> *TimestampType* StringType IntegerType LongType DoubleType TimestampType
> TimestampType TimestampType StringType
> *StringType* StringType StringType StringType StringType StringType
> StringType StringType StringType
>
> *After*
> InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
> DateTypeTimestampTypeStringType
> *NullType* StringType IntegerType LongType DoubleType DecimalType(38,0)
> DateType TimestampType StringType
> *IntegerType* IntegerType IntegerType LongType DoubleType
> DecimalType(38,0) StringType StringType StringType
> *LongType* LongType LongType LongType DoubleType DecimalType(38,0)
> StringType StringType StringType
> *DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
> StringType StringType StringType
> *DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) DecimalType(38,0)
> DoubleType DecimalType(38,0) StringType StringType StringType
> *DateType* DateType StringType StringType StringType StringType DateType
> TimestampType StringType
> *TimestampType* TimestampType StringType StringType StringType StringType
> TimestampType TimestampType StringType
> *StringType* StringType StringType StringType StringType StringType
> StringType StringType StringType
>
> Seems following Wenchen's comments (1. to 5.), and I also updated PR
> description there with some codes I used.
>
>
> > Can we also look at what Hive, standard SQL (Postgres?) do?
> > Also, this shouldn't be isolated to partition column inference.
> > We should make sure most of the type coercions are consistent across
> different functionalities, with the caveat that we need to preserve
> backward compatibility.
>
>
> Sure, so, if I understood correctly, we preserve backward compatibility
> for improvements, but not for bugs in general.
>
>
> Probably, there are two things to be done now:
>
> - Deduplicates the type coercion logics, and fixes the obvious bugs,
> that doesn't make sense, (e.g., decimal and timestamp ends up with decimal).
>
> - Improves the deduplicated type coercion to follow other systems like
> Hive and Postgres, or to make it sounds coherent by referring them.
>
>
> I think my PR here focuses on the former, because the current partition
> column inference itself is already isolated
> and I am trying to propose to put those type coercion into one place in
> the TypeCoercion. We could also consider
> some divergence as exceptions in the nature of this functionality too,
> maybe .. ? (although I agree that most of the
> type coercions are consistent across different functionalities in general).
>
> If the deduplicated type coercion itself across functionalities should be
> changed as an improvement, it should preserve
> backward compatibility if I understood correctly and requires to take a
> look for other systems.
>
> I am willing to do the latter soon; however, it probably takes a quite
> while for me to investigate and propose a change.
>
> So, meanwhile, could we separately proceed this PR maybe or, probably
> would there be something I missed?
>
>
>
> 2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
>
>> Most of those thoughts from Wenchen make sense to me.
>>
>>
>> Rather than a list, can we create a table? X-axis is data type, and
>> Y-axis is also data type, and the intersection explains what the coerced
>> type is? Can we also look at what Hive, standard SQL (Postgres?) do?
>>
>>
>> Also, this shouldn't be isolated to partition column inference. We should
>> make sure most of the type coercions are consistent across different
>> functionalities, with the caveat that we need to preserve backward
>> compatibility.
>>
>>
>>
>> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com> wrote:
>>
>>> My 2 cents:
>>>
>>> 1. when merging NullType with another type, the result should always be
>>> that type.
>>> 2. when merging StringType with another type, the result should always
>>> be StringType.
>>> 3. when merging integral types, the priority from high to low:
>>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>>> big integer when paring partition column values.
>>> 4. DoubleType can't be merged with other types, except DoubleType itself.
>>> 5. when merging TimestampType with DateType, return TimestampType.
>>>
>>>
>>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gu...@gmail.com>
>>> wrote:
>>>
>>>> Hi dev,
>>>>
>>>> I would like to post a proposal about partitioned column type inference
>>>> (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>>> configuration).
>>>>
>>>> This thread focuses on the type coercion (finding the common type) in
>>>> partitioned columns, in particular, when the different form of data is
>>>> inserted for the partition column and then it is read back with the type
>>>> inference.
>>>>
>>>>
>>>> *Problem:*
>>>>
>>>>
>>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>>> spark.read.load("/tmp/foo").printSchema()
>>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>>> spark.read.load("/tmp/bar").printSchema()
>>>>
>>>>
>>>>
>>>> It currently returns:
>>>>
>>>>
>>>> root
>>>> |-- i: integer (nullable = true)
>>>> |-- ts: date (nullable = true)
>>>>
>>>> root
>>>> |-- i: integer (nullable = true)
>>>> |-- decimal: integer (nullable = true)
>>>>
>>>>
>>>> The type coercion looks less well designed yet and currently there are
>>>> few holes which is not quite ideal:
>>>>
>>>>
>>>> private val upCastingOrder: Seq[DataType] =
>>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>>> ...
>>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>>
>>>>
>>>>
>>>> The current way does not deal with when the types are outside of the
>>>> upCastingOrder. It just returns the first type, as the type coerced one.
>>>>
>>>> This has been being discussed in https://github.com/apache/s
>>>> park/pull/19389#discussion_r150426911, but I would like to have more
>>>> feedback from community as it possibly is a breaking change.
>>>>
>>>> For the current releases of Spark (2.2.0 <=), we support the types
>>>> below for partitioned column schema inference, given my investigation -
>>>> https://github.com/apache/spark/pull/19389#discussion_r150528207:
>>>>
>>>> NullType
>>>> IntegerType
>>>> LongType
>>>> DoubleType,
>>>> *DecimalType(...)
>>>> DateType
>>>> TimestampType
>>>> StringType
>>>>
>>>> *DecimalType only when it's bigger than LongType:
>>>>
>>>> I believe this is something we should definitely fix.
>>>>
>>>>
>>>>
>>>> *Proposal:*
>>>>
>>>> I propose the change - https://github.com/apache/spark/pull/19389
>>>>
>>>> Simply, it reuses the case 2 specified in https://github.com/apache/s
>>>> park/blob/6412ea1759d39a2380c572ec24cfd8ae4f2d81f7/sql/catal
>>>> yst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Ty
>>>> peCoercion.scala#L40-L43
>>>>
>>>> Please refer the chart I produced here - https://github.com/apache/sp
>>>> ark/pull/19389/files#r150528361. The current proposal will brings the
>>>> type coercion behaviour change in those cases below:
>>>>
>>>>
>>>> Input typesOld output typeNew output type
>>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>>> [NullType, DateType] StringType DateType
>>>> [NullType, TimestampType] StringType TimestampType
>>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>>> [IntegerType, DateType] IntegerType StringType
>>>> [IntegerType, TimestampType] IntegerType StringType
>>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>>> [LongType, DateType] LongType StringType
>>>> [LongType, TimestampType] LongType StringType
>>>> [DoubleType, DateType] DoubleType StringType
>>>> [DoubleType, TimestampType] DoubleType StringType
>>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>>> [DateType, NullType] StringType DateType
>>>> [DateType, IntegerType] IntegerType StringType
>>>> [DateType, LongType] LongType StringType
>>>> [DateType, DoubleType] DoubleType StringType
>>>> [DateType, DecimalType(38,0)] DateType StringType
>>>> [DateType, TimestampType] DateType TimestampType
>>>> [TimestampType, NullType] StringType TimestampType
>>>> [TimestampType, IntegerType] IntegerType StringType
>>>> [TimestampType, LongType] LongType StringType
>>>> [TimestampType, DoubleType] DoubleType StringType
>>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>>
>>>>
>>>>
>>>> *Other possible suggestions**:*
>>>>
>>>> Probably, we could also consider simply making the merged type to
>>>> string types when there are any type conflicts.
>>>> Otherwise, there could be more stricter rules. I want more opinion from
>>>> the community.
>>>>
>>>>
>>>>
>>>> *Questions:*
>>>>
>>>> - Does the *Proposal:* above looks good?
>>>>
>>>> - If not, what would be the alternative?
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Hyukjin Kwon <gu...@gmail.com>.
Thanks all for feedback.
> 1. when merging NullType with another type, the result should always be
that type.
> 2. when merging StringType with another type, the result should always be
StringType.
> 3. when merging integral types, the priority from high to low:
DecimalType, LongType, IntegerType. This is because DecimalType is used as
big integer when paring partition column values.
> 4. DoubleType can't be merged with other types, except DoubleType itself.
> 5. when merging TimestampType with DateType, return TimestampType.
> Rather than a list, can we create a table? X-axis is data type, and
Y-axis is also data type, and the intersection explains what the coerced
type is?
Here, I produced a table as below:
*Before*
InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
DateTypeTimestampTypeStringType
*NullType* StringType IntegerType LongType DoubleType StringType StringType
StringType StringType
*IntegerType* IntegerType IntegerType LongType DoubleType IntegerType
IntegerType IntegerType StringType
*LongType* LongType LongType LongType DoubleType LongType LongType LongType
StringType
*DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
DoubleType DoubleType StringType
*DecimalType(38,0)* StringType IntegerType LongType DoubleType
DecimalType(38,0) DecimalType(38,0) DecimalType(38,0) StringType
*DateType* StringType IntegerType LongType DoubleType DateType DateType
DateType StringType
*TimestampType* StringType IntegerType LongType DoubleType TimestampType
TimestampType TimestampType StringType
*StringType* StringType StringType StringType StringType StringType
StringType StringType StringType
*After*
InputA \ InputBNullTypeIntegerTypeLongTypeDoubleTypeDecimalType(38,0)
DateTypeTimestampTypeStringType
*NullType* StringType IntegerType LongType DoubleType DecimalType(38,0)
DateType TimestampType StringType
*IntegerType* IntegerType IntegerType LongType DoubleType DecimalType(38,0)
StringType StringType StringType
*LongType* LongType LongType LongType DoubleType DecimalType(38,0)
StringType StringType StringType
*DoubleType* DoubleType DoubleType DoubleType DoubleType DoubleType
StringType StringType StringType
*DecimalType(38,0)* DecimalType(38,0) DecimalType(38,0) DecimalType(38,0)
DoubleType DecimalType(38,0) StringType StringType StringType
*DateType* DateType StringType StringType StringType StringType DateType
TimestampType StringType
*TimestampType* TimestampType StringType StringType StringType StringType
TimestampType TimestampType StringType
*StringType* StringType StringType StringType StringType StringType
StringType StringType StringType
Seems following Wenchen's comments (1. to 5.), and I also updated PR
description there with some codes I used.
> Can we also look at what Hive, standard SQL (Postgres?) do?
> Also, this shouldn't be isolated to partition column inference.
> We should make sure most of the type coercions are consistent across
different functionalities, with the caveat that we need to preserve
backward compatibility.
Sure, so, if I understood correctly, we preserve backward compatibility for
improvements, but not for bugs in general.
Probably, there are two things to be done now:
- Deduplicates the type coercion logics, and fixes the obvious bugs, that
doesn't make sense, (e.g., decimal and timestamp ends up with decimal).
- Improves the deduplicated type coercion to follow other systems like
Hive and Postgres, or to make it sounds coherent by referring them.
I think my PR here focuses on the former, because the current partition
column inference itself is already isolated
and I am trying to propose to put those type coercion into one place in the
TypeCoercion. We could also consider
some divergence as exceptions in the nature of this functionality too,
maybe .. ? (although I agree that most of the
type coercions are consistent across different functionalities in general).
If the deduplicated type coercion itself across functionalities should be
changed as an improvement, it should preserve
backward compatibility if I understood correctly and requires to take a
look for other systems.
I am willing to do the latter soon; however, it probably takes a quite
while for me to investigate and propose a change.
So, meanwhile, could we separately proceed this PR maybe or, probably would
there be something I missed?
2017-11-15 7:29 GMT+09:00 Reynold Xin <rx...@databricks.com>:
> Most of those thoughts from Wenchen make sense to me.
>
>
> Rather than a list, can we create a table? X-axis is data type, and Y-axis
> is also data type, and the intersection explains what the coerced type is?
> Can we also look at what Hive, standard SQL (Postgres?) do?
>
>
> Also, this shouldn't be isolated to partition column inference. We should
> make sure most of the type coercions are consistent across different
> functionalities, with the caveat that we need to preserve backward
> compatibility.
>
>
>
> On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com> wrote:
>
>> My 2 cents:
>>
>> 1. when merging NullType with another type, the result should always be
>> that type.
>> 2. when merging StringType with another type, the result should always be
>> StringType.
>> 3. when merging integral types, the priority from high to low:
>> DecimalType, LongType, IntegerType. This is because DecimalType is used as
>> big integer when paring partition column values.
>> 4. DoubleType can't be merged with other types, except DoubleType itself.
>> 5. when merging TimestampType with DateType, return TimestampType.
>>
>>
>> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gu...@gmail.com>
>> wrote:
>>
>>> Hi dev,
>>>
>>> I would like to post a proposal about partitioned column type inference
>>> (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>>> configuration).
>>>
>>> This thread focuses on the type coercion (finding the common type) in
>>> partitioned columns, in particular, when the different form of data is
>>> inserted for the partition column and then it is read back with the type
>>> inference.
>>>
>>>
>>> *Problem:*
>>>
>>>
>>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>>> spark.read.load("/tmp/foo").printSchema()
>>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>>> spark.read.load("/tmp/bar").printSchema()
>>>
>>>
>>>
>>> It currently returns:
>>>
>>>
>>> root
>>> |-- i: integer (nullable = true)
>>> |-- ts: date (nullable = true)
>>>
>>> root
>>> |-- i: integer (nullable = true)
>>> |-- decimal: integer (nullable = true)
>>>
>>>
>>> The type coercion looks less well designed yet and currently there are
>>> few holes which is not quite ideal:
>>>
>>>
>>> private val upCastingOrder: Seq[DataType] =
>>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>>> ...
>>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>>
>>>
>>>
>>> The current way does not deal with when the types are outside of the
>>> upCastingOrder. It just returns the first type, as the type coerced one.
>>>
>>> This has been being discussed in https://github.com/apache/s
>>> park/pull/19389#discussion_r150426911, but I would like to have more
>>> feedback from community as it possibly is a breaking change.
>>>
>>> For the current releases of Spark (2.2.0 <=), we support the types below
>>> for partitioned column schema inference, given my investigation -
>>> https://github.com/apache/spark/pull/19389#discussion_r150528207:
>>>
>>> NullType
>>> IntegerType
>>> LongType
>>> DoubleType,
>>> *DecimalType(...)
>>> DateType
>>> TimestampType
>>> StringType
>>>
>>> *DecimalType only when it's bigger than LongType:
>>>
>>> I believe this is something we should definitely fix.
>>>
>>>
>>>
>>> *Proposal:*
>>>
>>> I propose the change - https://github.com/apache/spark/pull/19389
>>>
>>> Simply, it reuses the case 2 specified in https://github.com/apache/s
>>> park/blob/6412ea1759d39a2380c572ec24cfd8ae4f2d81f7/sql/catal
>>> yst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Ty
>>> peCoercion.scala#L40-L43
>>>
>>> Please refer the chart I produced here - https://github.com/apache/sp
>>> ark/pull/19389/files#r150528361. The current proposal will brings the
>>> type coercion behaviour change in those cases below:
>>>
>>>
>>> Input typesOld output typeNew output type
>>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>>> [NullType, DateType] StringType DateType
>>> [NullType, TimestampType] StringType TimestampType
>>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>>> [IntegerType, DateType] IntegerType StringType
>>> [IntegerType, TimestampType] IntegerType StringType
>>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>>> [LongType, DateType] LongType StringType
>>> [LongType, TimestampType] LongType StringType
>>> [DoubleType, DateType] DoubleType StringType
>>> [DoubleType, TimestampType] DoubleType StringType
>>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>>> [DateType, NullType] StringType DateType
>>> [DateType, IntegerType] IntegerType StringType
>>> [DateType, LongType] LongType StringType
>>> [DateType, DoubleType] DoubleType StringType
>>> [DateType, DecimalType(38,0)] DateType StringType
>>> [DateType, TimestampType] DateType TimestampType
>>> [TimestampType, NullType] StringType TimestampType
>>> [TimestampType, IntegerType] IntegerType StringType
>>> [TimestampType, LongType] LongType StringType
>>> [TimestampType, DoubleType] DoubleType StringType
>>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>>
>>>
>>>
>>> *Other possible suggestions**:*
>>>
>>> Probably, we could also consider simply making the merged type to string
>>> types when there are any type conflicts.
>>> Otherwise, there could be more stricter rules. I want more opinion from
>>> the community.
>>>
>>>
>>>
>>> *Questions:*
>>>
>>> - Does the *Proposal:* above looks good?
>>>
>>> - If not, what would be the alternative?
>>>
>>>
>>>
>>>
>>>
>>
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Reynold Xin <rx...@databricks.com>.
Most of those thoughts from Wenchen make sense to me.
Rather than a list, can we create a table? X-axis is data type, and Y-axis
is also data type, and the intersection explains what the coerced type is?
Can we also look at what Hive, standard SQL (Postgres?) do?
Also, this shouldn't be isolated to partition column inference. We should
make sure most of the type coercions are consistent across different
functionalities, with the caveat that we need to preserve backward
compatibility.
On Tue, Nov 14, 2017 at 8:33 AM, Wenchen Fan <cl...@gmail.com> wrote:
> My 2 cents:
>
> 1. when merging NullType with another type, the result should always be
> that type.
> 2. when merging StringType with another type, the result should always be
> StringType.
> 3. when merging integral types, the priority from high to low:
> DecimalType, LongType, IntegerType. This is because DecimalType is used as
> big integer when paring partition column values.
> 4. DoubleType can't be merged with other types, except DoubleType itself.
> 5. when merging TimestampType with DateType, return TimestampType.
>
>
> On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gu...@gmail.com> wrote:
>
>> Hi dev,
>>
>> I would like to post a proposal about partitioned column type inference
>> (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
>> configuration).
>>
>> This thread focuses on the type coercion (finding the common type) in
>> partitioned columns, in particular, when the different form of data is
>> inserted for the partition column and then it is read back with the type
>> inference.
>>
>>
>> *Problem:*
>>
>>
>> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
>> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
>> spark.read.load("/tmp/foo").printSchema()
>> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
>> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
>> spark.read.load("/tmp/bar").printSchema()
>>
>>
>>
>> It currently returns:
>>
>>
>> root
>> |-- i: integer (nullable = true)
>> |-- ts: date (nullable = true)
>>
>> root
>> |-- i: integer (nullable = true)
>> |-- decimal: integer (nullable = true)
>>
>>
>> The type coercion looks less well designed yet and currently there are
>> few holes which is not quite ideal:
>>
>>
>> private val upCastingOrder: Seq[DataType] =
>> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
>> ...
>> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>>
>>
>>
>> The current way does not deal with when the types are outside of the
>> upCastingOrder. It just returns the first type, as the type coerced one.
>>
>> This has been being discussed in https://github.com/apache/s
>> park/pull/19389#discussion_r150426911, but I would like to have more
>> feedback from community as it possibly is a breaking change.
>>
>> For the current releases of Spark (2.2.0 <=), we support the types below
>> for partitioned column schema inference, given my investigation -
>> https://github.com/apache/spark/pull/19389#discussion_r150528207:
>>
>> NullType
>> IntegerType
>> LongType
>> DoubleType,
>> *DecimalType(...)
>> DateType
>> TimestampType
>> StringType
>>
>> *DecimalType only when it's bigger than LongType:
>>
>> I believe this is something we should definitely fix.
>>
>>
>>
>> *Proposal:*
>>
>> I propose the change - https://github.com/apache/spark/pull/19389
>>
>> Simply, it reuses the case 2 specified in https://github.com/apache/s
>> park/blob/6412ea1759d39a2380c572ec24cfd8ae4f2d81f7/sql/catal
>> yst/src/main/scala/org/apache/spark/sql/catalyst/analysis/
>> TypeCoercion.scala#L40-L43
>>
>> Please refer the chart I produced here - https://github.com/apache/sp
>> ark/pull/19389/files#r150528361. The current proposal will brings the
>> type coercion behaviour change in those cases below:
>>
>>
>> Input typesOld output typeNew output type
>> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
>> [NullType, DateType] StringType DateType
>> [NullType, TimestampType] StringType TimestampType
>> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
>> [IntegerType, DateType] IntegerType StringType
>> [IntegerType, TimestampType] IntegerType StringType
>> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
>> [LongType, DateType] LongType StringType
>> [LongType, TimestampType] LongType StringType
>> [DoubleType, DateType] DoubleType StringType
>> [DoubleType, TimestampType] DoubleType StringType
>> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
>> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
>> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
>> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
>> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
>> [DateType, NullType] StringType DateType
>> [DateType, IntegerType] IntegerType StringType
>> [DateType, LongType] LongType StringType
>> [DateType, DoubleType] DoubleType StringType
>> [DateType, DecimalType(38,0)] DateType StringType
>> [DateType, TimestampType] DateType TimestampType
>> [TimestampType, NullType] StringType TimestampType
>> [TimestampType, IntegerType] IntegerType StringType
>> [TimestampType, LongType] LongType StringType
>> [TimestampType, DoubleType] DoubleType StringType
>> [TimestampType, DecimalType(38,0)] TimestampType StringType
>>
>>
>>
>> *Other possible suggestions**:*
>>
>> Probably, we could also consider simply making the merged type to string
>> types when there are any type conflicts.
>> Otherwise, there could be more stricter rules. I want more opinion from
>> the community.
>>
>>
>>
>> *Questions:*
>>
>> - Does the *Proposal:* above looks good?
>>
>> - If not, what would be the alternative?
>>
>>
>>
>>
>>
>
Re: [discuss][SQL] Partitioned column type inference proposal
Posted by Wenchen Fan <cl...@gmail.com>.
My 2 cents:
1. when merging NullType with another type, the result should always be
that type.
2. when merging StringType with another type, the result should always be
StringType.
3. when merging integral types, the priority from high to low: DecimalType,
LongType, IntegerType. This is because DecimalType is used as big integer
when paring partition column values.
4. DoubleType can't be merged with other types, except DoubleType itself.
5. when merging TimestampType with DateType, return TimestampType.
On Tue, Nov 14, 2017 at 3:54 PM, Hyukjin Kwon <gu...@gmail.com> wrote:
> Hi dev,
>
> I would like to post a proposal about partitioned column type inference
> (related with 'spark.sql.sources.partitionColumnTypeInference.enabled'
> configuration).
>
> This thread focuses on the type coercion (finding the common type) in
> partitioned columns, in particular, when the different form of data is
> inserted for the partition column and then it is read back with the type
> inference.
>
>
> *Problem:*
>
>
> val df = Seq((1, "2015-01-01"), (2, "2016-01-01 00:00:00")).toDF("i", "ts")
> df.write.format("parquet").partitionBy("ts").save("/tmp/foo")
> spark.read.load("/tmp/foo").printSchema()
> val df = Seq((1, "1"), (2, "1" * 30)).toDF("i", "decimal")
> df.write.format("parquet").partitionBy("decimal").save("/tmp/bar")
> spark.read.load("/tmp/bar").printSchema()
>
>
>
> It currently returns:
>
>
> root
> |-- i: integer (nullable = true)
> |-- ts: date (nullable = true)
>
> root
> |-- i: integer (nullable = true)
> |-- decimal: integer (nullable = true)
>
>
> The type coercion looks less well designed yet and currently there are few
> holes which is not quite ideal:
>
>
> private val upCastingOrder: Seq[DataType] =
> Seq(NullType, IntegerType, LongType, FloatType, DoubleType, StringType)
> ...
> literals.map(_.dataType).maxBy(upCastingOrder.indexOf(_))
>
>
>
> The current way does not deal with when the types are outside of the
> upCastingOrder. It just returns the first type, as the type coerced one.
>
> This has been being discussed in https://github.com/apache/
> spark/pull/19389#discussion_r150426911, but I would like to have more
> feedback from community as it possibly is a breaking change.
>
> For the current releases of Spark (2.2.0 <=), we support the types below
> for partitioned column schema inference, given my investigation -
> https://github.com/apache/spark/pull/19389#discussion_r150528207:
>
> NullType
> IntegerType
> LongType
> DoubleType,
> *DecimalType(...)
> DateType
> TimestampType
> StringType
>
> *DecimalType only when it's bigger than LongType:
>
> I believe this is something we should definitely fix.
>
>
>
> *Proposal:*
>
> I propose the change - https://github.com/apache/spark/pull/19389
>
> Simply, it reuses the case 2 specified in https://github.com/apache/
> spark/blob/6412ea1759d39a2380c572ec24cfd8ae4f2d81f7/sql/catalyst/src/
> main/scala/org/apache/spark/sql/catalyst/analysis/
> TypeCoercion.scala#L40-L43
>
> Please refer the chart I produced here - https://github.com/apache/
> spark/pull/19389/files#r150528361. The current proposal will brings the
> type coercion behaviour change in those cases below:
>
>
> Input typesOld output typeNew output type
> [NullType, DecimalType(38,0)] StringType DecimalType(38,0)
> [NullType, DateType] StringType DateType
> [NullType, TimestampType] StringType TimestampType
> [IntegerType, DecimalType(38,0)] IntegerType DecimalType(38,0)
> [IntegerType, DateType] IntegerType StringType
> [IntegerType, TimestampType] IntegerType StringType
> [LongType, DecimalType(38,0)] LongType DecimalType(38,0)
> [LongType, DateType] LongType StringType
> [LongType, TimestampType] LongType StringType
> [DoubleType, DateType] DoubleType StringType
> [DoubleType, TimestampType] DoubleType StringType
> [DecimalType(38,0), NullType] StringType DecimalType(38,0)
> [DecimalType(38,0), IntegerType] IntegerType DecimalType(38,0)
> [DecimalType(38,0), LongType] LongType DecimalType(38,0)
> [DecimalType(38,0), DateType] DecimalType(38,0) StringType
> [DecimalType(38,0), TimestampType] DecimalType(38,0) StringType
> [DateType, NullType] StringType DateType
> [DateType, IntegerType] IntegerType StringType
> [DateType, LongType] LongType StringType
> [DateType, DoubleType] DoubleType StringType
> [DateType, DecimalType(38,0)] DateType StringType
> [DateType, TimestampType] DateType TimestampType
> [TimestampType, NullType] StringType TimestampType
> [TimestampType, IntegerType] IntegerType StringType
> [TimestampType, LongType] LongType StringType
> [TimestampType, DoubleType] DoubleType StringType
> [TimestampType, DecimalType(38,0)] TimestampType StringType
>
>
>
> *Other possible suggestions**:*
>
> Probably, we could also consider simply making the merged type to string
> types when there are any type conflicts.
> Otherwise, there could be more stricter rules. I want more opinion from
> the community.
>
>
>
> *Questions:*
>
> - Does the *Proposal:* above looks good?
>
> - If not, what would be the alternative?
>
>
>
>
>