Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[VL] Enable Spark legacy date formatter if spark.sql.legacy.timeParserPolicy is set to 'LEGACY' #7375

Merged
merged 2 commits into from
Oct 14, 2024

Conversation

NEUpanning
Copy link
Contributor

@NEUpanning NEUpanning commented Sep 27, 2024

What changes were proposed in this pull request?

Velox#10966 introduced simple date time formatter that aligns with Spark legacy date parsing/formatting behavior. This PR enables Velox simple date time formatter if spark.sql.legacy.timeParserPolicy is set to LEGACY.

Relates Velox issue: #10354

How was this patch tested?

Integration tests with Spark

@github-actions github-actions bot added CORE works for Gluten Core VELOX labels Sep 27, 2024
Copy link

Thanks for opening a pull request!

Could you open an issue for this pull request on Github Issues?

https://github.com/apache/incubator-gluten/issues

Then could you also rename commit message and pull request title in the following format?

[GLUTEN-${ISSUES_ID}][COMPONENT]feat/fix: ${detailed message}

See also:

Copy link

Run Gluten Clickhouse CI

@NEUpanning
Copy link
Contributor Author

@rui-mo Could you help to take a look please? Thanks.

@@ -520,6 +520,11 @@ std::unordered_map<std::string, std::string> WholeStageResultIterator::getQueryC

configs[velox::core::QueryConfig::kSparkPartitionId] = std::to_string(taskInfo_.partitionId);

// Enable Spark legacy date formatter if spark.sql.legacy.timeParserPolicy is set to 'LEGACY'.
if (veloxCfg_->get<std::string>(kSparkLegacyTimeParserPolicy, "") == "LEGACY") {
configs[velox::core::QueryConfig::kSparkLegacyDateFormatter] = "true";
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to set this config as false if 'kSparkLegacyTimeParserPolicy' is not 'LEGACY'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your feedback. It is necessary to do this. Updated.

Copy link

github-actions bot commented Oct 9, 2024

Run Gluten Clickhouse CI

@NEUpanning
Copy link
Contributor Author

@rui-mo Could you help to merge this PR? Thanks.

@rui-mo rui-mo merged commit d6e048a into apache:main Oct 14, 2024
47 checks passed
@surnaik
Copy link
Contributor

surnaik commented Nov 6, 2024

@NEUpanning does this work for spark properties
spark.sql.legacy.parquet.int96RebaseModeInRead=LEGACY
and spark.sql.legacy.parquet.datetimeRebaseModeInRead=LEGACY

@NEUpanning
Copy link
Contributor Author

NEUpanning commented Nov 7, 2024

@surnaik No, this PR only works for spark.sql.legacy.timeParserPolicy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CORE works for Gluten Core VELOX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants