Skip to content

[GLUTEN-11088][VL] Add config in GlutenCastSuite to cast to char/varchar#11181

Merged
zhouyuan merged 5 commits intoapache:mainfrom
marin-ma:cast_trycast_suite
Nov 26, 2025
Merged

[GLUTEN-11088][VL] Add config in GlutenCastSuite to cast to char/varchar#11181
zhouyuan merged 5 commits intoapache:mainfrom
marin-ma:cast_trycast_suite

Conversation

@marin-ma
Copy link
Copy Markdown
Contributor

@marin-ma marin-ma commented Nov 25, 2025

Need to explicitly set spark.sql.preserveCharVarcharTypeInfo=true for gluten's test framework. We need to add this into the Gluten test because it overrides the checkEvaluation that invokes Spark's RowEncoder, which requires this configuration to be set. But in Vanilla spark, the checkEvaluation method doesn't invoke RowEncoder.

Related issue: #11088

@github-actions github-actions bot added the CORE works for Gluten Core label Nov 25, 2025
@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

1 similar comment
@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

}

testGluten("Casting to char/varchar") {
withSQLConf(SQLConf.PRESERVE_CHAR_VARCHAR_TYPE_INFO.key -> "true") {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we not support PRESERVE_CHAR_VARCHAR_TYPE_INFO false? What's the failure? If we always need to set PRESERVE_CHAR_VARCHAR_TYPE_INFO to true, may set in the plugin as default value

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If false, then exception will be thrown in Spark. We need to add this into the Gluten test because it overrides the checkEvaluation that invokes Spark's RowEncoder, which requires this configuration to be set. But in Vanilla spark, the checkEvaluation method doesn't invoke RowEncoder.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for you explaination, could you add it to PR description?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What if we set the config to all the tests in this suite? Looks like if add the new test, we still needs this config

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Then we can set it in beforeClass

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Make sense. I will make this change.

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@github-actions
Copy link
Copy Markdown

Run Gluten Clickhouse CI on x86

@marin-ma
Copy link
Copy Markdown
Contributor Author

@jinchengchenghh Do you have further comments? cc: @zhouyuan

Copy link
Copy Markdown
Contributor

@jinchengchenghh jinchengchenghh left a comment

Choose a reason for hiding this comment

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

Thanks!

@zhouyuan zhouyuan merged commit 4d20123 into apache:main Nov 26, 2025
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CORE works for Gluten Core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants