Skip to content

Conversation

@gengliangwang
Copy link
Member

What changes were proposed in this pull request?

In the PR for supporting logical timestamp types #21935, a SQL configuration spark.sql.avro.outputTimestampType is added, so that user can specify the output timestamp precision they want.

With PR #21847, the output file can be written with user specified types.

So there is no need to have such trivial configuration. Otherwise to make it consistent we need to add configuration for all the Catalyst types that can be converted into different Avro types.

This PR also add a test case for user specified output schema with different timestamp types.

How was this patch tested?

Unit test

Copy link
Member

@HyukjinKwon HyukjinKwon left a comment

Choose a reason for hiding this comment

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

LGTM

@SparkQA
Copy link

SparkQA commented Aug 20, 2018

Test build #94949 has finished for PR 22151 at commit 088bc83.

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

s"Avro type $avroStruct.")
}
val avroFields = avroStruct.getFields
assert(avroFields.size() == catalystStruct.length)
Copy link
Member

Choose a reason for hiding this comment

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

Is this check redundant? Why remove it now?

Copy link
Member

Choose a reason for hiding this comment

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

I think he moved this condition above.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yea, strictly sounds unrelated tho.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I miss it. :)

}

test("Logical type: user specified schema") {
test("Logical type: user specified read schema") {
Copy link
Member

Choose a reason for hiding this comment

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

nit: Logical type: user specified read schema with different timestamp types.

@viirya
Copy link
Member

viirya commented Aug 20, 2018

LGTM

@HyukjinKwon
Copy link
Member

Merged to master.

@asfgit asfgit closed this in 60af250 Aug 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants