[jira] [Resolved] (SPARK-58769) Document the BoundFunction equality contract and drop the name-based transform comparison

["Peter Toth (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SPARK-58769?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Peter Toth resolved SPARK-58769.
--------------------------------
    Fix Version/s: 4.4.0
       Resolution: Fixed

Issue resolved by pull request 58002
[https://github.com/apache/spark/pull/58002]

> Document the BoundFunction equality contract and drop the name-based transform comparison
> -----------------------------------------------------------------------------------------
>
>                 Key: SPARK-58769
>                 URL: https://issues.apache.org/jira/browse/SPARK-58769
>             Project: Spark
>          Issue Type: Improvement
>          Components: SQL
>    Affects Versions: 5.0.0
>            Reporter: Peter Toth
>            Assignee: Peter Toth
>            Priority: Major
>              Labels: pull-request-available
>             Fix For: 4.4.0
>
>
> Spark compares partition transform expressions with ordinary expression equality, which reaches {{BoundFunction.equals}}. It binds a function afresh every time it converts a partitioning or ordering reported by a source, so two reports of one transform - from two scans of the same table, or from the partitioning and the ordering of a single scan - hold two bound instances. A function that does not implement {{equals}}/{{hashCode}} therefore yields expressions that never compare equal across binds, so Spark cannot deduplicate them, reuse a scan reporting them, keep a union's partitioning, or recognize two subplans as the same. That costs optimizations only, never correctness.
> {{canonicalName}} is a documented obligation; {{equals}} was never mentioned. Spark cannot derive the identity itself either, because {{BoundFunction}} is an open interface and a single class can capture behaviour-affecting state at bind time (a width, a precision, a timezone, a snapshot id), so there is no list of state to compare. Only the implementation knows which of its state matters.
> This documents the contract on {{BoundFunction}}: implementations SHOULD override {{equals}}/{{hashCode}}, comparing whatever state affects behaviour; {{canonicalName}} alone is not necessarily enough; whatever is compared must be stable across bind calls; {{hashCode}} must agree. It also states the relationship between the two comparisons - {{equals}} is the finer one, so two functions that compare equal must return the same {{canonicalName}}, while the same name does not make them equal - and scopes {{canonicalName}}'s own javadoc to the question it really answers.
> With that written down, SPARK-58549's {{PlanMerger}} workaround (comparing reported transforms by {{isSameFunction}}, i.e. by canonical name with arguments ignored) is removed, so plan identity has one notion instead of two: every other place that compares a reported key-grouped partitioning or ordering already treats it as an ordinary expression ({{UnionExec.comparePartitioning}}, {{GroupPartitionsExec.outputOrdering}}, {{BatchScanExec.equals}}, {{DataSourceV2ScanRelation}}'s canonical form). A connector without {{equals}} loses that one merge and keeps missing the others exactly as before; SPARK-58549 is unreleased, so no released behaviour changes.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@spark.apache.org
For additional commands, e-mail: issues-help@spark.apache.org