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

Enable regular expressions by default #5591

Merged
merged 4 commits into from
May 31, 2022

Conversation

andygrove
Copy link
Contributor

@andygrove andygrove commented May 23, 2022

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

Part of #4509

We want to enable regular expressions by default in 22.06. This does not mean we are guaranteeing 100% compatibility with Spark though so we need to document the edge cases that can produce different results.

  • Change configuration default to true
  • Document all known edge cases in the compatibility guide

Signed-off-by: Andy Grove <andygrove@nvidia.com>
"true will make supported regular expressions run on the GPU. See the compatibility " +
"guide for more information about which regular expressions are supported on the GPU.")
.doc("Specifies whether supported regular expressions should be evaluated on GPU. There " +
"are some known edge cases that can produce incorrect results and these are documented in " +
Copy link
Collaborator

@gerashegalov gerashegalov May 23, 2022

Choose a reason for hiding this comment

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

should this say that known edge cases are executed on CPU regardless of the config value, at least this is the impression I have from reading compatibility guide.

"true will make supported regular expressions run on the GPU. See the compatibility " +
"guide for more information about which regular expressions are supported on the GPU.")
.doc("Specifies whether supported regular expressions should be evaluated on GPU. There " +
"are some known edge cases that can produce incorrect results and these are documented in " +
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this say that known edge cases are executed on CPU, at least this is the impression I have from reading compatibility guide.

@sameerz sameerz added the feature request New feature or request label May 26, 2022
@sameerz sameerz added this to the May 23 - Jun 3 milestone May 26, 2022
@sameerz
Copy link
Collaborator

sameerz commented May 26, 2022

I agree with @gerashegalov 's comments, and am ok with the rest of the text wording.

@andygrove
Copy link
Contributor Author

I had been holding off updating this in the hope that #5610 and #5662 would be merged and then we would not need to list any known issues in this PR. However, I am blocked from merging those at the moment due to the need to have CI specify LANG=en_US.UTF-8 before invoking the Scala unit tests so I will update this PR listing the issues that would have been solved by the other PRs so we can get this merged,

@andygrove andygrove marked this pull request as ready for review May 27, 2022 22:49
@andygrove andygrove changed the title WIP: Enable regular expressions by default Enable regular expressions by default May 27, 2022
@andygrove
Copy link
Contributor Author

build

@andygrove
Copy link
Contributor Author

build

@andygrove andygrove merged commit 9f5f284 into NVIDIA:branch-22.06 May 31, 2022
@andygrove andygrove deleted the enable-regexp branch May 31, 2022 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants