[SPARK-11997][SQL] NPE when save a DataFrame as parquet and partitioned by long column

[dilipbiswal]

Check for partition column null-ability while building the partition spec.

[dilipbiswal]

@davies Can you please let me know your comments.

[yhuai]

ok to test

[yhuai]

@dilipbiswal Do you know why it is broken?

[dilipbiswal]

@yhuai Hi Yin, in the discoverPartitions method, we are trying to create the partition spec and are trying to cast a partition value to the corresponding user specified schema's column type. In case of null partition value the following code raises a null poiner exception in row.getString(i).

Cast(Literal.create(row.getString(i), StringType), userProvidedSchema.fields(i).dataType).eval()

in this fix i am trying to check for null first and create a null literal of string type as opposed to calling row.getString(). Hope it is okay .. Please let me know.

[SparkQA]

Test build #46777 has finished for PR 10001 at commit af508de.

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

[davies]

I think it's better to check null in InternalRow.getString()

[dilipbiswal]

@davies Thanks for reviewing the change. In cases when we know in advance that schema does not allow nulls , sometimes we can skip this null check. By moving it to internalRow , would we loose the opportunity to optimize ? Pl. let me know.

[davies]

The null-checking is cheap.

Or we could use Literal.create(row.getUTF8String(i), StringType), to avoid the conversion between String and UTF8String.

[dilipbiswal]

Sounds good @davies .Made the change. Thanks a lot for your feedback.

[yhuai]

@dilipbiswal I guess I did not ask my question clearly. I meant why 1.5 is good but 1.6 is broken. There must be a change that exposed this issue.

[dilipbiswal]

@yhuai Yeah.. this function discoverPartitions() was changed recently (10/22/2015) as part of spark-9735.

[davies]

LGTM

[SparkQA]

Test build #46791 has finished for PR 10001 at commit 4de7697.

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