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(catalog): Defines a session for connection state #3782

Merged
merged 3 commits into from
Feb 8, 2025

Conversation

rchowell
Copy link
Contributor

@rchowell rchowell commented Feb 7, 2025

This defines an initial session for table resolution which simply wraps the existing meta catalog. The purpose of this PR is to (1) introduce a primary stateful object for planning and execution which is shared by frontends and (2) audit the planner's current state management and name resolution to see what is required once bridging over to python.

Notes

  • This PR includes many follow-up TODOs which are being converted to issues.
  • This primarily introduces indirection first to keep the diff minimal, switchovers come later.
  • The session will hold a reference to the global context singleton from refactor: port DaftContext to rust side #3767.
  • This session will be converged with the ConnectSession and made available through python once stabilized.

Copy link

codecov bot commented Feb 7, 2025

Codecov Report

Attention: Patch coverage is 96.36364% with 4 lines in your changes missing coverage. Please review.

Project coverage is 77.88%. Comparing base (7f2e9b5) to head (8d166cb).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/daft-catalog/src/error.rs 0.00% 3 Missing ⚠️
src/daft-catalog/python-catalog/src/python.rs 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3782      +/-   ##
==========================================
+ Coverage   77.86%   77.88%   +0.02%     
==========================================
  Files         740      741       +1     
  Lines       93814    93880      +66     
==========================================
+ Hits        73050    73122      +72     
+ Misses      20764    20758       -6     
Files with missing lines Coverage Δ
src/daft-catalog/src/identifier.rs 59.57% <ø> (+8.51%) ⬆️
src/daft-catalog/src/lib.rs 66.95% <100.00%> (ø)
src/daft-connect/src/config.rs 54.54% <ø> (ø)
src/daft-connect/src/connect_service.rs 64.31% <100.00%> (ø)
src/daft-connect/src/execute.rs 89.41% <100.00%> (+0.07%) ⬆️
src/daft-connect/src/response_builder.rs 100.00% <100.00%> (ø)
src/daft-connect/src/session.rs 100.00% <ø> (ø)
src/daft-connect/src/spark_analyzer.rs 74.06% <100.00%> (+0.08%) ⬆️
src/daft-session/src/lib.rs 100.00% <100.00%> (ø)
src/daft-sql/src/lib.rs 100.00% <ø> (ø)
... and 4 more

... and 3 files with indirect coverage changes

src/daft-catalog/src/errors.rs Outdated Show resolved Hide resolved
src/daft-catalog/src/session.rs Outdated Show resolved Hide resolved
src/daft-catalog/src/session.rs Outdated Show resolved Hide resolved
src/daft-catalog/src/session.rs Outdated Show resolved Hide resolved
src/daft-sql/src/planner.rs Outdated Show resolved Hide resolved
Copy link

codspeed-hq bot commented Feb 8, 2025

CodSpeed Performance Report

Merging #3782 will improve performances by 98.11%

Comparing rchowell/session (8d166cb) with main (7f2e9b5)

Summary

⚡ 1 improvements
✅ 26 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_iter_rows_first_row[100 Small Files] 220.4 ms 111.3 ms +98.11%

@rchowell rchowell merged commit 2063d21 into main Feb 8, 2025
44 checks passed
@rchowell rchowell deleted the rchowell/session branch February 8, 2025 05:08
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