tustvold commented on PR #3839:
URL: https://github.com/apache/arrow-rs/pull/3839#issuecomment-1480775873

   I'm having a somewhat hard time understanding the code you've linked but 
please bear with me:
   
   `CachedBasedObjectStoreRegistry`:
     * Creates a `FileCacheObjectStore` combining a normal `ObjectStore` with a 
`FileCacheLayer`,
     * Computes a cache key from the URL
     * Creates a `ObjectStoreWithKey` to extend the `FileCacheObjectStore` with 
this key to wrap this construction
   
   Calling get on this object store then:
   
   * Calls through `ObjectStoreWithKey` to `FileCacheObjectStore`
   * Looks up the value in the cache, passing the inner store as the `GetExtra` 
argument
   * This propagates through to `FileCacheLoader::load`
   * Which then calls `CacheMedium::get_mapping_location`
   * Which finally calls through to `get_key` which attempts to downcast the 
provided `ObjectStore` to `ObjectStoreWithKey`
   * This will always fail as the `ObjectStoreWithKey` is decorating the 
`FileCacheObjectStore` not the other way round
   
   A simple fix might be to swap the decoration order, but taking a step back, 
I don't see the need for `ObjectStoreWithKey`. The cache key is known at the 
time `FileCacheObjectStore` is constructed by `CachedBasedObjectStoreRegistry`. 
I don't see a reason to not just store this key inside the 
`FileCacheObjectStore` and pass it along with the inner `ObjectStore` as the 
`GetExtra` argument to the cache. This would not only be simpler (and work) but 
would also avoid any need for downcasting?
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to