Skip to content

[SPARK-54557] [SQL] Make CSV/JSON/XmlOptions and CSV/JSON/XmlInferSchema comparable#53268

Closed
mihailoale-db wants to merge 1 commit intoapache:masterfrom
mihailoale-db:xmlequalsimplement
Closed

[SPARK-54557] [SQL] Make CSV/JSON/XmlOptions and CSV/JSON/XmlInferSchema comparable#53268
mihailoale-db wants to merge 1 commit intoapache:masterfrom
mihailoale-db:xmlequalsimplement

Conversation

@mihailoale-db
Copy link
Contributor

What changes were proposed in this pull request?

In this PR I propose to make XmlOptions and XmlInferSchema comparable.

Why are the changes needed?

In order to be able to compare them while working on the single-pass implementation (dual-runs).

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests.

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

No.

@github-actions github-actions bot added the SQL label Dec 1, 2025
@mihailoale-db
Copy link
Contributor Author

@dtenedor @cloud-fan PTAL when you find time. Thanks!

@cloud-fan
Copy link
Contributor

shall we fix all of them like JSONOptions?

@HyukjinKwon
Copy link
Member

I wonder if we should just make them case classes

@mihailoale-db
Copy link
Contributor Author

we did the same (overriding equals and hashCode) here #50454 and here #50212 so it felt straightforward to just follow those examples. I wonder if making them case classes could somehow backfire? Is it the best practice? If not, I would keep it as it is if you agree. cc @HyukjinKwon @cloud-fan

@HyukjinKwon
Copy link
Member

OK I am fine with implementing it like this. Let's at least do the same for others though? things like JsonOptions

@mihailoale-db
Copy link
Contributor Author

Makes sense. Added CSVOptions, CSVInferSchema, JsonInferSchema and JSONOptions.

@HyukjinKwon HyukjinKwon changed the title [SPARK-54557] [SQL] Make XmlOptions and XmlInferSchema comparable [SPARK-54557] [SQL] Make CSV/JSON/XmlOptions and CSV/JSON/XmlInferSchema comparable Dec 2, 2025
@HyukjinKwon
Copy link
Member

Merged to master.

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