Skip to content

Commit 426bb1a

Browse files
committed
[client] Fix KV limit-scan decode + enforce limit, support schema evolution
1 parent 1aa24ac commit 426bb1a

11 files changed

Lines changed: 1025 additions & 243 deletions

File tree

crates/fluss/src/client/table/batch_scanner.rs

Lines changed: 521 additions & 189 deletions
Large diffs are not rendered by default.

crates/fluss/src/client/table/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ mod scanner;
3636
mod upsert;
3737

3838
pub use append::{AppendWriter, TableAppend};
39-
pub use batch_scanner::BatchScanner;
39+
pub use batch_scanner::LimitBatchScanner;
4040
pub use lookup::{LookupResult, Lookuper, PrefixKeyLookuper, TableLookup, TablePrefixLookup};
4141
pub use reader::{RecordBatchLogReader, SyncRecordBatchLogReader};
4242
pub use remote_log::{

crates/fluss/src/client/table/scanner.rs

Lines changed: 59 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,11 @@
1515
// specific language governing permissions and limitations
1616
// under the License.
1717

18+
use crate::client::ClientSchemaGetter;
1819
use crate::client::connection::FlussConnection;
19-
use crate::client::table::batch_scanner::BatchScanner;
2020
use crate::client::credentials::SecurityTokenManager;
2121
use crate::client::metadata::Metadata;
22+
use crate::client::table::batch_scanner::LimitBatchScanner;
2223
use crate::client::table::log_fetch_buffer::{
2324
CompletedFetch, DefaultCompletedFetch, FetchErrorAction, FetchErrorContext, FetchErrorLogLevel,
2425
LogFetchBuffer, RemotePendingFetch,
@@ -27,7 +28,9 @@ use crate::client::table::remote_log::{RemoteLogDownloader, RemoteLogFetchInfo};
2728
use crate::config::Config;
2829
use crate::error::Error::UnsupportedOperation;
2930
use crate::error::{ApiError, Error, FlussError, Result};
30-
use crate::metadata::{LogFormat, PhysicalTablePath, RowType, TableBucket, TableInfo, TablePath};
31+
use crate::metadata::{
32+
LogFormat, PhysicalTablePath, RowType, SchemaInfo, TableBucket, TableInfo, TablePath,
33+
};
3134
use crate::metrics::ScannerMetrics;
3235
use crate::proto::{
3336
ErrorResponse, FetchLogRequest, FetchLogResponse, PbFetchLogReqForBucket, PbFetchLogReqForTable,
@@ -71,10 +74,10 @@ impl<'a> TableScan<'a> {
7174
}
7275
}
7376

74-
/// Sets a row limit for the scan, enabling [`Self::create_batch_scanner`].
77+
/// Sets a row limit for the scan, enabling [`Self::create_bucket_batch_scanner`].
7578
///
76-
/// The limit must be positive. Callers configure a limit prior to
77-
/// constructing a `BatchScanner` for a one-shot bounded read.
79+
/// The limit must be positive. A limit is incompatible with the log
80+
/// scanners, which reject it.
7881
pub fn limit(mut self, n: i32) -> Result<Self> {
7982
if n <= 0 {
8083
return Err(Error::IllegalArgument {
@@ -85,17 +88,31 @@ impl<'a> TableScan<'a> {
8588
Ok(self)
8689
}
8790

88-
/// Creates a `BatchScanner` that performs a single bounded scan of `table_bucket`.
91+
/// Log scanners don't support limit pushdown; reject a configured limit
92+
/// rather than silently ignoring it.
93+
fn reject_limit(&self, scanner: &str) -> Result<()> {
94+
if let Some(limit) = self.limit {
95+
return Err(Error::UnsupportedOperation {
96+
message: format!(
97+
"{scanner} doesn't support limit pushdown. Table: {}, requested limit: {limit}",
98+
self.table_info.table_path
99+
),
100+
});
101+
}
102+
Ok(())
103+
}
104+
105+
/// Creates a one-shot bounded scan of `table_bucket`.
89106
///
90-
/// Requires a previously-configured limit via [`Self::limit`]. The scanner sends
91-
/// a `LimitScanRequest` eagerly and exposes the resulting batch through
92-
/// [`BatchScanner::poll_batch`].
93-
pub async fn create_batch_scanner(
107+
/// Requires a previously-configured limit via [`Self::limit`]. Creation is
108+
/// cheap; the `LimitScanRequest` runs on the first
109+
/// [`LimitBatchScanner::next_batch`].
110+
pub fn create_bucket_batch_scanner(
94111
self,
95112
table_bucket: TableBucket,
96-
) -> Result<BatchScanner> {
113+
) -> Result<LimitBatchScanner> {
97114
let limit = self.limit.ok_or_else(|| Error::IllegalArgument {
98-
message: "create_batch_scanner requires a limit configured via .limit(n)"
115+
message: "create_bucket_batch_scanner requires a limit configured via .limit(n)"
99116
.to_string(),
100117
})?;
101118
if table_bucket.table_id() != self.table_info.table_id {
@@ -107,15 +124,40 @@ impl<'a> TableScan<'a> {
107124
),
108125
});
109126
}
110-
BatchScanner::new(
127+
let num_buckets = self.table_info.get_num_buckets();
128+
if table_bucket.bucket_id() < 0 || table_bucket.bucket_id() >= num_buckets {
129+
return Err(Error::IllegalArgument {
130+
message: format!(
131+
"Bucket id {} out of range for table with {num_buckets} buckets",
132+
table_bucket.bucket_id()
133+
),
134+
});
135+
}
136+
// Log tables decode as Arrow IPC, so only ARROW format is supported (KV
137+
// tables use the value-record path and are exempt).
138+
if !self.table_info.has_primary_key() {
139+
validate_scan_support(&self.table_info.table_path, &self.table_info)?;
140+
}
141+
// Pre-seed the current schema; older versions are fetched lazily during
142+
// KV decode. Mirrors `Table::new_lookup`.
143+
let latest = SchemaInfo::new(
144+
self.table_info.get_schema().clone(),
145+
self.table_info.get_schema_id(),
146+
);
147+
let schema_getter = Arc::new(ClientSchemaGetter::new(
148+
self.table_info.table_path.clone(),
149+
self.conn.get_admin()?,
150+
latest,
151+
));
152+
Ok(LimitBatchScanner::new(
111153
self.conn.get_connections(),
112154
self.metadata.clone(),
113155
self.table_info,
156+
schema_getter,
114157
self.projected_fields,
115158
table_bucket,
116159
limit,
117-
)
118-
.await
160+
))
119161
}
120162

121163
/// Projects the scan to only include specified columns by their indices.
@@ -270,6 +312,7 @@ impl<'a> TableScan<'a> {
270312
}
271313

272314
pub fn create_log_scanner(self) -> Result<LogScanner> {
315+
self.reject_limit("LogScanner")?;
273316
validate_scan_support(&self.table_info.table_path, &self.table_info)?;
274317
let inner = LogScannerInner::new(
275318
&self.table_info,
@@ -284,6 +327,7 @@ impl<'a> TableScan<'a> {
284327
}
285328

286329
pub fn create_record_batch_log_scanner(self) -> Result<RecordBatchLogScanner> {
330+
self.reject_limit("RecordBatchLogScanner")?;
287331
validate_scan_support(&self.table_info.table_path, &self.table_info)?;
288332
let inner = LogScannerInner::new(
289333
&self.table_info,

crates/fluss/src/record/kv/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ mod kv_record_batch;
2222
mod kv_record_batch_builder;
2323
mod kv_record_read_context;
2424
mod read_context;
25+
mod value_record_batch;
2526

2627
#[cfg(test)]
2728
mod test_util;
@@ -31,6 +32,7 @@ pub use kv_record_batch::*;
3132
pub use kv_record_batch_builder::*;
3233
pub use kv_record_read_context::{KvRecordReadContext, SchemaGetter};
3334
pub use read_context::ReadContext;
35+
pub use value_record_batch::ValueRecordBatch;
3436

3537
/// Current KV magic value
3638
pub const CURRENT_KV_MAGIC_VALUE: u8 = 0;
Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,188 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
18+
//! Reader for the value-record batch returned by a KV (primary-key) limit
19+
//! scan. This is a distinct wire format from [`super::KvRecordBatch`]: it
20+
//! carries value-only records (no keys, no CRC/writer-id header) and a schema
21+
//! id *per record* rather than per batch.
22+
//!
23+
//! Batch layout (little-endian):
24+
//! - Length => Int32 (size of everything after this field)
25+
//! - Magic => Int8
26+
//! - RecordCount => Int32
27+
//! - Records => [ValueRecord]
28+
//!
29+
//! Each `ValueRecord`:
30+
//! - Length => Int32 (size after this field: SchemaId + Value)
31+
//! - SchemaId => Int16
32+
//! - Value => row bytes
33+
//!
34+
//! Reference: `org.apache.fluss.record.DefaultValueRecordBatch` and
35+
//! `org.apache.fluss.record.DefaultValueRecord`.
36+
37+
use crate::error::{Error, Result};
38+
use byteorder::{ByteOrder, LittleEndian};
39+
use bytes::Bytes;
40+
use std::ops::Range;
41+
42+
const LENGTH_LENGTH: usize = 4;
43+
const MAGIC_LENGTH: usize = 1;
44+
const RECORD_COUNT_LENGTH: usize = 4;
45+
/// Offset of the record count within the batch header.
46+
const RECORD_COUNT_OFFSET: usize = LENGTH_LENGTH + MAGIC_LENGTH;
47+
/// Size of the batch header (`Length + Magic + RecordCount`).
48+
const RECORD_BATCH_HEADER_SIZE: usize = LENGTH_LENGTH + MAGIC_LENGTH + RECORD_COUNT_LENGTH;
49+
/// Size of a `ValueRecord`'s leading length field.
50+
const RECORD_LENGTH_LENGTH: usize = 4;
51+
52+
/// Read-only view over a serialized value-record batch.
53+
pub struct ValueRecordBatch {
54+
data: Bytes,
55+
}
56+
57+
impl ValueRecordBatch {
58+
/// Wraps raw batch bytes. The batch is expected to start at offset 0.
59+
pub fn new(data: Bytes) -> Self {
60+
Self { data }
61+
}
62+
63+
/// Number of records declared in the batch header.
64+
pub fn record_count(&self) -> Result<i32> {
65+
if self.data.len() < RECORD_BATCH_HEADER_SIZE {
66+
return Err(corrupt(format!(
67+
"value-record batch too short: {} bytes, need {} for header",
68+
self.data.len(),
69+
RECORD_BATCH_HEADER_SIZE
70+
)));
71+
}
72+
Ok(LittleEndian::read_i32(
73+
&self.data[RECORD_COUNT_OFFSET..RECORD_COUNT_OFFSET + RECORD_COUNT_LENGTH],
74+
))
75+
}
76+
77+
/// Returns one byte range per record, each spanning `[SchemaId | Value]`:
78+
/// the payload [`crate::row::FixedSchemaDecoder::decode`] expects. Index
79+
/// [`Self::data`] with a returned range to get it without copying.
80+
pub fn value_ranges(&self) -> Result<Vec<Range<usize>>> {
81+
let count = self.record_count()?;
82+
if count < 0 {
83+
return Err(corrupt(format!("invalid record count {count}")));
84+
}
85+
let mut ranges = Vec::with_capacity(count as usize);
86+
let mut pos = RECORD_BATCH_HEADER_SIZE;
87+
for i in 0..count as usize {
88+
if pos + RECORD_LENGTH_LENGTH > self.data.len() {
89+
return Err(corrupt(format!(
90+
"truncated value-record batch: record {i} length field runs past end"
91+
)));
92+
}
93+
let rec_len = LittleEndian::read_i32(&self.data[pos..pos + RECORD_LENGTH_LENGTH]);
94+
if rec_len < 0 {
95+
return Err(corrupt(format!("record {i} has negative length {rec_len}")));
96+
}
97+
let start = pos + RECORD_LENGTH_LENGTH;
98+
let end = start + rec_len as usize;
99+
if end > self.data.len() {
100+
return Err(corrupt(format!(
101+
"truncated value-record batch: record {i} payload runs past end"
102+
)));
103+
}
104+
ranges.push(start..end);
105+
pos = end;
106+
}
107+
Ok(ranges)
108+
}
109+
110+
/// The underlying batch bytes.
111+
pub fn data(&self) -> &Bytes {
112+
&self.data
113+
}
114+
}
115+
116+
fn corrupt(message: String) -> Error {
117+
Error::UnexpectedError {
118+
message,
119+
source: None,
120+
}
121+
}
122+
123+
#[cfg(test)]
124+
mod tests {
125+
use super::*;
126+
use crate::record::kv::SCHEMA_ID_LENGTH;
127+
128+
/// Build a value-record batch from `(schema_id, row_bytes)` pairs, mirroring
129+
/// the Java `DefaultValueRecordBatch.Builder` wire layout.
130+
fn build_batch(records: &[(i16, &[u8])]) -> Vec<u8> {
131+
let mut body = Vec::new();
132+
for (schema_id, row) in records {
133+
let rec_len = (SCHEMA_ID_LENGTH + row.len()) as i32;
134+
body.extend_from_slice(&rec_len.to_le_bytes());
135+
body.extend_from_slice(&schema_id.to_le_bytes());
136+
body.extend_from_slice(row);
137+
}
138+
let mut out = Vec::new();
139+
// Length covers Magic + RecordCount + body.
140+
let length = (MAGIC_LENGTH + RECORD_COUNT_LENGTH + body.len()) as i32;
141+
out.extend_from_slice(&length.to_le_bytes());
142+
out.push(0); // magic
143+
out.extend_from_slice(&(records.len() as i32).to_le_bytes());
144+
out.extend_from_slice(&body);
145+
out
146+
}
147+
148+
#[test]
149+
fn parses_record_count_and_ranges() {
150+
let raw = build_batch(&[(7, &[1, 2, 3]), (7, &[4, 5])]);
151+
let batch = ValueRecordBatch::new(Bytes::from(raw));
152+
assert_eq!(batch.record_count().unwrap(), 2);
153+
154+
let ranges = batch.value_ranges().unwrap();
155+
assert_eq!(ranges.len(), 2);
156+
// First record payload = [schema_id(2) | row(3)] = 5 bytes.
157+
let r0 = &batch.data()[ranges[0].clone()];
158+
assert_eq!(r0.len(), 5);
159+
assert_eq!(LittleEndian::read_i16(&r0[..2]), 7);
160+
assert_eq!(&r0[2..], &[1, 2, 3]);
161+
// Second record payload = [schema_id(2) | row(2)] = 4 bytes.
162+
let r1 = &batch.data()[ranges[1].clone()];
163+
assert_eq!(r1.len(), 4);
164+
assert_eq!(&r1[2..], &[4, 5]);
165+
}
166+
167+
#[test]
168+
fn empty_batch_has_no_ranges() {
169+
let raw = build_batch(&[]);
170+
let batch = ValueRecordBatch::new(Bytes::from(raw));
171+
assert_eq!(batch.record_count().unwrap(), 0);
172+
assert!(batch.value_ranges().unwrap().is_empty());
173+
}
174+
175+
#[test]
176+
fn truncated_payload_errors() {
177+
let mut raw = build_batch(&[(7, &[1, 2, 3])]);
178+
raw.truncate(raw.len() - 2); // chop into the row payload
179+
let batch = ValueRecordBatch::new(Bytes::from(raw));
180+
assert!(batch.value_ranges().is_err());
181+
}
182+
183+
#[test]
184+
fn short_header_errors() {
185+
let batch = ValueRecordBatch::new(Bytes::from(vec![0u8, 1, 2]));
186+
assert!(batch.record_count().is_err());
187+
}
188+
}

0 commit comments

Comments
 (0)