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

feat: add url_download/url_upload as sql function #3575

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

frankliee
Copy link

@frankliee frankliee commented Dec 14, 2024

Currently, download_url/upload_url only support DataFrame API, this PR adds download_url/upload_url to sql, which could be used for image processing demo.

df = daft.from_pydict({"urls": [
    "https://user-images.githubusercontent.com/17691182/190476440-28f29e87-8e3b-41c4-9c28-e112e595f558.png",
    "https://user-images.githubusercontent.com/17691182/190476440-28f29e87-8e3b-41c4-9c28-e112e595f558.png",
    "https://user-images.githubusercontent.com/17691182/190476440-28f29e87-8e3b-41c4-9c28-e112e595f558.png",
]})
df = daft.sql("SELECT image_decode(url_download(urls)) FROM df")
df.show()

@github-actions github-actions bot added the feat label Dec 14, 2024
Copy link

codspeed-hq bot commented Dec 14, 2024

CodSpeed Performance Report

Merging #3575 will degrade performances by 19.68%

Comparing frankliee:sql-url (655307a) with main (95a61d2)

Summary

❌ 1 regressions
✅ 26 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main frankliee:sql-url Change
test_iter_rows_first_row[100 Small Files] 232.2 ms 289.1 ms -19.68%

Copy link

codecov bot commented Dec 14, 2024

Codecov Report

Attention: Patch coverage is 55.55556% with 52 lines in your changes missing coverage. Please review.

Project coverage is 77.76%. Comparing base (95a61d2) to head (655307a).

Files with missing lines Patch % Lines
src/daft-sql/src/modules/url.rs 55.17% 52 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3575      +/-   ##
==========================================
- Coverage   77.79%   77.76%   -0.04%     
==========================================
  Files         716      717       +1     
  Lines       87989    88106     +117     
==========================================
+ Hits        68451    68515      +64     
- Misses      19538    19591      +53     
Files with missing lines Coverage Δ
src/daft-functions/src/uri/download.rs 83.46% <ø> (ø)
src/daft-functions/src/uri/mod.rs 100.00% <ø> (ø)
src/daft-functions/src/uri/upload.rs 65.24% <ø> (+0.60%) ⬆️
src/daft-sql/src/functions.rs 81.85% <100.00%> (+0.08%) ⬆️
src/daft-sql/src/modules/url.rs 55.17% <55.17%> (ø)

... and 2 files with indirect coverage changes

@frankliee frankliee closed this Dec 15, 2024
@frankliee frankliee reopened this Dec 15, 2024
@frankliee frankliee changed the title feat: add download_url/upload_url to sql feat: add download_url/upload_url as sql function Dec 15, 2024
@frankliee frankliee changed the title feat: add download_url/upload_url as sql function feat: add url_download/url_upload as sql function Dec 16, 2024
@jaychia
Copy link
Contributor

jaychia commented Dec 17, 2024

@andrewgazelka could you help get this through?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants