Skip to content

Conversation

@xupefei
Copy link
Contributor

@xupefei xupefei commented Jul 25, 2024

What changes were proposed in this pull request?

This PR modifies some Mima rules which are used for checking the binary compatibility between sql and connect modules. Major changes include:

  • Removed unnecessary filters for specific private[sql] constructors - there's a wildcard rule which filters out all of them.
  • Removed outdated filters about APIs that are already consistent.
  • Add a warning about unused filters.

Current output:

$ ./dev/connect-jvm-client-mima-check
Do connect-client-jvm module mima check ...
Warning: ExcludeByName[Problem]("org.apache.spark.sql.Dataset.queryExecution") did not filter out any problems.
Warning: ExcludeByName[Problem]("org.apache.spark.sql.Dataset.sqlContext") did not filter out any problems.
Warning: ExcludeByName[Problem]("org.apache.spark.sql.Dataset.selectUntyped") did not filter out any problems.
Warning: ExcludeByName[Problem]("org.apache.spark.sql.Dataset.rdd") did not filter out any problems.
Warning: ExcludeByName[Problem]("org.apache.spark.sql.Dataset.toJavaRDD") did not filter out any problems.
Warning: ExcludeByName[Problem]("org.apache.spark.sql.Dataset.javaRDD") did not filter out any problems.
finish connect-client-jvm module mima check ...
connect-client-jvm module mima check passed.

I manually audited all rules defined in the list.

One issue I found is that all APIs in Dataset are not being checked at all, likely due to having a private[sql] companion object in spark-core. Changing the object's visibility from private[sql] to public will resolve this issue. The exact reason is unknown and is to be investigated.

Why are the changes needed?

Need to make sure Mima is really working.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Not needed.

Was this patch authored or co-authored using generative AI tooling?

No.

@xupefei xupefei changed the title Mima [SPARK-49060] Clean up Mima rules for SQL-Connect binary compatibility checks Jul 30, 2024
@xupefei xupefei marked this pull request as ready for review July 30, 2024 12:33
* See [[Encoder]] for more details on what types are encodable to Spark SQL.
*/
def transformWithState[U: Encoder, S: Encoder](
private[sql] def transformWithState[U: Encoder, S: Encoder](
Copy link
Member

Choose a reason for hiding this comment

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

why do we make this API private?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found these methods are private in Spark Core:

private[sql] def transformWithState[U: Encoder](
.

@HyukjinKwon HyukjinKwon changed the title [SPARK-49060] Clean up Mima rules for SQL-Connect binary compatibility checks [SPARK-49060][CONNECT] Clean up Mima rules for SQL-Connect binary compatibility checks Aug 5, 2024
@HyukjinKwon
Copy link
Member

Merged to master.

@xupefei xupefei deleted the mima-refactor branch August 5, 2024 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants