Skip to content

refactor(table): remove TableSource abstraction, materialize to pa.Ta… - #423

Merged
danqi437 merged 2 commits into
mainfrom
danqi/revert_table
Aug 5, 2026
Merged

refactor(table): remove TableSource abstraction, materialize to pa.Ta…#423
danqi437 merged 2 commits into
mainfrom
danqi/revert_table

Conversation

@danqi437

@danqi437 danqi437 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

…ble eagerly

The store.save path pickles its argument directly via FileSystemBackend.put. When the value was a TableValue returned by run_sql, its _data was a QueryTableSource holding a live DuckDBPyRelation + DuckDBPyConnection, neither picklable — so save aborted with "cannot pickle 'DuckDBPyRelation' object".

Remove the lazy TableSource abstraction so TableValue._data is always a picklable pa.Table and store.save persists real results:

  • Drop TableSource, FileTableSource, QueryTableSource, DuckDBState, ParquetReader and the fix(table): enforce declared schema for file registration #422 DuckDB-CAST helpers
  • run_sql_impl: fresh DuckDB conn per call, register pa.Table inputs, materialize the result via fetch_arrow_table()
  • read_impl: read files straight into pa.Table, enforcing declared schema at read time (parse-time column_types for CSV, post-read cast otherwise)
  • write_impl: drop the TableSource branch; inputs are always pa.Table
  • Simplify TableValue to WrapValue[pa.Table] (no lazy materialization)

Tradeoff: file reads now load fully into memory; DuckDB connections are no longer reused across run_sql calls.

…ble eagerly

The store.save path pickles its argument directly via FileSystemBackend.put.
When the value was a TableValue returned by run_sql, its _data was a
QueryTableSource holding a live DuckDBPyRelation + DuckDBPyConnection, neither
picklable — so save aborted with "cannot pickle 'DuckDBPyRelation' object".

Remove the lazy TableSource abstraction so TableValue._data is always a
picklable pa.Table and store.save persists real results:
- Drop TableSource, FileTableSource, QueryTableSource, DuckDBState, ParquetReader
  and the #422 DuckDB-CAST helpers
- run_sql_impl: fresh DuckDB conn per call, register pa.Table inputs, materialize
  the result via fetch_arrow_table()
- read_impl: read files straight into pa.Table, enforcing declared schema at
  read time (parse-time column_types for CSV, post-read cast otherwise)
- write_impl: drop the TableSource branch; inputs are always pa.Table
- Simplify TableValue to WrapValue[pa.Table] (no lazy materialization)

Tradeoff: file reads now load fully into memory; DuckDB connections are no
longer reused across run_sql calls.
@danqi437
danqi437 requested a review from a team as a code owner August 4, 2026 10:01
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

Comment thread mplang/backends/table_impl.py

@oeqqwq oeqqwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@oeqqwq oeqqwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@oeqqwq oeqqwq left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@danqi437
danqi437 merged commit 71ef549 into main Aug 5, 2026
11 checks passed
@danqi437
danqi437 deleted the danqi/revert_table branch August 5, 2026 06:16
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants