Skip to content

Conversation

dongjoon-hyun
Copy link
Member

@dongjoon-hyun dongjoon-hyun commented Oct 1, 2025

What changes were proposed in this pull request?

This PR aims to recover alphabetic ordering of rules in RuleIdCollection class for Apache Spark 4.1.0.

Why are the changes needed?

Since rulesNeedingIds was originally defined to be in an alphabetic order like the following, we had better recover the ordering according to the original intention.

// a changing, external state. Rules here are in alphabetical order.
private var rulesNeedingIds: Seq[String] = {

Rules here are in alphabetical order.

Currently, (1) it has several outliers in terms of ordering and (2) the ordering is mixed with full class name and simple class name. For instance, AnsiCombinedTypeCoercionRule should be placed at the second if we consider simple class name. This PR makes it consistent via full name to fix the inconsistency including the following.

"org.apache.spark.sql.catalyst.analysis.Analyzer$ResolveFunctions" ::
"org.apache.spark.sql.catalyst.analysis.Analyzer$ResolveProcedures" ::
"org.apache.spark.sql.catalyst.analysis.Analyzer$ResolveGenerate" ::

"org.apache.spark.sql.catalyst.analysis.ResolveUpdateEventTimeWatermarkColumn" ::
"org.apache.spark.sql.catalyst.expressions.EliminatePipeOperators" ::
"org.apache.spark.sql.catalyst.expressions.ValidateAndStripPipeExpressions" ::
"org.apache.spark.sql.catalyst.analysis.ResolveUnresolvedHaving" ::
"org.apache.spark.sql.catalyst.analysis.ResolveTableConstraints" ::
"org.apache.spark.sql.catalyst.expressions.ExtractSemiStructuredFields" ::

"org.apache.spark.sql.catalyst.optimizer.TransposeWindow" ::
"org.apache.spark.sql.catalyst.optimizer.EvalInlineTables" ::

"org.apache.spark.sql.catalyst.analysis.AnsiTypeCoercion$AnsiCombinedTypeCoercionRule" ::

Does this PR introduce any user-facing change?

No behavior change.

How was this patch tested?

Pass the CIs.

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

No.

@github-actions github-actions bot added the SQL label Oct 1, 2025
@dongjoon-hyun
Copy link
Member Author

cc @gengliangwang

@dongjoon-hyun
Copy link
Member Author

Thank you, @peter-toth and @sarutak .

Merged to master for Apache Spark 4.1.0-preview3.

@dongjoon-hyun dongjoon-hyun deleted the SPARK-53773 branch October 1, 2025 14:58
@gengliangwang
Copy link
Member

Late LGTM 👍

@dongjoon-hyun
Copy link
Member Author

Thank you, @gengliangwang !

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.

4 participants