Skip to content

fix: correctly rectify Urls with dots in DeltaTableBuilder#3929

Merged
rtyler merged 2 commits intodelta-io:mainfrom
rtyler:fix-for-url-with-dots
Nov 12, 2025
Merged

fix: correctly rectify Urls with dots in DeltaTableBuilder#3929
rtyler merged 2 commits intodelta-io:mainfrom
rtyler:fix-for-url-with-dots

Conversation

@rtyler
Copy link
Copy Markdown
Member

@rtyler rtyler commented Nov 11, 2025

While doing some unrelated work I did some dry-by test coverage addition and discovered this issue with some of our DAT tests.

The only way I believe we can be certain our path segments don't have .. in them is to re-parse the Url 😢

…om_uri

Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>
Signed-off-by: R. Tyler Croy <rtyler@brokenco.de>
@github-actions github-actions bot added the binding/rust Issues for the Rust crate label Nov 11, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 11, 2025

Codecov Report

❌ Patch coverage is 84.12698% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.98%. Comparing base (864fbc0) to head (a76346b).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
crates/core/src/table/builder.rs 37.50% 6 Missing and 4 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3929      +/-   ##
==========================================
+ Coverage   73.94%   73.98%   +0.03%     
==========================================
  Files         153      153              
  Lines       39477    39489      +12     
  Branches    39477    39489      +12     
==========================================
+ Hits        29193    29214      +21     
+ Misses       8960     8953       -7     
+ Partials     1324     1322       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rtyler rtyler marked this pull request as ready for review November 11, 2025 16:23
@rtyler rtyler requested review from hntd187 and roeap as code owners November 11, 2025 16:23
@rtyler rtyler enabled auto-merge (rebase) November 11, 2025 16:23
Copy link
Copy Markdown
Collaborator

@hntd187 hntd187 left a comment

Choose a reason for hiding this comment

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

One question, but otherwise GTG

let fields = self
.iter()
.map(|(k, v)| {
let sep = String::from('/');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Would this clone the string on every interspersion? Pedantic here, but maybe a const SEP: &str = "/"; would make more sense?

@rtyler rtyler merged commit 8a8403b into delta-io:main Nov 12, 2025
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

binding/rust Issues for the Rust crate

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants