-
Notifications
You must be signed in to change notification settings - Fork 2.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Fix bug with incrementally pulling older data #458
Conversation
@@ -88,8 +88,12 @@ class IncrementalRelation(val sqlContext: SQLContext, | |||
.get, classOf[HoodieCommitMetadata]) | |||
fileIdToFullPath ++= metadata.getFileIdAndFullPaths(basePath).toMap | |||
} | |||
// unset the path filter, otherwise if end_instant_time < latest_instant_time, path filter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you describe this a little more please ? By reading offhand, this is not clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing context is in how the RO view is implemented. It sets a path filter to filter all files except latest.. if we don't unset here, filter still kicks in and filters out all files when dealing with older instant ranges
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Looks great.
@@ -98,14 +98,24 @@ class DataSourceTest extends AssertionsForJUnit { | |||
|
|||
|
|||
// Read Incremental View | |||
val firstCommit = HoodieDataSourceHelpers.listCommitsSince(fs, basePath, "000").get(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a description here for the newly added test-case ?
ff5cbc9
to
1e8cee7
Compare
@@ -88,8 +88,12 @@ class IncrementalRelation(val sqlContext: SQLContext, | |||
.get, classOf[HoodieCommitMetadata]) | |||
fileIdToFullPath ++= metadata.getFileIdAndFullPaths(basePath).toMap | |||
} | |||
// unset the path filter, otherwise if end_instant_time is not the latest instant, path filter set for RO view |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@n3nash updated comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -98,14 +98,26 @@ class DataSourceTest extends AssertionsForJUnit { | |||
|
|||
|
|||
// Read Incremental View | |||
// we have 2 commits, try pulling the first commit (which is not the latest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bvaradar done
No description provided.