Skip to content
This repository was archived by the owner on Jun 14, 2024. It is now read-only.

Conversation

@sezruby
Copy link
Collaborator

@sezruby sezruby commented Dec 22, 2020

What is the context for this pull request?

Fixes #295

What changes were proposed in this pull request?

This PR includes following bug fixes for Delta Lake source.

Does this PR introduce any user-facing change?

Yes, this PR fixes several issues described above.

How was this patch tested?

Unit test

@sezruby sezruby changed the title Support incremental refresh for Delta Lake source Support incremental refresh for Delta Lake Dec 22, 2020
@sezruby sezruby self-assigned this Dec 22, 2020
* @param relation [[Relation]] object to read partial data files.
* @return File format to read partial data files.
*/
def partialReadFileFormat(relation: Relation): Option[String]
Copy link
Contributor

@imback82 imback82 Jan 7, 2021

Choose a reason for hiding this comment

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

I think this should be def internalFileFormatName, underlyingFileFormatName, physicalFileFormatName or something to denote the actual file format used for storing the files?

@imback82
Copy link
Contributor

@apoorvedave1 @pirz Can you review this PR? Thanks.

apoorvedave1
apoorvedave1 previously approved these changes Jan 13, 2021
Copy link
Contributor

@apoorvedave1 apoorvedave1 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @sezruby

pirz
pirz previously approved these changes Jan 13, 2021
Copy link
Contributor

@pirz pirz left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks.

logicalRelation.relation match {
case HadoopFsRelation(_: PartitioningAwareFileIndex, _, _, _, format, _)
if isSupportedFileFormat(format) =>
if isSupportedFileFormat(format) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Is indentation here correct?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes it's fixed by scalafmt; it's because this if is for the case.

@sezruby sezruby dismissed stale reviews from pirz and apoorvedave1 via af8fb7e January 14, 2021 05:12
Copy link
Contributor

@imback82 imback82 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @sezruby!

@imback82 imback82 merged commit 08650ff into microsoft:master Jan 14, 2021
@sezruby sezruby deleted the deltalake_incremental branch January 14, 2021 07:08
@imback82 imback82 added the enhancement New feature or request label Jan 29, 2021
@imback82 imback82 added this to the January 2021 milestone Jan 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate if refresh incremental works fine with Delta lake

4 participants