Skip to content

Commit 486f41d

Browse files
BtXinclaude
andauthored
Allow pipeline parameter binding for k in lance_knn and pg_knn (#76)
lance_knn and pg_knn rejected non-literal k at planning time, so pipeline `{k}` parameters (which surface as NULL during schema inference) could not be used — unlike sqlite_knn which already had a NULL fallback. Hoist the parsing into a shared knn_utils::extract_k helper that handles the NULL inference placeholder and the MAX_KNN_K bound check, and adopt it in all three KNN table functions. Update the lance/pg demo pipelines and their READMEs to take k (and keep limit where it was already a parameter). Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b107429 commit 486f41d

20 files changed

Lines changed: 114 additions & 94 deletions

File tree

crates/skardi/src/sources/providers/knn_utils.rs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,48 @@ pub const MAX_KNN_K: usize = 500;
99
use arrow::array::{
1010
Array, BinaryArray, FixedSizeListArray, Float32Array, Float64Array, ListArray, StringArray,
1111
};
12+
use datafusion::common::{ScalarValue, plan_err};
1213
use datafusion::error::{DataFusionError, Result as DFResult};
1314
use datafusion::execution::TaskContext;
15+
use datafusion::logical_expr::Expr;
1416
use datafusion::physical_plan::{ExecutionPlan, execute_stream};
1517
use futures::StreamExt;
1618
use std::sync::Arc;
1719

20+
/// Extract the `k` argument for a KNN table function from a planning-time expression.
21+
///
22+
/// Accepts positive integer literals (`Int32`, `Int64`, `UInt64`) and enforces the
23+
/// global [`MAX_KNN_K`] upper bound. During pipeline schema inference, pipeline
24+
/// `{param}` placeholders are converted to SQL `?` which reach the table function
25+
/// as `ScalarValue::Null` — in that case we return a dummy value of `1` so inference
26+
/// succeeds. The real integer is substituted textually at request time before
27+
/// re-planning, so the dummy value never runs.
28+
///
29+
/// `fn_name` is used as the error-message prefix (e.g. `"sqlite_knn"`).
30+
pub fn extract_k(expr: &Expr, fn_name: &str) -> DFResult<usize> {
31+
let k = match expr {
32+
Expr::Literal(ScalarValue::Int64(Some(v @ 1..)), _) => *v as usize,
33+
Expr::Literal(ScalarValue::Int64(Some(v)), _) => {
34+
return plan_err!("{fn_name}: k must be a positive integer, got {v}");
35+
}
36+
Expr::Literal(ScalarValue::Int32(Some(v @ 1..)), _) => *v as usize,
37+
Expr::Literal(ScalarValue::Int32(Some(v)), _) => {
38+
return plan_err!("{fn_name}: k must be a positive integer, got {v}");
39+
}
40+
Expr::Literal(ScalarValue::UInt64(Some(v)), _) if *v > 0 => *v as usize,
41+
// Pipeline `{k}` parameter arrives as `?` → Null during schema inference.
42+
// Return a dummy value; the real k is substituted textually at execute time.
43+
Expr::Literal(ScalarValue::Null, _) => 1,
44+
_ => {
45+
return plan_err!("{fn_name}: k must be a positive integer literal");
46+
}
47+
};
48+
if k > MAX_KNN_K {
49+
return plan_err!("{fn_name}: k must be between 1 and {MAX_KNN_K}, got {k}");
50+
}
51+
Ok(k)
52+
}
53+
1854
/// Execute a child plan and extract the first row's first column as `Vec<f32>`.
1955
///
2056
/// Returns `None` if the plan produces no rows (e.g. a subquery filter matched nothing).

crates/skardi/src/sources/providers/lance/knn_table_function.rs

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use std::sync::Arc;
2929

3030
use super::knn_exec::LanceKnnExec;
3131
use super::utils::expr_to_lance_sql;
32-
use crate::sources::providers::knn_utils::MAX_KNN_K;
32+
use crate::sources::providers::knn_utils::extract_k;
3333
use crate::sources::providers::{DatasetEntry, DatasetRegistry};
3434

3535
/// Table function that creates KNN search on Lance tables
@@ -56,13 +56,7 @@ impl TableFunctionImpl for LanceKnnTableFunction {
5656
// Extract string arguments
5757
let table_name = extract_string(&exprs[0], "table_name")?;
5858
let vector_column = extract_string(&exprs[1], "vector_column")?;
59-
let k = {
60-
let k = extract_int(&exprs[3], "k")?;
61-
if k == 0 || k > MAX_KNN_K {
62-
return plan_err!("lance_knn: k must be between 1 and {MAX_KNN_K}, got {k}");
63-
}
64-
k
65-
};
59+
let k = extract_k(&exprs[3], "lance_knn")?;
6660
let filter = if exprs.len() == 5 {
6761
Some(extract_string(&exprs[4], "filter")?)
6862
} else {
@@ -244,15 +238,6 @@ fn extract_string(expr: &Expr, name: &str) -> DFResult<String> {
244238
}
245239
}
246240

247-
fn extract_int(expr: &Expr, name: &str) -> DFResult<usize> {
248-
match expr {
249-
Expr::Literal(ScalarValue::Int64(Some(n)), _) => Ok(*n as usize),
250-
Expr::Literal(ScalarValue::Int32(Some(n)), _) => Ok(*n as usize),
251-
Expr::Literal(ScalarValue::UInt64(Some(n)), _) => Ok(*n as usize),
252-
_ => plan_err!("lance_knn: {} must be an integer literal", name),
253-
}
254-
}
255-
256241
fn try_extract_vector(expr: &Expr) -> DFResult<Option<ArrayRef>> {
257242
match expr {
258243
Expr::Literal(ScalarValue::List(arr), _) => {

crates/skardi/src/sources/providers/sqlite/knn_table_function.rs

Lines changed: 2 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use tokio_rusqlite::Connection;
3636

3737
use super::knn_exec::SqliteKnnExec;
3838
use super::{expr_to_sqlite_sql, extract_string};
39-
use crate::sources::providers::knn_utils::MAX_KNN_K;
39+
use crate::sources::providers::knn_utils::extract_k;
4040
use crate::sources::providers::{DatasetEntry, DatasetRegistry};
4141

4242
/// Entry stored in the registry for each registered SQLite table.
@@ -76,7 +76,7 @@ impl TableFunctionImpl for SqliteKnnTableFunction {
7676

7777
let table_name = extract_string(&exprs[0], "table")?;
7878
let vector_col = extract_string(&exprs[1], "vector_col")?;
79-
let k = extract_k(&exprs[3])?;
79+
let k = extract_k(&exprs[3], "sqlite_knn")?;
8080

8181
// Try to extract a literal vector.
8282
let literal_vector = extract_vector(&exprs[2]).ok();
@@ -259,27 +259,6 @@ pub fn register_sqlite_knn_udtf(ctx: &SessionContext, registry: DatasetRegistry)
259259

260260
// ─── Argument extraction helpers ─────────────────────────────────────────────
261261

262-
fn extract_k(expr: &Expr) -> DFResult<usize> {
263-
let k = match expr {
264-
Expr::Literal(ScalarValue::Int64(Some(v @ 1..)), _) => Ok(*v as usize),
265-
Expr::Literal(ScalarValue::Int64(Some(v)), _) => {
266-
plan_err!("sqlite_knn: k must be a positive integer, got {}", v)
267-
}
268-
Expr::Literal(ScalarValue::Int32(Some(v @ 1..)), _) => Ok(*v as usize),
269-
Expr::Literal(ScalarValue::Int32(Some(v)), _) => {
270-
plan_err!("sqlite_knn: k must be a positive integer, got {}", v)
271-
}
272-
Expr::Literal(ScalarValue::UInt64(Some(v)), _) => Ok(*v as usize),
273-
// NULL placeholder during schema inference
274-
Expr::Literal(ScalarValue::Null, _) => Ok(1),
275-
_ => plan_err!("sqlite_knn: k must be a positive integer literal"),
276-
}?;
277-
if k > MAX_KNN_K {
278-
return plan_err!("sqlite_knn: k must be between 1 and {MAX_KNN_K}, got {k}");
279-
}
280-
Ok(k)
281-
}
282-
283262
fn extract_vector(expr: &Expr) -> DFResult<Vec<f32>> {
284263
let values: Arc<dyn arrow::array::Array> = match expr {
285264
Expr::Literal(ScalarValue::List(arr), _) => {

crates/skardi/src/sources/providers/sqlx/pg/knn_table_function.rs

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ use std::sync::Arc;
4141

4242
use super::knn_exec::{DistanceMetric, PgKnnExec, PgVectorFetchExec};
4343
use super::utils::expr_to_pg_sql;
44-
use crate::sources::providers::knn_utils::MAX_KNN_K;
44+
use crate::sources::providers::knn_utils::extract_k;
4545
use crate::sources::providers::{DatasetEntry, DatasetRegistry};
4646

4747
/// Entry stored in the registry for each registered Postgres table.
@@ -89,7 +89,7 @@ impl TableFunctionImpl for PgKnnTableFunction {
8989
.map_err(datafusion::error::DataFusionError::Plan)?
9090
};
9191

92-
let k = extract_k(&exprs[4])?;
92+
let k = extract_k(&exprs[4], "pg_knn")?;
9393

9494
let inline_filter = if exprs.len() == 6 {
9595
extract_string(&exprs[5], "filter").ok()
@@ -467,19 +467,6 @@ fn pg_type_to_arrow(
467467

468468
// ─── Argument extraction helpers ─────────────────────────────────────────────
469469

470-
fn extract_k(expr: &Expr) -> DFResult<usize> {
471-
let k = match expr {
472-
Expr::Literal(ScalarValue::Int64(Some(n)), _) => Ok(*n as usize),
473-
Expr::Literal(ScalarValue::Int32(Some(n)), _) => Ok(*n as usize),
474-
Expr::Literal(ScalarValue::UInt64(Some(n)), _) => Ok(*n as usize),
475-
_ => plan_err!("pg_knn: k must be a positive integer literal"),
476-
}?;
477-
if k == 0 || k > MAX_KNN_K {
478-
return plan_err!("pg_knn: k must be between 1 and {MAX_KNN_K}, got {k}");
479-
}
480-
Ok(k)
481-
}
482-
483470
fn extract_string(expr: &Expr, name: &str) -> DFResult<String> {
484471
match expr {
485472
Expr::Literal(ScalarValue::Utf8(Some(s)), _) => Ok(s.clone()),

demo/embeddings/candle/README.md

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,12 @@ cargo run --bin skardi-server --features candle -- \
9999
curl -X POST http://localhost:8080/semantic-search/execute \
100100
-H "Content-Type: application/json" \
101101
-d '{
102-
"query": "how does similarity search work in vector databases?"
102+
"query": "how does similarity search work in vector databases?",
103+
"k": 10
103104
}' | jq .
104105
```
105106

106-
**Response** (truncated — returns up to 10 results):
107+
**Response** (truncated — returns up to `k` results):
107108
```json
108109
{
109110
"id": 1,
@@ -173,24 +174,25 @@ curl -X POST http://localhost:8080/semantic-search/execute \
173174
# Retrieval-Augmented Generation
174175
curl -X POST http://localhost:8080/semantic-search/execute \
175176
-H "Content-Type: application/json" \
176-
-d '{"query": "how to ground LLM responses with retrieved documents"}' | jq .
177+
-d '{"query": "how to ground LLM responses with retrieved documents", "k": 10}' | jq .
177178

178179
# Arrow / columnar formats
179180
curl -X POST http://localhost:8080/semantic-search/execute \
180181
-H "Content-Type: application/json" \
181-
-d '{"query": "columnar data formats for analytics"}' | jq .
182+
-d '{"query": "columnar data formats for analytics", "k": 5}' | jq .
182183

183184
# Model quantization
184185
curl -X POST http://localhost:8080/semantic-search/execute \
185186
-H "Content-Type: application/json" \
186-
-d '{"query": "running models on CPU without a GPU"}' | jq .
187+
-d '{"query": "running models on CPU without a GPU", "k": 5}' | jq .
187188
```
188189

189190
## Pipeline Parameters
190191

191192
| Parameter | Type | Required | Description |
192193
|---|---|---|---|
193194
| `query` | string | Yes | Free-text search query |
195+
| `k` | integer | Yes | Number of nearest neighbours to return |
194196

195197
## Directory Layout
196198

demo/embeddings/candle/pipelines/pipeline_semantic_search.yaml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,16 @@ metadata:
77
the nearest documents by cosine similarity.
88
author: Skardi Demo
99

10+
# Parameters:
11+
# {query} - Natural language search query (embedded on the fly)
12+
# {k} - Number of nearest neighbours to return
13+
1014
query: |
1115
SELECT id, title, content, _distance
1216
FROM lance_knn(
1317
'doc_embeddings',
1418
'embedding',
1519
candle('models/generated/bge-small-en-v1.5', {query}),
16-
10
20+
{k}
1721
)
1822
ORDER BY _distance
19-
LIMIT 10

demo/embeddings/gguf/README.md

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ cargo run --bin skardi-server --features gguf -- \
104104
curl -X POST http://localhost:8080/semantic-search-gguf/execute \
105105
-H "Content-Type: application/json" \
106106
-d '{
107-
"query": "how does similarity search work in vector databases?"
107+
"query": "how does similarity search work in vector databases?",
108+
"k": 10
108109
}' | jq .
109110
```
110111

@@ -186,24 +187,25 @@ curl -X POST http://localhost:8080/semantic-search-gguf/execute \
186187
# Retrieval-Augmented Generation
187188
curl -X POST http://localhost:8080/semantic-search-gguf/execute \
188189
-H "Content-Type: application/json" \
189-
-d '{"query": "how to ground LLM responses with retrieved documents"}' | jq .
190+
-d '{"query": "how to ground LLM responses with retrieved documents", "k": 10}' | jq .
190191

191192
# GGUF and quantisation
192193
curl -X POST http://localhost:8080/semantic-search-gguf/execute \
193194
-H "Content-Type: application/json" \
194-
-d '{"query": "running quantised models on CPU without a GPU"}' | jq .
195+
-d '{"query": "running quantised models on CPU without a GPU", "k": 5}' | jq .
195196

196197
# Arrow / columnar formats
197198
curl -X POST http://localhost:8080/semantic-search-gguf/execute \
198199
-H "Content-Type: application/json" \
199-
-d '{"query": "columnar data formats for analytics"}' | jq .
200+
-d '{"query": "columnar data formats for analytics", "k": 5}' | jq .
200201
```
201202

202203
## Pipeline Parameters
203204

204205
| Parameter | Type | Required | Description |
205206
|---|---|---|---|
206207
| `query` | string | Yes | Free-text search query |
208+
| `k` | integer | Yes | Number of nearest neighbours to return |
207209

208210
## Directory Layout
209211

demo/embeddings/gguf/pipelines/pipeline_semantic_search_gguf.yaml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,16 @@ metadata:
88
the nearest documents by cosine similarity.
99
author: Skardi Demo
1010

11+
# Parameters:
12+
# {query} - Natural language search query (embedded on the fly)
13+
# {k} - Number of nearest neighbours to return
14+
1115
query: |
1216
SELECT id, title, content, _distance
1317
FROM lance_knn(
1418
'doc_embeddings_gguf',
1519
'embedding',
1620
gguf('models/generated/embeddinggemma-300m', {query}),
17-
10
21+
{k}
1822
)
1923
ORDER BY _distance
20-
LIMIT 10

demo/embeddings/remote/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ cargo run --bin skardi-server --features remote-embed -- \
4545
```bash
4646
curl -s "http://localhost:8080/semantic-search-remote/execute" \
4747
-H 'Content-Type: application/json' \
48-
-d '{"query": "how does semantic search work?"}' | jq .
48+
-d '{"query": "how does semantic search work?", "k": 10}' | jq .
4949
```
5050

5151
**Response**:

demo/embeddings/remote/pipelines/pipeline_semantic_search_remote.yaml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,16 @@ metadata:
77
OpenAI API; lance_knn() finds the nearest documents by cosine similarity.
88
author: Skardi Demo
99

10+
# Parameters:
11+
# {query} - Natural language search query (embedded on the fly)
12+
# {k} - Number of nearest neighbours to return
13+
1014
query: |
1115
SELECT id, title, content, _distance
1216
FROM lance_knn(
1317
'doc_embeddings_openai',
1418
'embedding',
1519
remote_embed('openai', 'text-embedding-3-small', {query}),
16-
10
20+
{k}
1721
)
1822
ORDER BY _distance
19-
LIMIT 10

0 commit comments

Comments
 (0)