All stores created using DeltaObjectStore::new have an identical object_store_url

[gruuya]

Environment

Delta-rs version: 0.7.0 (latest main)

Binding: Rust

Environment:

• Cloud provider: N/A
• OS: MacOS
• Other:

---

Bug

What happened:

Using DeltaObjectStore::new to create new object stores leads to those stores having an identical object_store_url. Consequently, in situations where multiple Delta tables are being scanned concurrently, they will overwrite each other's object stores in the registry, leading to errors.

What you expected to happen:

DeltaObjectStore::new should parse the prefix from the provided location url, as is done in DeltaObjectStore::try_new, which will ensure that object_store_url are unique across the different tables.

How to reproduce it:
Currently, running

let tmp_dir_1 = TempDir::new("table_1").unwrap();
let location_1 = Url::from_file_path(tmp_dir_1.path()).unwrap();
let store_1 = DeltaObjectStore::new(
    Arc::from(LocalFileSystem::new_with_prefix(tmp_dir_1.path()).unwrap()),
    location_1.clone(),
);

let tmp_dir_2 = TempDir::new("table_2").unwrap();
let location_2 = Url::from_file_path(tmp_dir_2.path()).unwrap();
let store_2 = DeltaObjectStore::new(
    Arc::from(LocalFileSystem::new_with_prefix(tmp_dir_2.path()).unwrap()),
    location_2.clone(),
);
println!("Object store url 1: {}", store_1.object_store_url());
println!("Object store url 2: {}", store_2.object_store_url());

leads to:

Object store url 1: delta-rs:///
Object store url 2: delta-rs:///

More details:

[roeap]

Thanks for reporting!

While the generated URLs are right now not unique, they should distinguish at least on the table prefix / path within the store. One reason here might be that both stores are rooted directly into the deta table. The urls with no host or any other part would suggest as much.

That said, we can probably generate a more unique URl form the table state itself... Eventually we wnaat to support absolute file path in the log as well, but this is so far un-scoped work.

I'll dig a bit deeper.

[gruuya]

Hey, thank you for a fast reply!

One reason here might be that both stores are rooted directly into the deta table.

I think the reason is simply that the object_store_url is controlled solely via the prefix, which is hard-coded in case of DeltaObjectStore::new.
(Also the method signature specifies that the provided object stores should be rooted at the table root.)

What I've had in mind is something along these lines:

diff --git a/rust/src/storage/mod.rs b/rust/src/storage/mod.rs
index 360c241..ed90ce6 100644
--- a/rust/src/storage/mod.rs
+++ b/rust/src/storage/mod.rs
@@ -66,15 +66,16 @@ impl std::fmt::Display for DeltaObjectStore {
 impl DeltaObjectStore {
     /// Create a new instance of [`DeltaObjectStore`]
     ///
-    /// # Arguemnts
+    /// # Arguments
     ///
     /// * `storage` - A shared reference to an [`ObjectStore`](object_store::ObjectStore) with "/" pointing at delta table root (i.e. where `_delta_log` is located).
-    /// * `location` - A url corresponding to the storagle location of `storage`.
+    /// * `location` - A url corresponding to the storage location of `storage`.
     pub fn new(storage: Arc<DynObjectStore>, location: Url) -> Self {
+        let prefix = Path::from(location.path());
         Self {
             storage,
             location,
-            prefix: Path::from("/"),
+            prefix,
             options: HashMap::new().into(),
         }
     }

which then produces a unique object_store_url in line with the try_new:

Object store url 1: delta-rs://var-folders-b7-zvxnq_n96rj09ggrc_c5qts00000gn-T-table_1.06FYeNS7SdpL/
Object store url 2: delta-rs://var-folders-b7-zvxnq_n96rj09ggrc_c5qts00000gn-T-table_2.iWQ5DpnHBPzN/

However, that change reveals another problem, perhaps more appropriate for Discussions or the Slack workspace (which I can't seem to join). Namely it seems to me that the logic inside DeltaScan::execute is largely redundant?

What it does is simply try to overwrite the object store for the scanned table that was already registered in TableProvider::scan, but using a different store (important to note that df_url is different from object_store_url provided to Parquet's FileScanConfig inside of DeltaTable::scan).

Arguably, this makes sense in case of multi-node deployments, where the physical plan needs to be serialized and transferred in-between creation and execution. However, even then someone needs to register that df_url store that is fetched at the beginning of the DeltaScan::execute, so they might as well register the proper DeltaObjectStore under object_store_url key, in which case DeltaScan::execute logic is redundant again.

Of course I could be missing some important points here, so I'd be glad to learn more.

[gruuya]

In case it is of any help I can also present my confusion with DeltaScan::execute in a more detailed/concrete manner, using the test_datafusion_write_from_delta_scan test as an example.

Here are the noteworthy points:

1. The absolute path to the table directory in the test in my case is /Users/markogrujic/Splitgraph/delta-rs/rust/tests/data/delta-0.8.0-date/
2. The table is opened and the contained DeltaObjectStore has the prefix set to Users/markogrujic/Splitgraph/delta-rs/rust/tests/data/delta-0.8.0-date
3. A scan plan is generated which involves:
   • Registering the table's object store under the object_store_url key. The object_store_url is generated out of the prefix from above and ends up being delta-rs://Users-markogrujic-Splitgraph-delta-rs-rust-tests-data-delta-0.8.0-date
   • Creating the inner Parquet scan, and passing the object_store_url key to be used during actual execution.
   • Returning the DeltaScan wrapping the inner Parquet scan, with the url set to "file:///Users/markogrujic/Splitgraph/delta-rs/rust/tests/data/delta-0.8.0-date/"
4. Subsequently, during the execution of the DeltaScan, a ListingTableUrl is parsed out of the above url, which basically just wraps it. This url is then used for a lookup in the object store registry. However the url has path_start set to value 7, which means that the actual key for lookup that is used is file://, which is always found. Note that this corresponds neither to the object_store_url store nor to the actual root of the table, but nonetheless this issue is silent because of the points 5 and 6 below.
5. The table is constructed using the fetched object store. Since building the table ends up calling DeltaObjectStore::new the table prefix is hardcoded to /.
6. Finally, the table registers it's object store again, however thanks to the previous point the object_store_url used is delta-rs://, thus the object store that was configured in the Parquet scan (point 3) is not overridden in this case.
7. The scan finishes successfully using the object store from point 3.

This is a silent problem currently, but once I apply the diff from my previous comment this is no longer the case. Namely, everything is the same up to the points 5, 6 and 7, which now are:

5. The constructed table's storage now has a proper prefix matching the one from point 3.
6. Consequently, the object store registration now overwrites the proper object store with the one rooted at file://
7. The scan fails using the object store from point 6 with Error: ParquetError(General("AsyncChunkReader::get_metadata error: Object Store error: Object at location /part-00000-d22c627d-9655-4153-9527-f8995620fa42-c000.snappy.parquet not found: No such file or directory (os error 2)"))

UPDATE: on the off chance that DeltaScan::execute actually isn't used (or that it can be circumvented by registering an object store out-of-band under a right object_store_url), I've opened a PR for this issue: #1212