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

[#4582] fix(hadoop-catalog): Fix some location without slash issue #5296

Merged
merged 1 commit into from
Oct 28, 2024

Conversation

jerryshao
Copy link
Contributor

What changes were proposed in this pull request?

Add the trailing slash to avoid issues when user set illegal location to catalog and schema. For example, if user set "hdfs://ip:port" as a catalog or schema location, we need to add a trailing "/" to avoid issues in creating a Path.

Why are the changes needed?

Users can set Catalog and schema "location" with improper path, like "hdfs://ip:port", to support this case, we need to add an additional slash to the end of the location.

Fix: #4582

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Add a new IT to test.

@jerryshao jerryshao added the branch-0.7 Automatically cherry-pick commit to branch-0.7 label Oct 28, 2024
@jerryshao jerryshao self-assigned this Oct 28, 2024
@jerryshao jerryshao marked this pull request as ready for review October 28, 2024 06:41
@jerryshao
Copy link
Contributor Author

@shaofengshi @xloya @yuqi1129 please help to review this PR.

@jerryshao jerryshao changed the title [#4582] fix(hadoop-catalog): Fix some location withouth slash issue [#4582] fix(hadoop-catalog): Fix some location without slash issue Oct 28, 2024
@xloya
Copy link
Collaborator

xloya commented Oct 28, 2024

I think we should also add some checks when creating a catalog / schema with a location to force the user to input the correct location, WDYT?

@jerryshao
Copy link
Contributor Author

I think we can, previously, I postponed this to the fileset creation. Let me think a bit on this. Maybe we can have a simple fix here and do a thorough fix in the next version?

@xloya
Copy link
Collaborator

xloya commented Oct 28, 2024

LGTM.

Copy link
Contributor

@yuqi1129 yuqi1129 left a comment

Choose a reason for hiding this comment

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

LGTM

@yuqi1129 yuqi1129 merged commit 1998816 into apache:main Oct 28, 2024
26 checks passed
github-actions bot pushed a commit that referenced this pull request Oct 28, 2024
…5296)

### What changes were proposed in this pull request?

Add the trailing slash to avoid issues when user set illegal location to
catalog and schema. For example, if user set "hdfs://ip:port" as a
catalog or schema location, we need to add a trailing "/" to avoid
issues in creating a `Path`.

### Why are the changes needed?

Users can set Catalog and schema "location" with improper path, like
"hdfs://ip:port", to support this case, we need to add an additional
slash to the end of the location.

Fix: #4582 

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Add a new IT to test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-0.7 Automatically cherry-pick commit to branch-0.7
Projects
None yet
3 participants