Gabriel39 commented on code in PR #68613:
URL: https://github.com/apache/doris/pull/68613#discussion_r4141905514
##########
thirdparty/patches/lance-c-foyer.patch:
##########
@@ -1087,24 +941,35 @@ index 0000000..9a10e43
+ }
+
+ async fn cached_get(&self, location: &Path, options: GetOptions) ->
Result<GetResult> {
-+ // Fetch metadata separately so the returned GetResult retains the
origin's identity while
-+ // its payload uses the same block cache as get_ranges(). This also
provides the object size
-+ // needed to resolve bounded, offset, and suffix ranges.
-+ let GetResult {
-+ meta: metadata,
-+ attributes,
-+ ..
-+ } = self
++ let metadata_key = self
+ .reader
-+ .original
-+ .get_opts(
-+ location,
-+ GetOptions {
-+ head: true,
-+ ..Default::default()
-+ },
-+ )
-+ .await?;
++ .cache
++ .size_key(&self.reader.store_prefix, location);
++ let cached_metadata = self.reader.cache.metadata.get(&metadata_key);
Review Comment:
Fixed in 3e9d8fd2163, regenerated from lance-c
24c7ca4bcb9422c113b0d3e07e4efe1173b0bc9f. Metadata, size, and block keys now
include a random namespace for the exact live underlying store; the weak
registry cannot keep stores alive or reuse a persisted pointer identity. The
wrapper interface does not expose a complete stable endpoint identity, so new
store instances and process restarts deliberately start cold, including fresh
Dataset opens that create a new store. This tradeoff is documented. Regressions
cover real HTTP endpoints sharing bucket/path/ETag, batched reads, NotFound,
replacement after disk recovery, and retained same-store reuse. All 461 regular
Rust tests passed. Upstream source update:
https://github.com/zhangstar333/lance-c/pull/3.
##########
thirdparty/patches/lance-c-foyer.patch:
##########
@@ -1087,24 +941,34 @@ index 0000000..9a10e43
+ }
+
+ async fn cached_get(&self, location: &Path, options: GetOptions) ->
Result<GetResult> {
-+ // Fetch metadata separately so the returned GetResult retains the
origin's identity while
-+ // its payload uses the same block cache as get_ranges(). This also
provides the object size
-+ // needed to resolve bounded, offset, and suffix ranges.
-+ let GetResult {
-+ meta: metadata,
-+ attributes,
-+ ..
-+ } = self
++ let metadata_key = self
+ .reader
-+ .original
-+ .get_opts(
-+ location,
-+ GetOptions {
-+ head: true,
-+ ..Default::default()
-+ },
-+ )
-+ .await?;
++ .cache
++ .size_key(&self.reader.store_prefix, location);
++ let cached_metadata = self.reader.cache.metadata.get(&metadata_key);
++ let (metadata, attributes, extensions) = if let Some(entry) =
cached_metadata {
Review Comment:
Fixed in 3e9d8fd2163 (lance-c source 24c7ca4). cached_get now checks the
size entry in both hybrid tiers and inserts only when absent or invalid. The
regression reopens the disk cache with a cold metadata cache, performs five
warm range reads, drains the actual disk writer, and checks disk_write_bytes.
It failed before the fix with 40960 bytes and passes after the fix with zero.
All 461 regular Rust tests passed. The source change is included in
https://github.com/zhangstar333/lance-c/pull/3.
##########
thirdparty/test/lance-prefilter-patch-test.sh:
##########
@@ -0,0 +1,109 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -euo pipefail
+
+ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." &>/dev/null && pwd)"
+ARCHIVE_DIR="${1:?Usage: $0
directory-containing-the-pinned-lance-and-lance-c-archives}"
+ARCHIVE_DIR="$(cd "${ARCHIVE_DIR}" && pwd)"
+TP_DIR="${ROOT}"
+# Load only the repository-owned archive definitions, never extracted code.
+source "${ROOT}/vars.sh"
Review Comment:
Fixed in 3e9d8fd2163. The optional ADBC source guard now uses
${ARROW_ADBC_FLIGHTSQL_SOURCE:-}, so sourcing vars.sh under nounset is safe
when that variable is absent. The harness checks simulated Darwin x86_64 and
arm64 with the variable explicitly unset. This reproduced the unbound-variable
failure before the fix; both platform checks and the full downloader lifecycle
pass afterward. This is shell-level platform coverage, not a native macOS build.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
