feat: implement DuckDB filesystem integration for Vortex file handling by iceboundrock · Pull Request #6198 · vortex-data/vortex

iceboundrock · 2026-01-29T05:33:02Z

This PR addresses DuckDB extension to use DuckDB FileSystem #5767
This PR utilizes AI (codex from OpenAI) to generate code.

vortex-duckdb/src/scan.rs

codspeed-hq · 2026-01-29T09:48:19Z

Merging this PR will not alter performance

✅ 1135 untouched benchmarks
⏩ 1268 skipped benchmarks¹

_{Comparing Lychee-Technology:develop (20ea783) with develop (8c848c9)}

1268 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

0ax1

Thanks for taking the time to look into this. There's a couple of issues around thread safety / locking, error handling and memory leaks.

If you used AI to assist in writing the code for your PR please mention this in your PR description (see: https://github.com/vortex-data/vortex/blob/develop/CONTRIBUTING.md).

vortex-duckdb/src/copy.rs

vortex-duckdb/cpp/file_system.cpp

vortex-duckdb/src/duckdb/file_system.rs

vortex-duckdb/src/copy.rs

vortex-duckdb/src/utils/glob.rs

vortex-duckdb/src/duckdb/file_system.rs

vortex-duckdb/cpp/error.cpp

0ax1 · 2026-02-04T21:03:36Z

vortex-duckdb/src/duckdb/client_context.rs

    |_| {}
 );

+#[derive(Clone, Copy)]


Why do we need a separate type (SendableClientContext) given that Send + Sync is set on ClientContext directly now?

Because it separates ownership/lifetime semantics from thread-safe handle passing.

ClientContext is a wrapper! type with owned state and Drop logic. It is not Copy and should not be casually moved across async boundaries or spawned tasks.

On the other hand, SendableClientContext is a tiny, Copy wrapper around the raw duckdb_vx_client_context pointer with no destructor. It’s just a handle, which is ideal for passing into async or spawn_blocking code.

Fair enough. Let's add a SAFETY comment on the wrapper stating that the same guarantees apply as for ClientContext.

0ax1 · 2026-02-04T21:08:59Z

vortex-duckdb/src/scan.rs

