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

refactor(dwio): Rewrite partition file path unescape utility functions using folly::uriUnescape() #12036

Closed
wants to merge 1 commit into from

Conversation

darrenfu
Copy link
Contributor

@darrenfu darrenfu commented Jan 7, 2025

Summary:
Refactor the uri unescape utility function using folly:: uriUnescape

NOTE: UriEscape refactor is not part of this PR due to its underlying escape
char set is only a subset of folly::uriEscape's char set causing slightly
behavior discrepancy

Differential Revision: D67917130

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 7, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

Copy link

netlify bot commented Jan 7, 2025

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 816a542
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/67803226bc03dc00085eedee

darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 7, 2025
Summary:

To troubleshoot an unreproducable hex string unescape error in prod:
https://fburl.com/scuba/xldb_koski_queries/1o0x05d8
```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 7, 2025
…kincubator#12036)

Summary:

To troubleshoot an unreproducable hex string unescape error in prod:
https://fburl.com/scuba/xldb_koski_queries/1o0x05d8
```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

@darrenfu darrenfu changed the title print invalid hex string for troubleshooting refactor(dwio): print invalid hex string for troubleshooting Jan 7, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 7, 2025
…kincubator#12036)

Summary:

To troubleshoot an unreproducable hex string unescape error in prod:
https://fburl.com/scuba/xldb_koski_queries/1o0x05d8
```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 8, 2025
…unctions using folly::uriEscape() (facebookincubator#12036)

Summary:

Refactor the uri escape/unescape utility function to 
solve an unreproducable hex string unescape error in prod:
https://fburl.com/scuba/xldb_koski_queries/1o0x05d8
```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

@darrenfu darrenfu changed the title refactor(dwio): print invalid hex string for troubleshooting refactor(dwio): Rewrite partition file path escape/unescape utility functions using folly::uriEscape() Jan 8, 2025
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 8, 2025
…unctions using folly::uriEscape() (facebookincubator#12036)

Summary:
Pull Request resolved: facebookincubator#12036

Refactor the uri escape/unescape utility function to
solve an unreproducable hex string unescape error in prod

```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 8, 2025
…s using folly::uriUnescape() (facebookincubator#12036)

Summary:

Refactor the uri unescape utility function to 
solve an unreproducable hex string unescape error in prod

```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

1 similar comment
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 8, 2025
…s using folly::uriUnescape() (facebookincubator#12036)

Summary:
Pull Request resolved: facebookincubator#12036

Refactor the uri unescape utility function to
solve an unreproducable hex string unescape error in prod

```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 8, 2025
…s using folly::uriUnescape() (facebookincubator#12036)

Summary:

Refactor the uri unescape utility function to
solve an unreproducable hex string unescape error in prod

```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 9, 2025
…s using folly::uriUnescape() (facebookincubator#12036)

Summary:

Refactor the uri unescape utility function to
solve an unreproducable hex string unescape error in prod

```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

@darrenfu darrenfu changed the title refactor(dwio): Rewrite partition file path escape/unescape utility functions using folly::uriEscape() refactor(dwio): Rewrite partition file path unescape utility functions using folly::uriUnescape() Jan 9, 2025
darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 9, 2025
…s using folly::uriUnescape() (facebookincubator#12036)

Summary:

Refactor the uri unescape utility function using folly::uriUnescape
NOTE: UriEscape refactor is not part of this PR due to its underlying escape
char set is only a subset of folly::uriEscape's char set causing slightly
behavior discrepancy

```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
darrenfu added a commit to darrenfu/velox that referenced this pull request Jan 9, 2025
…s using folly::uriUnescape() (facebookincubator#12036)

Summary:

Refactor the uri unescape utility function to
solve an unreproducable hex string unescape error in prod

```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```

Reviewed By: spershin

Differential Revision: D67917130
@facebook-github-bot
Copy link
Contributor

@darrenfu has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

…s using folly::uriUnescape() (facebookincubator#12036)

Summary:
Refactor the uri unescape utility function using folly:: uriUnescape

NOTE: UriEscape refactor is not part of this PR due to its underlying escape 
char set is only a subset of folly::uriEscape's char set causing slightly 
behavior discrepancy


Test Plan:
```
buck test //velox/dwio/catalog/fbhive/test:file-utils-tests
```

## Koski error events before the change
```
Expression: (*__errno_location ()) != 34 && end == tmp.data() + HEX_WIDTH
Function: unescapePathName
File: fbcode/velox/dwio/catalog/fbhive/FileUtils.cpp
Line: 154
```
https://fburl.com/scuba/xldb_koski_queries/c9z0941x

Reviewed By: spershin

Differential Revision: D67917130

Pulled By: darrenfu
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D67917130

Copy link
Contributor

@spershin spershin left a comment

Choose a reason for hiding this comment

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

Looks good!

@facebook-github-bot
Copy link
Contributor

@darrenfu merged this pull request in b13e209.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants