Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ TEST_JOBS ?= 10
EXTENSION_E2E_TEST_JOBS ?= 1
PGEMBED_PYTHON ?= 3.12
PGEMBED_FIXTURE ?= python3 scripts/run_pgembed_fixture.py --
EXTENSION_LIST ?= adbc;httpfs;duckdb;json;postgres;pg_client;sqlite;fts;delta;iceberg;azure;unity_catalog;vector;neo4j;algo;llm;gql
EXTENSION_LIST ?= adbc;httpfs;duckdb;json;postgres;pg_client;sqlite;fts;delta;iceberg;azure;unity_catalog;ducklake;vector;neo4j;algo;llm;gql
EXTENSION_TEST_EXCLUDE_FILTER ?= ""

ifeq ($(OS),Windows_NT)
Expand Down
78 changes: 66 additions & 12 deletions src/optimizer/foreign_join_push_down_optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,22 @@ static std::optional<ForeignJoinPatternInfo> matchPattern(const LogicalOperator*
return info;
}

static std::string stripIdentifierQuotes(const std::string& name) {
if (name.size() >= 2 && name.front() == '"' && name.back() == '"') {
return name.substr(1, name.size() - 2);
}
return name;
}

// Drop the catalog/schema qualifier: `catalog[.schema]."table"` -> `table`.
// Note: dots inside a quoted identifier are not handled; such names simply
// miss both lookups and surface as empty below.
static std::string unqualifyTableName(const std::string& tableName) {
auto dotPos = tableName.rfind('.');
auto unqualified = dotPos == std::string::npos ? tableName : tableName.substr(dotPos + 1);
return stripIdentifierQuotes(unqualified);
}

// Helper to get column names from a foreign table. tableName may be a bare
// table name or a qualified `catalog[.schema].table` reference; the attached
// database scopes the lookup when qualification is present. Falls back to the
Expand All @@ -358,11 +374,23 @@ static std::vector<std::string> getForeignTableColumnNames(const std::string& db
if (!columnNames.empty() || tableName.find('.') == std::string::npos) {
return columnNames;
}
auto unqualified = tableName.substr(tableName.rfind('.') + 1);
if (unqualified.size() >= 2 && unqualified.front() == '"' && unqualified.back() == '"') {
unqualified = unqualified.substr(1, unqualified.size() - 2);
}
return attachedDB->getTableColumnNames(unqualified);
// Last resort for older extension builds. Note getTableColumnNames()
// reports both "not found" and genuine errors as empty, so a qualified
// miss retries against the attached database's default scope and could
// bind a same-named table in a different schema. Accepted for backward
// compatibility; current extension builds scope the qualified lookup first.
return attachedDB->getTableColumnNames(unqualifyTableName(tableName));
}

// Endpoint-column convention shared with DuckDBCatalog::createForeignRelTable
// (extension): relationship endpoint columns carry src/dst/dest prefixes
// (e.g. src_id, dst, destination). Keep the two in sync. The prefixes are
// disjoint, so a column matches at most one side.
static bool isEndpointColumn(const std::string& lowerColumnName, bool wantSrc) {
const bool isSrc = lowerColumnName.rfind("src", 0) == 0;
const bool isDst =
lowerColumnName.rfind("dst", 0) == 0 || lowerColumnName.rfind("dest", 0) == 0;
return wantSrc ? isSrc : isDst;
}

struct JoinQueryInfo {
Expand Down Expand Up @@ -421,17 +449,18 @@ static JoinQueryInfo buildJoinQuery(const ForeignJoinPatternInfo& info,
for (auto& column : tableColumnNames) {
auto lowerCol = column;
common::StringUtils::toLower(lowerCol);
const bool isSrc = lowerCol == "src_id" || lowerCol.rfind("src", 0) == 0;
const bool isDst = lowerCol == "dst_id" || lowerCol.rfind("dst", 0) == 0 ||
lowerCol.rfind("dest", 0) == 0;
if ((wantSrc && isSrc && !isDst) || (!wantSrc && isDst && !isSrc)) {
if (isEndpointColumn(lowerCol, wantSrc)) {
return column;
}
}
return "";
};
auto srcCol = findEndpointColumn(true /* wantSrc */);
auto dstCol = findEndpointColumn(false /* wantSrc */);
// All-or-nothing: a lone prefix match without its counterpart is ambiguous
// (it may be a non-endpoint property that happens to share the prefix), so
// fall back to ordinal position for both columns to preserve the prior
// "first two columns" invariant.
if (!srcCol.empty() && !dstCol.empty()) {
firstCol = srcCol;
secondCol = dstCol;
Expand All @@ -444,15 +473,40 @@ static JoinQueryInfo buildJoinQuery(const ForeignJoinPatternInfo& info,
dstJoinCol = firstCol;
}

auto getNodeIDColumn = [&](const std::string& tableName) {
// Resolve the node-table ID column without assuming column order. The
// bound catalog entry's primary key is the single source of truth (the
// extension registers foreign column names as property names, so it names
// the foreign column directly); only fall back to a foreign lookup when
// the entry carries no primary key (e.g. FOREIGN_TABLE_ENTRY).
auto getNodeIDColumn = [&](const NodeExpression* node, const std::string& tableName) {
if (node && node->getNumEntries() == 1) {
if (auto entry = node->getEntry(0);
entry && entry->getType() == CatalogEntryType::NODE_TABLE_ENTRY) {
if (auto nodeEntry = entry->ptrCast<NodeTableCatalogEntry>(); nodeEntry) {
auto pkName = nodeEntry->getPrimaryKeyName();
if (!pkName.empty()) {
return pkName;
}
}
}
}
auto columnNames = getForeignTableColumnNames(info.dbName, tableName, context);
if (columnNames.empty()) {
return std::string{InternalKeyword::ID};
}
// SQL catalogs do not guarantee the PK column comes first; prefer a
// column literally named `id` over ordinal position.
for (auto& column : columnNames) {
auto lowerCol = column;
common::StringUtils::toLower(lowerCol);
if (lowerCol == "id") {
return column;
}
}
return columnNames[0];
};
auto srcIDCol = getNodeIDColumn(info.srcTable);
auto dstIDCol = getNodeIDColumn(info.dstTable);
auto srcIDCol = getNodeIDColumn(srcNode.get(), info.srcTable);
auto dstIDCol = getNodeIDColumn(dstNode.get(), info.dstTable);

// Build SELECT items from output columns and collect column names
std::vector<std::string> columnNames;
Expand Down
4 changes: 2 additions & 2 deletions src/processor/operator/simple/attach_database.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ void AttachDatabase::executeInternal(ExecutionContext* context) {
auto errMsg =
std::format("No loaded extension can handle database type: {}.", attachInfo.dbType);
auto dbType = common::StringUtils::getLower(attachInfo.dbType);
if (dbType == "adbc" || dbType == "duckdb" || dbType == "postgres" || dbType == "sqlite" ||
dbType == "iceberg" || dbType == "uc_catalog") {
if (dbType == "adbc" || dbType == "duckdb" || dbType == "ducklake" || dbType == "postgres" ||
dbType == "sqlite" || dbType == "iceberg" || dbType == "uc_catalog") {
errMsg += std::format("\nDid you forget to load {} extension?\nYou can load it by: load "
"extension {};",
dbType, dbType);
Expand Down
Loading