From 772519eb44b3f073a557151082960c0cffa1a0cf Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Sun, 4 Oct 2026 10:01:36 -0700 Subject: [PATCH 1/2] fix(fts): snapshot FTS aux info in bind data (Fixes ladybugdb/ladybug#1105) QueryFTSBindData held a reference into the versioned catalog, which dangles when the prepared plan is reused on a second parameterized execution on the same connection, segfaulting (Refs ladybugdb/ladybug#1082, Refs ladybugdb/ladybug#1088). Store an owned copy of FTSIndexAuxInfo instead. --- fts/src/function/query_fts_bind_data.cpp | 4 ++-- fts/src/function/query_fts_index.cpp | 5 +++-- fts/src/include/function/query_fts_bind_data.h | 14 +++++++++----- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/fts/src/function/query_fts_bind_data.cpp b/fts/src/function/query_fts_bind_data.cpp index 1e19ad0c..c40d5771 100644 --- a/fts/src/function/query_fts_bind_data.cpp +++ b/fts/src/function/query_fts_bind_data.cpp @@ -46,7 +46,7 @@ void QueryFTSOptionalParams::evaluateParams(main::ClientContext* context) { std::vector QueryFTSBindData::getQueryTerms(main::ClientContext& context) const { auto queryInStr = ExpressionUtil::evaluateLiteral(&context, query, LogicalType::STRING()); - auto config = entry.getAuxInfo().cast().config; + auto config = auxInfo.config; FTSUtils::normalizeQuery(queryInStr, config.ignorePatternQuery, true /* protectWildcardChars */); auto terms = FTSUtils::tokenizeString(queryInStr, config); @@ -57,7 +57,7 @@ std::vector QueryFTSBindData::getQueryTerms(main::ClientContext& co config.stopWordsTableName) ->getTableID()) ->ptrCast(); - return FTSUtils::stemTerms(terms, entry.getAuxInfo().cast().config, + return FTSUtils::stemTerms(terms, auxInfo.config, MemoryManager::Get(context), stopWordsTable, transaction::Transaction::Get(context), optionalParams->constCast().conjunctive.getParamVal(), true /* isQuery */); diff --git a/fts/src/function/query_fts_index.cpp b/fts/src/function/query_fts_index.cpp index 1c007e7e..501cfd43 100644 --- a/fts/src/function/query_fts_index.cpp +++ b/fts/src/function/query_fts_index.cpp @@ -212,7 +212,7 @@ void QFTSOutputWriter::write(processor::FactorizedTable& scoreFT, nodeID_t docNo scoreInfo.scoreData.size() != numUniqueTerms) { return; } - auto auxInfo = bindData.entry.getAuxInfo().cast(); + auto auxInfo = bindData.auxInfo; for (auto& scoreData : scoreInfo.scoreData) { auto numDocs = bindData.numDocs; auto avgDocLen = bindData.avgDocLen; @@ -492,7 +492,8 @@ static std::unique_ptr bindFunc(main::ClientContext* context, auto& ftsIndex = index.value()->cast(); auto [numDocs, avgDocLen] = ftsIndex.getStats(transaction); auto bindData = std::make_unique(std::move(columns), std::move(graphEntry), - nodeOutput, std::move(query), *ftsIndexEntry, + nodeOutput, std::move(query), + ftsIndexEntry->getAuxInfo().cast(), std::make_unique(input->optionalParamsLegacy), numDocs, avgDocLen); context->setUseInternalCatalogEntry(false /* useInternalCatalogEntry */); return bindData; diff --git a/fts/src/include/function/query_fts_bind_data.h b/fts/src/include/function/query_fts_bind_data.h index e3601bc8..fbc6f2b6 100644 --- a/fts/src/include/function/query_fts_bind_data.h +++ b/fts/src/include/function/query_fts_bind_data.h @@ -1,7 +1,7 @@ #pragma once #include "binder/expression/node_expression.h" -#include "catalog/catalog_entry/index_catalog_entry.h" +#include "catalog/fts_index_catalog_entry.h" #include "function/fts_config.h" #include "function/gds/gds.h" @@ -31,18 +31,22 @@ struct QueryFTSOptionalParams : public function::OptionalParams { struct QueryFTSBindData final : public function::GDSBindData { std::shared_ptr query; - const catalog::IndexCatalogEntry& entry; + // Owned copy of the FTS aux info. This must NOT be a reference/pointer into the + // catalog: bind data outlives the bind transaction (prepared-plan cache reuses it + // across executions on the same connection), so a catalog reference dangles on the + // second execution and segfaults (see issue #1082). + FTSIndexAuxInfo auxInfo; common::table_id_t outputTableID; common::idx_t numDocs; double avgDocLen; QueryFTSBindData(binder::expression_vector columns, graph::NativeGraphEntry graphEntry, std::shared_ptr docs, std::shared_ptr query, - const catalog::IndexCatalogEntry& entry, + const FTSIndexAuxInfo& auxInfo, std::unique_ptr optionalParams, common::idx_t numDocs, double avgDocLen) : GDSBindData{std::move(columns), std::move(graphEntry), binder::expression_vector{docs}}, - query{std::move(query)}, entry{entry}, + query{std::move(query)}, auxInfo{auxInfo}, outputTableID{output[0]->constCast().getTableIDs()[0]}, numDocs{numDocs}, avgDocLen{avgDocLen} { auto& nodeExpr = output[0]->constCast(); @@ -51,7 +55,7 @@ struct QueryFTSBindData final : public function::GDSBindData { this->optionalParams = std::move(optionalParams); } QueryFTSBindData(const QueryFTSBindData& other) - : GDSBindData{other}, query{other.query}, entry{other.entry}, + : GDSBindData{other}, query{other.query}, auxInfo{other.auxInfo}, outputTableID{other.outputTableID}, numDocs{other.numDocs}, avgDocLen{other.avgDocLen} {} std::vector getQueryTerms(main::ClientContext& context) const; From fc0fa8cf5a01e51acccae14d18b3367fafecfc8f Mon Sep 17 00:00:00 2001 From: Arun Sharma Date: Tue, 6 Oct 2026 13:18:17 -0700 Subject: [PATCH 2/2] fix(fts): invalidate cached FTS plan on index drop/recreate The prepared-plan cache reuses QueryFTSBindData across executions on the same connection, so the snapshotted aux info / backing-table graph entry / index stats went stale after DROP_FTS_INDEX (stale results) and broke after CREATE_FTS_INDEX (dropped table IDs). Store the queried table/index names and re-resolve them against the catalog on every execution (refreshFromCatalog). A missing index throws the same BinderException as the literal path, and a recreated index refreshes the snapshot. The cached physical plan also reuses the shared graph, so rebuild it when the backing table IDs changed. --- fts/src/function/query_fts_bind_data.cpp | 45 +++++++++++++++- fts/src/function/query_fts_index.cpp | 54 ++++++++++++++----- .../include/function/query_fts_bind_data.h | 24 ++++++++- 3 files changed, 106 insertions(+), 17 deletions(-) diff --git a/fts/src/function/query_fts_bind_data.cpp b/fts/src/function/query_fts_bind_data.cpp index c40d5771..dcedaea8 100644 --- a/fts/src/function/query_fts_bind_data.cpp +++ b/fts/src/function/query_fts_bind_data.cpp @@ -6,11 +6,14 @@ #include "catalog/fts_index_catalog_entry.h" #include "common/exception/binder.h" #include "common/string_utils.h" +#include "index/fts_index.h" #include "libstemmer.h" +#include "main/client_context.h" #include "re2.h" #include "storage/storage_manager.h" #include "storage/table/node_table.h" #include "utils/fts_utils.h" +#include namespace lbug { namespace fts_extension { @@ -57,11 +60,49 @@ std::vector QueryFTSBindData::getQueryTerms(main::ClientContext& co config.stopWordsTableName) ->getTableID()) ->ptrCast(); - return FTSUtils::stemTerms(terms, auxInfo.config, - MemoryManager::Get(context), stopWordsTable, transaction::Transaction::Get(context), + return FTSUtils::stemTerms(terms, auxInfo.config, MemoryManager::Get(context), stopWordsTable, + transaction::Transaction::Get(context), optionalParams->constCast().conjunctive.getParamVal(), true /* isQuery */); } +void QueryFTSBindData::refreshFromCatalog(main::ClientContext* context) { + std::lock_guard guard{refreshMutex}; + context->setUseInternalCatalogEntry(true /* useInternalCatalogEntry */); + try { + auto catalog = catalog::Catalog::Get(*context); + auto transaction = transaction::Transaction::Get(*context); + auto tableEntry = catalog->getTableCatalogEntry(transaction, tableName); + if (!catalog->containsIndex(transaction, tableEntry->getTableID(), indexName)) { + throw common::BinderException{std::format( + "Table {} doesn't have an index with name {}.", tableEntry->getName(), indexName)}; + } + auto ftsIndexEntry = catalog->getIndex(transaction, tableEntry->getTableID(), indexName); + auto termsEntry = catalog->getTableCatalogEntry(transaction, + FTSUtils::getTermsTableName(tableEntry->getTableID(), indexName)); + auto docsEntry = catalog->getTableCatalogEntry(transaction, + FTSUtils::getDocsTableName(tableEntry->getTableID(), indexName)); + auto appearsInEntry = catalog->getTableCatalogEntry(transaction, + FTSUtils::getAppearsInTableName(tableEntry->getTableID(), indexName)); + graphEntry = graph::NativeGraphEntry({termsEntry, docsEntry}, {appearsInEntry}); + auxInfo.config = ftsIndexEntry->getAuxInfo().cast().config; + auto nodeTable = StorageManager::Get(*context) + ->getTable(ftsIndexEntry->getTableID()) + ->ptrCast(); + auto index = nodeTable->getIndex(indexName); + if (!index.has_value()) { + throw common::BinderException{std::format( + "Table {} doesn't have an index with name {}.", tableEntry->getName(), indexName)}; + } + auto [numDocs_, avgDocLen_] = index.value()->cast().getStats(transaction); + numDocs = numDocs_; + avgDocLen = avgDocLen_; + } catch (...) { + context->setUseInternalCatalogEntry(false /* useInternalCatalogEntry */); + throw; + } + context->setUseInternalCatalogEntry(false /* useInternalCatalogEntry */); +} + } // namespace fts_extension } // namespace lbug diff --git a/fts/src/function/query_fts_index.cpp b/fts/src/function/query_fts_index.cpp index 501cfd43..7296bad0 100644 --- a/fts/src/function/query_fts_index.cpp +++ b/fts/src/function/query_fts_index.cpp @@ -141,8 +141,7 @@ struct QFTSEdgeCompute final : EdgeCompute { : scores{scores}, dfs{dfs}, scoresMutex{std::make_shared()} {} QFTSEdgeCompute(node_id_map_t& scores, - const std::unordered_map& dfs, - std::shared_ptr scoresMutex) + const std::unordered_map& dfs, std::shared_ptr scoresMutex) : scores{scores}, dfs{dfs}, scoresMutex{std::move(scoresMutex)} {} std::vector edgeCompute(nodeID_t boundNodeID, graph::NbrScanState::Chunk& resultChunk, @@ -385,11 +384,39 @@ static void initFrontier(FrontierPair& frontierPair, table_id_t termsTableID, static offset_t tableFunc(const TableFuncInput& input, TableFuncOutput&) { auto& clientContext = *input.context->clientContext; - auto transaction = transaction::Transaction::Get(clientContext); + auto& qFTSBindData = input.bindData->cast(); + // The prepared-plan cache reuses this bind data across executions, so re-resolve the + // index on every execution. This throws when the index was dropped (matching the + // literal path) and picks up the new backing tables after a recreate. + qFTSBindData.refreshFromCatalog(&clientContext); auto sharedState = input.sharedState->ptrCast(); + // The cached physical plan also reuses the shared state (and its graph) across + // executions, so rebuild the graph when the backing tables changed. + { + auto cachedEntry = sharedState->graph->getGraphEntry(); + bool stale = cachedEntry->nodeInfos.size() != qFTSBindData.graphEntry.nodeInfos.size() || + cachedEntry->relInfos.size() != qFTSBindData.graphEntry.relInfos.size(); + for (size_t i = 0; !stale && i < cachedEntry->nodeInfos.size(); ++i) { + if (cachedEntry->nodeInfos[i].entry->getTableID() != + qFTSBindData.graphEntry.nodeInfos[i].entry->getTableID()) { + stale = true; + } + } + for (size_t i = 0; !stale && i < cachedEntry->relInfos.size(); ++i) { + if (cachedEntry->relInfos[i].entry->getTableID() != + qFTSBindData.graphEntry.relInfos[i].entry->getTableID()) { + stale = true; + } + } + if (stale) { + sharedState->graph = std::make_unique(&clientContext, + qFTSBindData.graphEntry.copy()); + sharedState->outputTableID = qFTSBindData.outputTableID; + } + } + auto transaction = transaction::Transaction::Get(clientContext); auto graph = sharedState->graph.get(); auto graphEntry = graph->getGraphEntry(); - auto& qFTSBindData = input.bindData->cast(); auto qFTSOptionalParams = qFTSBindData.optionalParams->constCast(); if (qFTSOptionalParams.topK.isSet()) { sharedState->ptrCast()->setTopK(qFTSOptionalParams.topK.getParamVal()); @@ -492,8 +519,8 @@ static std::unique_ptr bindFunc(main::ClientContext* context, auto& ftsIndex = index.value()->cast(); auto [numDocs, avgDocLen] = ftsIndex.getStats(transaction); auto bindData = std::make_unique(std::move(columns), std::move(graphEntry), - nodeOutput, std::move(query), - ftsIndexEntry->getAuxInfo().cast(), + nodeOutput, std::move(query), ftsIndexEntry->getAuxInfo().cast(), + inputTableName, indexName, std::make_unique(input->optionalParamsLegacy), numDocs, avgDocLen); context->setUseInternalCatalogEntry(false /* useInternalCatalogEntry */); return bindData; @@ -522,16 +549,17 @@ static void getLogicalPlan(Planner* planner, const BoundReadingClause& readingCl } std::shared_ptr initSharedState(const TableFuncInitSharedStateInput& input) { - auto bindData = input.bindData->constPtrCast(); + auto& bindData = input.bindData->cast(); + bindData.refreshFromCatalog(input.context->clientContext); auto graph = std::make_unique(input.context->clientContext, - bindData->graphEntry.copy()); - if (!bindData->optionalParams->constCast().topK.isSet()) { + bindData.graphEntry.copy()); + if (!bindData.optionalParams->constCast().topK.isSet()) { // The user does not give a topK parameter, skip topK optimization. - return std::make_shared(bindData->getResultTable(), std::move(graph), - bindData->outputTableID); + return std::make_shared(bindData.getResultTable(), std::move(graph), + bindData.outputTableID); } else { - return std::make_shared(bindData->getResultTable(), std::move(graph), - bindData->outputTableID); + return std::make_shared(bindData.getResultTable(), std::move(graph), + bindData.outputTableID); } } diff --git a/fts/src/include/function/query_fts_bind_data.h b/fts/src/include/function/query_fts_bind_data.h index fbc6f2b6..06bae69f 100644 --- a/fts/src/include/function/query_fts_bind_data.h +++ b/fts/src/include/function/query_fts_bind_data.h @@ -1,5 +1,8 @@ #pragma once +#include +#include + #include "binder/expression/node_expression.h" #include "catalog/fts_index_catalog_entry.h" #include "function/fts_config.h" @@ -36,17 +39,26 @@ struct QueryFTSBindData final : public function::GDSBindData { // across executions on the same connection), so a catalog reference dangles on the // second execution and segfaults (see issue #1082). FTSIndexAuxInfo auxInfo; + // Identity of the queried index. The prepared-plan cache reuses this bind data + // across executions, so every execution must re-resolve these against the catalog + // (see refreshFromCatalog): after DROP_FTS_INDEX the cached snapshot would + // otherwise keep serving stale results, and after a recreate it would reference + // dropped backing tables. + std::string tableName; + std::string indexName; common::table_id_t outputTableID; common::idx_t numDocs; double avgDocLen; + mutable std::mutex refreshMutex; QueryFTSBindData(binder::expression_vector columns, graph::NativeGraphEntry graphEntry, std::shared_ptr docs, std::shared_ptr query, - const FTSIndexAuxInfo& auxInfo, + const FTSIndexAuxInfo& auxInfo, std::string tableName, std::string indexName, std::unique_ptr optionalParams, common::idx_t numDocs, double avgDocLen) : GDSBindData{std::move(columns), std::move(graphEntry), binder::expression_vector{docs}}, - query{std::move(query)}, auxInfo{auxInfo}, + query{std::move(query)}, auxInfo{auxInfo}, tableName{std::move(tableName)}, + indexName{std::move(indexName)}, outputTableID{output[0]->constCast().getTableIDs()[0]}, numDocs{numDocs}, avgDocLen{avgDocLen} { auto& nodeExpr = output[0]->constCast(); @@ -56,8 +68,16 @@ struct QueryFTSBindData final : public function::GDSBindData { } QueryFTSBindData(const QueryFTSBindData& other) : GDSBindData{other}, query{other.query}, auxInfo{other.auxInfo}, + tableName{other.tableName}, indexName{other.indexName}, outputTableID{other.outputTableID}, numDocs{other.numDocs}, avgDocLen{other.avgDocLen} {} + // Re-resolve the index against the current catalog and refresh the cached snapshot + // (aux info, backing-table graph entry, index stats). Throws BinderException when the + // index no longer exists so the parameterized path matches the literal path. + // Must be called at execution time before the snapshot is used, because the + // prepared-plan cache reuses this bind data across executions on the same connection. + void refreshFromCatalog(main::ClientContext* context); + std::vector getQueryTerms(main::ClientContext& context) const; std::unique_ptr copy() const override {