Skip to content

[SPARK-58178][SQL][TEST][3.5] Expected size needs to increase#57358

Closed
holdenk wants to merge 1 commit into
apache:branch-3.5from
holdenk:SPARK-58178-fix-string-comp-in-suits
Closed

[SPARK-58178][SQL][TEST][3.5] Expected size needs to increase#57358
holdenk wants to merge 1 commit into
apache:branch-3.5from
holdenk:SPARK-58178-fix-string-comp-in-suits

Conversation

@holdenk

@holdenk holdenk commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Increases expected string size.

Why are the changes needed?

Version string embedded in results

Does this PR introduce any user-facing change?

Test only

How was this patch tested?

Previous failing test now passes

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

no

@sfc-gh-hkarau @holdenk
expected size needs to increase now that we've got 3.5.10 (one more c…
9b972d0 
…har)

Co-authored-by: Holden Karau <holden@pigscanfly.ca>
@holdenk holdenk changed the title [SPARK-58178][SQL][TEST] Expected size needs to increase [SPARK-58178][SQL][TEST][3.5] Expected size needs to increase Jul 19, 2026
pan3793
pan3793 reviewed Jul 21, 2026
View reviewed changes
Comment thread sql/hive/src/test/scala/org/apache/spark/sql/hive/StatisticsSuite.scala
sql(s"INSERT INTO $tblName VALUES (1, 'a', '2019-12-13')")

val expectedSize = 657
val expectedSize = 658

@pan3793 pan3793 Jul 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess due to 3.5.9 => 3.5.10?

uros-b
uros-b reviewed Jul 21, 2026
View reviewed changes
Comment thread sql/hive/src/test/scala/org/apache/spark/sql/hive/StatisticsSuite.scala
Seq(tbl, ext_tbl).foreach { tblName =>
sql(s"INSERT INTO $tblName VALUES (1, 'a', '2019-12-13')")

val expectedSize = 657

@uros-b uros-b Jul 21, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The fix again hardcodes another magic byte count (658), preserving the fragility that caused this issue.
It will fail again on any future version-string-length change or Parquet bump on 3.5.
A better fix (SPARK-53422) is already merged on master. Should we consider cherry picking it?

@pan3793 pan3793 Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, I wrote this patch ... Since Spark 3.5.[x] won't go to 3.5.100, I'm fine with either backporting that or this simple fix

uros-b
uros-b approved these changes Jul 21, 2026
View reviewed changes

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you @holdenk! I left one comment to consider, but otherwise LGTM

@dongjoon-hyun

dongjoon-hyun commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

I would agree with @uros-b 's comment if this is a normal branch. However, I prefer @holdenk 's approach for now.

It will fail again on any future version-string-length change or Parquet bump on 3.5.

Technically, @uros-b 's argument is invalid because

  1. We are not going to bump to 3.5.100-SNAPSHOT in the remaining branch-3.5 lifetime.
  2. Currently we are using Apache Parquet 1.13.1 and Apache Spark community is very reluctant to change Apache Parquet version from 1.13.1 to 1.13.2 in branch-3.5.

spark/pom.xml

Line 143 in 71d3aca

<parquet.version>1.13.1</parquet.version>

Moreover, Apache Parquet 1.13.2 doesn't exist and I don't think Apache Parquet community will do a new security releases for 1.13.1.

For the following, we didn't backport SPARK-53422 to branch-4.0 also. So, @uros-b 's suggestion implies to change branch-4.0 for this branch-3.5's special case. That implication is also not desirable.

A better fix (SPARK-53422) is already merged on master. S
hould we consider cherry picking it?

I'd like to recommend to merge this surgical one to branch-3.5 as the last part of @holdenk 's Apache Spark 3.5.9 release manager role because this has been broken due to the release commits.

@dongjoon-hyun

dongjoon-hyun commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Let me merge this in order to recover branch-3.5 healthy back.

For @uros-b 's suggestion, we can still cherry-pick independently on the healthy branch-3.5 (if the community needs it).

Of course, we should decide to backport it to branch-4.0 first independently (before discussing about branch-3.5)

dongjoon-hyun pushed a commit that referenced this pull request Jul 22, 2026
@holdenk @sfc-gh-hkarau @dongjoon-hyun
[SPARK-58178][3.5][SQL][TEST] Expected size needs to increase
439f584 
### What changes were proposed in this pull request?

Increases expected string size.

### Why are the changes needed?

Version string embedded in results

### Does this PR introduce _any_ user-facing change?

Test only

### How was this patch tested?

Previous failing test now passes

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

Closes #57358 from holdenk/SPARK-58178-fix-string-comp-in-suits.

Lead-authored-by: Holden Karau <holden@pigscanfly.ca>
Co-authored-by: Holden Karau <holden.karau@snowflake.com>
Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
@dongjoon-hyun

dongjoon-hyun commented Jul 22, 2026

Copy link
Copy Markdown
Member

Merge Summary:

Posted by merge_spark_pr.py

@holdenk

holdenk commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @dongjoon-hyun !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pan3793 pan3793 pan3793 approved these changes
@dongjoon-hyun dongjoon-hyun dongjoon-hyun approved these changes
@uros-b uros-b uros-b approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@holdenk @dongjoon-hyun @pan3793 @uros-b @sfc-gh-hkarau