Skip to content

fix(fs): normalize local filesystem paths#335

Merged
zjw1111 merged 10 commits into
alibaba:mainfrom
zjw1111:codex/fix-filesystem-issues
Jun 5, 2026
Merged

fix(fs): normalize local filesystem paths#335
zjw1111 merged 10 commits into
alibaba:mainfrom
zjw1111:codex/fix-filesystem-issues

Conversation

@zjw1111
Copy link
Copy Markdown
Collaborator

@zjw1111 zjw1111 commented Jun 3, 2026

Purpose

Linked issue: N/A

This PR fixes several filesystem and system-field handling issues:

  • Use normalized local paths for LocalFileSystem::Rename, so file: URIs are not passed raw to ::rename.
  • Normalize local file paths through LocalFile::Create, including empty paths as the current directory and relative paths as absolute paths based on the current working directory.
  • Return LocalFile::Create, LocalFile::GetParentFile, and LocalFile::ListFiles results through std::unique_ptr<LocalFile> so callers use the validated factory and ownership is explicit.
  • Move LocalFile ownership into LocalInputStream / LocalOutputStream, and make stream creation transfer ownership with std::unique_ptr<LocalFile>.
  • Rename the normalized local path accessor from LocalFile::GetAbsolutePath to LocalFile::GetPath, and update local file tests to construct files through LocalFile::Create.
  • Move working-directory lookup into PathUtil::GetWorkingDirectory and use dynamically allocated getcwd storage for deep working directories.
  • Make ExternalPathProvider safe for concurrent path generation by replacing unsynchronized position_ mutation with an atomic round-robin counter.
  • Treat rowkind as a special/system field in SpecialFields::IsSpecialFieldName, matching the already-defined SpecialFields::RowKind() field.

Tests

  • git diff --check
  • git diff --cached --check
  • git diff --check origin/codex/fix-filesystem-issues..HEAD
  • cmake --build build --target paimon-fs-test -j64
  • ./build/release/paimon-fs-test
  • ./build/release/paimon-fs-test --gtest_filter=LocalFileTest.*
  • Earlier verification before the branch merged the latest main: cmake --build build --target paimon-common-test -j64
  • Earlier verification before the branch merged the latest main: ./build/release/paimon-common-test --gtest_filter=ExternalPathProviderTest.*:PathUtilsTest.*
  • Earlier verification before the branch merged the latest main: ./build/release/paimon-common-test --gtest_filter=SpecialFieldsTest.*
  • Earlier verification before the branch merged the latest main: ./build/release/paimon-common-test --gtest_filter=ExternalPathProviderTest.*
  • Latest cmake --build build --target paimon-common-test -j64 is currently blocked by an unrelated Parquet/Arrow API mismatch on the updated branch: WhenBufferedRanges / PreBufferRanges are not available on parquet::ParquetFileReader.

API and Format

No storage format or protocol changes. This adds an internal PathUtil::GetWorkingDirectory helper, renames an internal LocalFile path accessor to GetPath, and changes internal local-file ownership APIs to use std::unique_ptr<LocalFile>.

Documentation

No.

Generative AI tooling

Generated-by: OpenAI Codex

Copilot AI review requested due to automatic review settings June 3, 2026 08:41
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@zjw1111 zjw1111 requested a review from Copilot June 3, 2026 09:26
@zjw1111 zjw1111 changed the title fix: normalize local filesystem paths fix(fs): normalize local filesystem paths Jun 3, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Comment thread src/paimon/common/utils/path_util.cpp Outdated
lucasfang
lucasfang previously approved these changes Jun 4, 2026
Copy link
Copy Markdown
Collaborator

@lucasfang lucasfang left a comment

Choose a reason for hiding this comment

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

+1

Comment thread src/paimon/fs/local/local_file.cpp
Copy link
Copy Markdown
Collaborator

@lxy-9602 lxy-9602 left a comment

Choose a reason for hiding this comment

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

+1

@zjw1111 zjw1111 merged commit 287c0d4 into alibaba:main Jun 5, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants