Skip to content

Conversation

@izchen
Copy link
Contributor

@izchen izchen commented Dec 23, 2021

When I cherry-pick, I found that the method testDeleteFromTableAtSnapshot is missing a Test annotation.

@github-actions github-actions bot added the spark label Dec 23, 2021
@izchen izchen closed this Dec 24, 2021
@rdblue
Copy link
Contributor

rdblue commented Dec 29, 2021

@izchen, why did you close this? It looks like this test should be running. @wypoon, any idea why this tests was disabled?

@wypoon
Copy link
Contributor

wypoon commented Dec 29, 2021

Hmm, looks like an oversight on my part when porting the change to v3.0. It should have the @Test annotation. However, I am on vacation, don't have my work machine at hand, and not able to test it at the moment.

@rdblue
Copy link
Contributor

rdblue commented Dec 29, 2021

Enjoy your vacation! We'll have to track this down, but I don't think there's a rush.

@izchen
Copy link
Contributor Author

izchen commented Jan 4, 2022

Why did you close this? It looks like this test should be running.

@rdblue, this UT failed in spark version 3.0. I think this is because the [SPARK-33623][SQL] Add canDeleteWhere to SupportsDelete does not exist in 3.0.

@izchen
Copy link
Contributor Author

izchen commented Jan 4, 2022

I submitted a PR to fix this: #3840

@wypoon
Copy link
Contributor

wypoon commented Jan 4, 2022

@izchen thanks for the explanation. I'd forgotten about the fact that canDeleteWhere was added to SupportsDelete only in Spark 3.1 and is not present in Spark 3.0. The fact that we didn't try to clean up all such differences when we reorganized the Spark support into separate directories tripped me up. I think I inadvertently omitted the @Test annotation for the new test and didn't mean to disable it, but this omission led to the CI passing.

@izchen izchen deleted the hotfix_testDeleteFromTableAtSnapshot' branch January 5, 2022 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants