-
Notifications
You must be signed in to change notification settings - Fork 168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Fix some regressions with Spark 3.5.1 #674
Conversation
@parthchandra @kazuyukitanimura This PR is ready for review now and fixes almost all of the ignored tests for Spark 3.5.1. I will follow up with a separate PR for the final two tests. |
ShimFileFormat.findRowIndexColumnIndexInSchema(sparkSchema) >= 0 | ||
} | ||
// see SPARK-39634 | ||
protected def isNeededForSchema(sparkSchema: StructType): Boolean = false |
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.
We still have this in the 4.0 shim. Is this not needed any more?
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.
nvm. I was looking at the wrong file. This is also changed in the 4.0 shim.
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.
I updated this to match the version in the 4.0 shims because https://issues.apache.org/jira/browse/SPARK-39634 is marked as fixed in 3.5
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
@kazuyukitanimura @huaxingao Could I get a committer review? Parth has already approved. |
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
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
Which issue does this PR close?
Part of #617
Rationale for this change
What changes are included in this PR?
How are these changes tested?