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 Nov 26, 2020

What is the context for this pull request?

What changes were proposed in this pull request?

This PR refactors Hybrid Scan test suites.

  • new HybridScanForDeltaLakeSuite.scala for Delta Lake index Hybrid Scan
  • refactored non partitioned / partitioned data test suite

Does this PR introduce any user-facing change?

No

How was this patch tested?

Unit tests

@sezruby sezruby self-assigned this Dec 3, 2020
@sezruby sezruby added this to the November 2020 milestone Dec 3, 2020
@pirz
Copy link
Contributor

pirz commented Dec 7, 2020

@sezruby As there is overlap between changes in this PR and #265 , can you list the classes that need to be reviewed here? Are they only test classes?

@sezruby
Copy link
Collaborator Author

sezruby commented Dec 8, 2020

@priz yes, this PR just refactors Hybrid Scan tests, not including any changes in src/main/*.
Please review the last commit :) Thanks!
36359fa

@sezruby sezruby force-pushed the hybridtest_refactoring branch from 36359fa to ea65e60 Compare December 9, 2020 07:36
@imback82
Copy link
Contributor

imback82 commented Jan 7, 2021

  • renamed HybridScanTest.scala => HybridScanTestSuite.scala

Can we do this as a separate PR? There are rename / changes, so diff is too big to review.

@sezruby sezruby force-pushed the hybridtest_refactoring branch from ea65e60 to f9cdef5 Compare January 8, 2021 06:32
assert(basePlan.equals(join.queryExecution.optimizedPlan))
// Check if the given plan is transformed correctly to handle deleted files. The plan
// should include only one LogicalRelation node which is transformed with the given index.
def checkDeletedFiles(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note to reviewers: Instead of checking the transformed plan in every test, I introduced following functions to check the transformed plan properly.

def checkFilterIndexHybridScanUnion( // for filter rule
def checkJoinIndexHybridScan( // for join rule
def checkDeletedFiles( // check the deleted files for both filter & join 

HybridScanSuite contains general testcases for all non-partitioned/partitioned/delta lake dataset.
Dataset-specific tests are located in each test class.

# Conflicts:
#	src/test/scala/com/microsoft/hyperspace/index/HybridScanForNonPartitionedDataTest.scala
#	src/test/scala/com/microsoft/hyperspace/index/HybridScanSuite.scala
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 efe33ec into microsoft:master Jan 19, 2021
@imback82 imback82 added the enhancement New feature or request label Jan 29, 2021
@imback82 imback82 modified the milestones: November 2020, January 2021 Jan 29, 2021
@sezruby sezruby deleted the hybridtest_refactoring branch April 30, 2021 03:26
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.

3 participants