+                vortex_bail!("GCS glob expansion not yet implemented")
+            }
+            _ => {
+                let paths = glob(file_glob.as_ref())


Shouldn't we also hook into the duckdb_fs_glob for local paths? Or asked differently, should we make any assumptions at all here about globbing details of duckdb_fs_glob but rather unconditionally delegate to duckdb? Whether or not gcs is supported is now a Duckdb implementation detail.

My suggestion is to just do this let file_urls = duckdb_fs_glob(..).

ok, I will move the local file handling logic into duckdb_fs_glob

0ax1

Almost there, thanks for taking the time to address the comments!

0ax1 · 2026-02-05T08:49:32Z

vortex-duckdb/src/scan.rs

    fn pushdown_complex_filter(
        bind_data: &mut Self::BindData,
-        expr: &duckdb::Expression,
+        expr: &crate::duckdb::Expression,


keep duckdb::Expression

Ok, I’ll rename it to just Expression and add use crate::duckdb::Expression; at the top of the file.

0ax1 · 2026-02-05T08:55:06Z

vortex-duckdb/cpp/include/duckdb_vx/file_system.h

+typedef struct {
+    const char **entries;
+    size_t count;
+} duckdb_vx_string_list;


We only use this as a URI list, and this currently lives in filesystem.hpp. Let's rename this to duckdb_vx_uri_list to be explicit.

Signed-off-by: Ruoshi <me@ruoshi.li>

iceboundrock · 2026-02-11T19:13:40Z

Almost there, thanks for taking the time to address the comments!

Hi @0ax1 , any other comments?

0ax1 · 2026-02-11T19:27:25Z

Almost there, thanks for taking the time to address the comments!

Hi @0ax1 , any other comments?

Currently swamped with other work but will get back to it when I find a free moment.

gatesn · 2026-02-13T19:35:13Z

vortex-duckdb/src/scan.rs

+    options: VortexOpenOptions,
+) -> VortexResult<VortexFile> {
+    match url.scheme() {
+        "http" | "https" | "s3" => {


Is there any reason to match only on these schemes? Should we just give everything to duckdb to handle?

make sense, I will update it

gatesn · 2026-02-13T19:39:55Z

@iceboundrock - quick question on this. Do you know if it's possible to grab the config back out from DuckDB's filesystems?

I don't want to block this PR, don't worry(!), but we do have plans to implement a high performance I/O layer at some point and it would probably make more sense to try and configure our own I/O using duckdb configs, rather than attempt to optimize calling back into DuckDB's I/O layer.

iceboundrock · 2026-02-13T21:16:55Z

@iceboundrock - quick question on this. Do you know if it's possible to grab the config back out from DuckDB's filesystems?

I don't want to block this PR, don't worry(!), but we do have plans to implement a high performance I/O layer at some point and it would probably make more sense to try and configure our own I/O using duckdb configs, rather than attempt to optimize calling back into DuckDB's I/O layer.

Could you please elaborate on what config refers to? If it’s S3 credentials, HTTP Proxy configuration, etc., ~~I don’t believe we can read them from DuckDB. They are stored in Secrets Manager and the read API redacts sensitive information.~~

We probably can do the same as the httpfs extension does. Code link:

1
2

…chemas.

AdamGS reviewed Jan 29, 2026

View reviewed changes

vortex-duckdb/src/scan.rs Outdated Show resolved Hide resolved

joseph-isaacs added the action/benchmark Trigger full benchmarks to run on this PR label Jan 29, 2026

0ax1 self-requested a review January 29, 2026 10:40

0ax1 added action/benchmark-sql Trigger SQL benchmarks to run on this PR changelog/feature A new feature and removed action/benchmark Trigger full benchmarks to run on this PR labels Jan 29, 2026

joseph-isaacs added action/benchmark-sql Trigger SQL benchmarks to run on this PR and removed action/benchmark-sql Trigger SQL benchmarks to run on this PR labels Jan 29, 2026

0ax1 reviewed Jan 29, 2026

View reviewed changes

joseph-isaacs added action/benchmark-sql Trigger SQL benchmarks to run on this PR and removed action/benchmark-sql Trigger SQL benchmarks to run on this PR labels Feb 2, 2026

0ax1 reviewed Feb 2, 2026

View reviewed changes

0ax1 reviewed Feb 4, 2026

View reviewed changes

vortex-duckdb/cpp/error.cpp Outdated Show resolved Hide resolved

0ax1 reviewed Feb 4, 2026

View reviewed changes

0ax1 reviewed Feb 5, 2026

View reviewed changes

ruoshili force-pushed the develop branch from 7ed3c29 to 66fedc0 Compare February 11, 2026 02:17

feat: implement DuckDB filesystem integration for Vortex file handling

b6e15a3

Signed-off-by: Ruoshi <me@ruoshi.li>

ruoshili force-pushed the develop branch from 66fedc0 to b6e15a3 Compare February 11, 2026 02:21

gatesn reviewed Feb 13, 2026

View reviewed changes

gatesn added action/benchmark-sql Trigger SQL benchmarks to run on this PR and removed action/benchmark-sql Trigger SQL benchmarks to run on this PR labels Feb 13, 2026

Merge branch 'vortex-data:develop' into develop

b292604

refactor: simplify open_file function by letting duckdb handles all s…

20ea783

…chemas.

Conversation

iceboundrock commented Jan 29, 2026 • edited by 0ax1 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codspeed-hq bot commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Footnotes

Uh oh!

0ax1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iceboundrock Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0ax1 Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0ax1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iceboundrock Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iceboundrock commented Feb 11, 2026

Uh oh!

0ax1 commented Feb 11, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gatesn commented Feb 13, 2026

Uh oh!

iceboundrock commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

iceboundrock commented Jan 29, 2026 •

edited by 0ax1

Loading

codspeed-hq bot commented Jan 29, 2026 •

edited

Loading

iceboundrock Feb 5, 2026 •

edited

Loading

0ax1 Feb 4, 2026 •

edited

Loading

iceboundrock Feb 5, 2026 •

edited

Loading

iceboundrock commented Feb 13, 2026 •

edited

Loading