[SPARK-2729] [SQL] Forgot to match Timestamp type in ColumnBuilder

[chutium]

just a match forgot, found after SPARK-2710 , TimestampType can be used by a SchemaRDD generated from JDBC ResultSet

[AmplabJenkins]

Can one of the admins verify this patch?

[marmbrus]

Can you please add [SQL] to any PR title that touches Spark SQL?

test this please

[marmbrus]

We should also add a test case for this.

[chutium]

jo, thanks, i will add [SQL] next time

[marmbrus]

Will you have time to add a test case before Friday (the merge deadline for 1.1) or should I?

[chutium]

it seems should be this one?
NullableColumnBuilderSuite.scala

i can do it

[chutium]

NullableColumnBuilderSuite:
...
...
- TIMESTAMP column builder: empty column
- TIMESTAMP column builder: buffer size auto growth
- TIMESTAMP column builder: null values
All tests passed.

[chutium]

if you have time, maybe take a look at #1612 ? it is about this ticket, https://issues.apache.org/jira/browse/SPARK-2710 , test suite just added.

it will be a good feature for some use cases, such as we use RDBMS (OLTP) to store daily incremental data, and full data stored in Hadoop, we want to use Spark SQL to join or union the tables from these two data source. now we must hard code case class for each tables came from JDBC...

and i find this one for CSV #1351 , it seems it is a good sample for adding new SchemaRDD types, need also to add some interface in sql.py and JavaSQLContext right?

[marmbrus]

Thanks for working on this. I think that that test actually passes even without your change, so I don't think its sufficient. Maybe @liancheng has an idea of how to test this better.

Regarding #1612, I'm very excited about this functionality as it is something we have been talking about for a while. I will look at it after we cut a release candidate for Spark 1.1 (this friday).

[liancheng]

@chutium Michael is right, NullableColumnBuilderSuite doesn't call ColumnBuilder.apply, thus it always passes. But adding TIMESTAMP still makes sense. Actually, would you mind to add TIMESTAMP to NullableColumnAccessorSuite too? These two test suites are structurally similar. Thanks!

To add a valid test case for this PR, I'd recommend InMemoryColumnarQuerySuite. You may refer to my comment in SPARK-2729 to write this test.

[chutium]

thanks a lot to you both, cast string to timestamp is really clever :)

[marmbrus]

Friendly reminder, it would be great to have a the test cases suggested by @liancheng so we can merge this by the deadline (friday).

[liancheng]

@marmbrus I've discussed with @chutium offline. Since 1.1 code freeze deadline is close, we can merge this first. I tested this PR locally and it looks good. Will submit another PR to add the test case.

[marmbrus]

Thanks! I've merged this into master.