Support Hybrid Scan with Delta Lake relation #224
Conversation
sezruby
changed the title
sezruby
force-pushed
the
deltamulti
branch
2 times, most recently
from
dbb04f7 to
fa84c7e
Compare
| import spark.implicits._ | ||
| val dfFromSample = sampleData.toDF("Date", "RGUID", "Query", "imprs", "clicks") | ||
| hyperspace = new Hyperspace(spark) | ||
|
|
There was a problem hiding this comment.
Note: remove pre-construction of indexes and create data & index in each test.
| basePlan | ||
| .equals(filter.queryExecution.optimizedPlan) | ||
| .equals(!transformationExpected)) | ||
| withTempPathAsString { testPath => |
There was a problem hiding this comment.
Note: just added this "withTempPathAsString" for this test
| } | ||
| } | ||
|
|
||
| test("Delete-only: filter rule, number of delete files threshold") { |
There was a problem hiding this comment.
Note: moved from HybridScanTestSuite as no need to run this test for different source file formats.
| hyperspace = new Hyperspace(spark) | ||
| } | ||
|
|
||
| FileUtils.delete(new Path(sampleDataLocationRoot)) |
There was a problem hiding this comment.
Note: removed pre-built indexes
| indexConfig: IndexConfig, | ||
| appendCnt: Int, | ||
| deleteCnt: Int): (Seq[String], Seq[String]) = { | ||
| dfFromSample.write.partitionBy("clicks", "Date").format(sourceFileFormat).save(sourcePath) |
There was a problem hiding this comment.
Note: as this function needs to create source data accordingly, added (override) function for partitioned data.
src/test/scala/com/microsoft/hyperspace/index/HybridScanTestSuite.scala
Outdated
Show resolved
Hide resolved
src/test/scala/com/microsoft/hyperspace/index/HybridScanTestSuite.scala
Outdated
Show resolved
Hide resolved
| // Exclude "versionAsOf" and "timestampAsOf" options, so that this DataFrame for refresh | ||
| // can see the latest snapshot. |
There was a problem hiding this comment.
does it mean that user cannot create index on a previous version?
There was a problem hiding this comment.
No, but this is the refresh action so that we should remove the versionAsOf or timestampAsOf option to refresh based on the latest version.
If a user would like to refresh with a specific version, not the latest version, then we might need to extend the refresh API.
| // it will be matched again and transformed recursively which causes stack overflow exception. | ||
| case baseRelation @ LogicalRelation( | ||
| _ @HadoopFsRelation(location: PartitioningAwareFileIndex, _, _, _, _, _), | ||
| _ @HadoopFsRelation(baseLocation: FileIndex, _, _, _, _, _), |
There was a problem hiding this comment.
could you restrict it to PartitionAwareFileIndex or TahoeFileIndex. There are other FileIndex implementations which we don't support / haven't investigated fully and we don't know what the behavior could be I guess.
There was a problem hiding this comment.
In that case, an exception will be thrown with pluggable source provider impl.
There was a problem hiding this comment.
got it. Could you please add the comment in the code then? it's otherwise difficult to guess this part for reviewers
| Some(basePath.toString) | ||
| case d: TahoeLogFileIndex => | ||
| Some(d.path.toString) | ||
| } |
There was a problem hiding this comment.
match error for non-PAFI and non-TLFI
| } | ||
|
|
||
| def retrieveCurFileInfos(plan: LogicalPlan): Seq[FileInfo] = { | ||
| val ret = plan.collect { |
There was a problem hiding this comment.
rename ret => fileInfos?
There was a problem hiding this comment.
or groupedFileInfos or something?
|
This PR will be delivered by #265. |
|
Closed by #265 |
4 participants
What is the context for this pull request?
Please just check the last commit to check the diff as the base is not yet merged - #197
What changes were proposed in this pull request?
This PR enables to do Hybrid Scan with Delta Lake relation.
A new util function
def retrieveCurFileInfos(plan: LogicalPlan)is defined and it returns the current file list in a given logical plan regardless of the type of the relation.We can optimize
getCandidateIndexfor Hybrid Scan by using the version info and path of Delta Lake later.Does this PR introduce any user-facing change?
Yes, a user can utilize Hybrid Scan with Delta Lake relation.
How was this patch tested?
Unit test