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

date_format should not suggest enabling incompatibleDateFormats for formats we cannot support #2532

Merged
merged 2 commits into from
Jun 21, 2021

Conversation

andygrove
Copy link
Contributor

@andygrove andygrove commented May 27, 2021

Signed-off-by: Andy Grove andygrove@nvidia.com

Closes #2517.

@andygrove andygrove added the bug Something isn't working label May 27, 2021
@andygrove andygrove added this to the May 24 - Jun 4 milestone May 27, 2021
@andygrove andygrove self-assigned this May 27, 2021
@andygrove andygrove changed the title WIP: date_format should not suggest enabling incompatibleDateFormats for formats we cannot support date_format should not suggest enabling incompatibleDateFormats for formats we cannot support May 27, 2021
@andygrove andygrove marked this pull request as ready for review May 27, 2021 22:27
assert_gpu_and_cpu_are_equal_collect(
lambda spark : unary_op_df(spark, data_gen).selectExpr("date_format(a, '{}')".format(date_format)))

unsupported_date_formats_force = ['F']
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be duplicated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The second version of the test specifies conf = {"spark.rapids.sql.incompatibleDateFormats.enabled": "true"}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, sorry, I mean the variable unsupported_date_formats_force is assigned but not used.

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, this is fixed now. There were numerous other issues with the tests that are also resolved now.

assert_gpu_and_cpu_are_equal_collect(
lambda spark : unary_op_df(spark, data_gen).selectExpr("date_format(a, '{}')".format(date_format)))

maybe_supported_date_formats_force = ['dd-MM-yyyy']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question here

@andygrove
Copy link
Contributor Author

build

@andygrove
Copy link
Contributor Author

@revans2 @wjxiz1992 Let me know if any more changes are needed

@sameerz
Copy link
Collaborator

sameerz commented Jun 5, 2021

@andygrove can we retarget this to 21.08?

revans2
revans2 previously approved these changes Jun 7, 2021
@sameerz sameerz changed the base branch from branch-21.06 to branch-21.08 June 8, 2021 23:02
@sameerz sameerz dismissed revans2’s stale review June 8, 2021 23:02

The base branch was changed.

@sameerz
Copy link
Collaborator

sameerz commented Jun 8, 2021

Moving to branch-21.08 after @Salonijain27 confirmed with the PR author.

revans2
revans2 previously approved these changes Jun 9, 2021
integration_tests/src/main/python/date_time_test.py Outdated Show resolved Hide resolved
…ormats we cannot support

Signed-off-by: Andy Grove <andygrove@nvidia.com>
@andygrove
Copy link
Contributor Author

build

@andygrove andygrove merged commit 00e6102 into NVIDIA:branch-21.08 Jun 21, 2021
@andygrove andygrove deleted the date-fmt-compat branch June 21, 2021 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEA] date_format should not suggest enabling incompatibleDateFormats for formats we cannot support
4 participants