MisterRaindrop opened a new pull request, #1991: URL: https://github.com/apache/cloudberry/pull/1991
Follows up on discussion [#1967](https://github.com/apache/cloudberry/discussions/1967), where the agreed first step was to add agent-facing retrieval tools over Cloudberry's existing capabilities, ahead of any storage-layer work. ### What does this PR do? The MCP server can describe a database and run a query, but it cannot search one. An agent has no way to find rows by meaning or by keyword, which is the capability an analytical backend has to expose before it is useful for retrieval workloads. This adds four tools: | Tool | What it does | | --- | --- | | `list_searchable_columns` | Reports embedding columns with their dimension, tsvector columns, and text columns carrying a full-text index, each with the indexes on it | | `vector_search` | Ranks rows by embedding distance over `vector`, `halfvec` or `sparsevec`, with filters pushed into the same scan that walks the index | | `fulltext_search` | Ranks rows by `ts_rank` over a text or tsvector column | | `hybrid_search` | Runs both arms and fuses them by reciprocal rank | Everything runs over Cloudberry's own storage. Nothing is cached or indexed outside the database, and no new runtime dependency is added. Identifiers are resolved against the catalog before they reach a statement and quoted afterwards; every caller-supplied value is bound as a parameter. A filter cannot inject SQL, and an unknown column produces an error listing the ones that exist. ### Details worth a reviewer's attention Each of these is a silent wrong answer rather than an error if it is got wrong, so they are called out explicitly. **Recall is not a property of the query alone.** At the default `ivfflat.probes` of 1 an index scan reads a single list and can miss the true nearest neighbour, and a selective pre-filter can then return nothing while matching rows exist. `probes` and `ef_search` are exposed, and their bounds are read from `pg_settings` rather than assumed. **Recall settings have to be session level on Cloudberry.** `SET LOCAL` and `set_config(..., true)` are not dispatched to the segments where the scan runs, and a session-level `SET` only reaches them once a query executor gang exists. A trivial distributed statement creates the gang first, and the settings are reset when the statement finishes. **The document expression is spelled the way a `to_tsvector()` index is.** No `coalesce()` wrapper, and the text search configuration written as its catalog OID. Either would read as harmless and both stop the planner matching a GIN expression index, turning every full-text search into a sequential scan. `to_tsvector` is strict, so a NULL document is already excluded without a wrapper. **A natural sentence usually matches nothing**, because PostgreSQL's query constructors require every term. `match_mode` defaults to requiring all terms and widening to any of them only when that matched nothing; the result reports which reading produced the rows. Widening is refused when the query carries a negation or a phrase, since ORing the lexemes of `latency -dashboard` would return precisely the rows the caller excluded. **Hybrid fusion keys a row on `gp_segment_id` and `ctid`**, because a ctid repeats across segments and fusing on it alone would merge unrelated rows. Plain PostgreSQL has no `gp_segment_id` and uses `ctid` by itself. ### Also fixes three existing defects These are in the query tools this work depends on: - `execute_query` and `explain_query` passed parameters to asyncpg as keyword arguments, which asyncpg does not accept, so **any call with parameters failed**. They now take a list bound to `$1, $2, ...` in order. This is a behaviour change to `params` on both tools. - `explain_query` ran `EXPLAIN ANALYZE`, which executes the statement, **with no read-only validation at all**. - `get_table_info` used a bind parameter as a `FROM` target, which is not allowed, so the function always failed. ### Type of Change - [x] Bug fix (non-breaking change) - [x] New feature (non-breaking change) - [ ] Breaking change (fix or feature with breaking changes) - [x] Documentation update ### Breaking Changes The `params` argument of `execute_query` and `explain_query` changes from an object to a list. The previous shape never worked, so no working caller is affected. `explain_query` now refuses write statements. That is a deliberate narrowing: `EXPLAIN ANALYZE` executes what it is given. ### Test Plan - [x] Unit tests added/updated - [x] Integration tests added/updated - [ ] Passed `make installcheck` - [ ] Passed `make -C src/test installcheck-cbdb-parallel` The two `make` suites cover the backend and are not affected; this PR touches only `mcp-server/`. 82 tests were added in `mcp-server/tests/test_search_tools.py`, run against a three-segment Apache Cloudberry 3.0.0-devel cluster with pgvector 0.8.0. They build and drop their own fixtures, and the module skips when no cluster or no pgvector is reachable. Coverage includes the tools and their rejection paths, NULL and empty cases, a pre-filtered ANN result checked against an exact search, recall settings reaching the segments, widening and its refusal, alias collisions, `sparsevec`, and an append-optimized column-oriented table for the fusion row key. ``` 126 passed, 9 skipped ``` Run them with: ``` cd mcp-server && pip install -e ".[dev]" DB_HOST=... DB_NAME=... DB_USER=... pytest tests/test_search_tools.py ``` ### Impact **Performance:** No change to existing paths. The new tools are index-assisted where an index exists; the generated SQL is returned with every result so it can be inspected or explained. **User-facing changes:** Four new MCP tools. The `params` change described above. **Dependencies:** None added. Vector search needs the `pgvector` extension in the target database; full-text search needs nothing. ### Checklist - [x] Followed [contribution guide](https://cloudberry.apache.org/contribute/code) - [x] Added/updated documentation - [x] Reviewed code for security implications - [x] This PR contains AI-assisted code generation - [ ] Requested review from [cloudberry committers](https://github.com/orgs/apache/teams/cloudberry-committers) ### Additional Context Scope was kept to retrieval over existing capabilities. Known gaps, listed so they are not mistaken for oversights: - No embedding generation. The caller supplies the query vector, and the server records nothing about which model produced a given column, so it is on the caller to keep them consistent. - Only the stdio transport has been exercised; streamable-http is untested here. - `hybrid_search` has not been validated on partitioned tables or views, where `ctid` is not a stable row key. `list_searchable_columns` currently reports those relkinds as searchable. - `mcp-server/` has no CI coverage in this repository. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
