From eee248933923d1187c33e0f737e1729143eb35d0 Mon Sep 17 00:00:00 2001 From: Neeraj Dwivedi Date: Wed, 22 Jul 2026 21:44:15 +0530 Subject: [PATCH 1/3] feat(bq_driver): Optimize SQLForeignKeys api impl --- .../internal/odbc_sql_foreign_keys.cc | 357 ++++++++++++------ .../internal/odbc_sql_foreign_keys.h | 10 +- .../internal/odbc_sql_foreign_keys_test.cc | 18 +- .../odbc/bq_driver/odbc_driver_metadata.cc | 27 +- .../examples/catalog_performance_example.cc | 2 +- .../cloud/odbc/testing/odbc_utils/catalog.cc | 7 + 6 files changed, 269 insertions(+), 152 deletions(-) diff --git a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc index c557c9716f..fff0fccd13 100644 --- a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc +++ b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc @@ -13,6 +13,7 @@ // limitations under the License. #include "google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h" +#include "google/cloud/odbc/bq_driver/internal/odbc_sql_columns.h" #include "google/cloud/odbc/bq_driver/internal/trace_utils.h" #include @@ -22,67 +23,173 @@ using ::google::cloud::odbc_internal::SQLStates; using ::google::cloud::odbc_internal::StatusRecord; using ::google::cloud::odbc_internal::StatusRecordOr; -namespace { -std::string const kNamedCatalogParam = "catalog_name"; -std::string const kNamedSchemaParam = "schema_name"; -std::string const kNamedPKTableParam = "pk_table_name"; -std::string const kNamedFKTableParam = "fk_table_name"; - -std::string const kBasicForeignKeysQueryPrefix = - "WITH pk_constraint AS ( " - "SELECT key_column_usage.constraint_catalog as pk_catalog," - "key_column_usage.constraint_schema as pk_dataset, " - "key_column_usage.table_name as pk_table, " - "key_column_usage.column_name as pk_column, " - "key_column_usage.constraint_name as pk_name, " - "key_column_usage.ordinal_position as pk_column_ordinal_position " - "FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE key_column_usage " - "INNER JOIN INFORMATION_SCHEMA.TABLE_CONSTRAINTS table_constraints " - "ON table_constraints.table_name = key_column_usage.table_name " - "AND table_constraints.constraint_name = key_column_usage.constraint_name " - "AND table_constraints.constraint_schema = " - "key_column_usage.constraint_schema " - "WHERE table_constraints.CONSTRAINT_TYPE = 'PRIMARY KEY' " - "), " - "pk_references AS ( " - "SELECT pk_constraint.*, " - "constraints_column_usage.constraint_schema as fk_constraint_schema, " - "constraints_column_usage.constraint_name as fk_constraint_name " - "FROM pk_constraint " - "JOIN INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE constraints_column_usage " - "ON true " - "AND pk_constraint.pk_table = constraints_column_usage.table_name " - "AND pk_constraint.pk_column = constraints_column_usage.column_name " - "AND pk_constraint.pk_dataset = constraints_column_usage.TABLE_SCHEMA " - ") " - "SELECT pk_references.pk_catalog, " - "pk_references.pk_dataset, " - "pk_references.pk_table, " - "pk_references.pk_column, " - "key_column_usage.table_catalog as fk_catalog, " - "key_column_usage.table_schema as fk_dataset, " - "key_column_usage.table_name as fk_table, " - "key_column_usage.column_name as fk_column, " - "key_column_usage.ordinal_position as fk_column_ordinal_position, " - "CAST(NULL AS INT64) AS update_rule, " - "CAST(NULL AS INT64) AS delete_rule, " - "key_column_usage.constraint_name as fk_name, " - "pk_references.pk_name, " - "CAST(2 AS INT64) AS deferrability " - "FROM INFORMATION_SCHEMA.KEY_COLUMN_USAGE key_column_usage " - "JOIN pk_references " - "ON pk_references.fk_constraint_name = key_column_usage.constraint_name " - "AND pk_references.fk_constraint_schema = " - "key_column_usage.constraint_schema " - "AND pk_references.pk_column_ordinal_position = " - "key_column_usage.POSITION_IN_UNIQUE_CONSTRAINT "; - -std::string const kBasicForeignKeysQuerySuffix = - "ORDER BY pk_table, pk_column_ordinal_position, pk_column"; - -} // namespace - -odbc_internal::StatusRecordOr FetchForeignKeysFromDataSource( +StatusRecordOr CreateResultSetForForeignKeys( + std::string const& pk_catalog_name, std::string const& pk_schema_name, + std::string const& pk_table_name, std::string const& pk_column_name, + std::string const& fk_catalog_name, std::string const& fk_schema_name, + std::string const& fk_table_name, std::string const& fk_column_name, + SQLSMALLINT field_pos) { + DSRow ds_row; + // PK_TABLE_CAT + DSValue ds_pk_table_cat = kNullValue; + if (!pk_catalog_name.empty()) { + StringToDSValue(pk_catalog_name, ds_pk_table_cat); + } + ds_row.emplace_back(ds_pk_table_cat); + + // PKTABLE_SCHEM + DSValue ds_pk_table_schema = kNullValue; + if (!pk_schema_name.empty()) { + StringToDSValue(pk_schema_name, ds_pk_table_schema); + } + ds_row.emplace_back(ds_pk_table_schema); + + // PK_TABLE_NAME + DSValue ds_pk_table_name = kNullValue; + if (!pk_table_name.empty()) { + StringToDSValue(pk_table_name, ds_pk_table_name); + } + ds_row.emplace_back(ds_pk_table_name); + + // PK_COLUMN_NAME + DSValue ds_pk_column_name = kNullValue; + if (!pk_column_name.empty()) { + StringToDSValue(pk_column_name, ds_pk_column_name); + } + ds_row.emplace_back(ds_pk_column_name); + + // FK_TABLE_CAT + DSValue ds_fk_table_cat = kNullValue; + if (!fk_catalog_name.empty()) { + StringToDSValue(fk_catalog_name, ds_fk_table_cat); + } + ds_row.emplace_back(ds_fk_table_cat); + + // FKTABLE_SCHEM + DSValue ds_fk_table_schema = kNullValue; + if (!fk_schema_name.empty()) { + StringToDSValue(fk_schema_name, ds_fk_table_schema); + } + ds_row.emplace_back(ds_fk_table_schema); + + // FK_TABLE_NAME + DSValue ds_fk_table_name = kNullValue; + if (!fk_table_name.empty()) { + StringToDSValue(fk_table_name, ds_fk_table_name); + } + ds_row.emplace_back(ds_fk_table_name); + + // FK_COLUMN_NAME + DSValue ds_fk_column_name = kNullValue; + if (!fk_column_name.empty()) { + StringToDSValue(fk_column_name, ds_fk_column_name); + } + ds_row.emplace_back(ds_fk_column_name); + + // FK_COLUMN_ORDINAL_POSITION + DSValue ds_fk_column_ordinal_pos = kNullValue; + // field_pos is always >= 0 any other value is error. + if (field_pos < 0) { + LOG(ERROR) << "CreateResultSetDSRow:: Invalid ordinal position: " + << field_pos; + return StatusRecord{SQLStates::k_HY000(), "Invalid ordinal position"}; + } + ArithmeticToDSValue(static_cast(field_pos), + ds_fk_column_ordinal_pos); + ds_row.emplace_back(ds_fk_column_ordinal_pos); + + // UPDATE_RULE + DSValue ds_update_rule = kNullValue; + ds_row.emplace_back(ds_update_rule); + + // DELETE_RULE + DSValue ds_delete_rule = kNullValue; + ds_row.emplace_back(ds_delete_rule); + + // FK_NAME + DSValue ds_fk_name = kNullValue; + if (!fk_table_name.empty()) { + std::string fk_name = fk_table_name + ".fk$" + std::to_string(field_pos); + StringToDSValue(fk_name, ds_fk_name); + } + ds_row.emplace_back(ds_fk_name); + + // PK_NAME + DSValue ds_pk_name = kNullValue; + if (!pk_table_name.empty()) { + std::string pk_name = pk_table_name + ".pk$"; + StringToDSValue(pk_name, ds_pk_name); + } + ds_row.emplace_back(ds_pk_name); + + // DEFERRABILITY + DSValue ds_deferrability = kNullValue; + IntToDSValue(SQL_SET_NULL, ds_deferrability); + ds_row.emplace_back(ds_deferrability); + + return ds_row; +} + +StatusRecordOr CreateFKResultRows( + ConnectionHandle& conn_handle, std::string const& catalog_name, + std::string const& schema_name, std::string const& table_name, + std::string const& pk_catalog_name, std::string const& pk_schema_name, + std::string const& pk_table_name, std::vector pk_key_columns, + bool const& has_pk_table_only) { + ResultSetRows result_rows; + if (table_name == pk_table_name && catalog_name == pk_catalog_name && + schema_name == pk_schema_name) { + return result_rows; + } + + auto table_status = + FetchBQTableData(conn_handle, catalog_name, schema_name, table_name); + if (!table_status) { + return table_status.GetStatusRecord(); + } + auto const& metadata = *table_status; + auto const& foreign_keys = metadata.table_constraints.foreign_keys; + + for (auto const& fk_key : foreign_keys) { + bool matches_pk_table = false; + if (has_pk_table_only) { + matches_pk_table = (fk_key.referenced_table.table_id == pk_table_name); + } else { + auto const& referenced_table = fk_key.referenced_table; + + matches_pk_table = (referenced_table.table_id == pk_table_name); + } + if (!matches_pk_table) { + continue; + } + + int ord_pos = 1; + for (auto const& column_reference : fk_key.column_references) { + auto pk_column_it = + std::find(pk_key_columns.begin(), pk_key_columns.end(), + column_reference.referenced_column); + + if (pk_column_it == pk_key_columns.end()) { + ++ord_pos; + continue; + } + + auto row_status = CreateResultSetForForeignKeys( + pk_catalog_name, pk_schema_name, pk_table_name, + column_reference.referenced_column, catalog_name, schema_name, + table_name, column_reference.referencing_column, ord_pos); + + if (!row_status) { + return row_status.GetStatusRecord(); + } + result_rows.emplace_back(std::move(*row_status)); + ++ord_pos; + } + } + return result_rows; +} + +StatusRecordOr FetchFKResultSetFromTableMetaData( StatementHandle& stmt_handle, std::string const& pk_catalog_name, int pk_catalog_name_len, std::string const& pk_schema_name, int pk_schema_name_len, std::string const& pk_table_name, @@ -95,7 +202,7 @@ odbc_internal::StatusRecordOr FetchForeignKeysFromDataSource( (!pk_catalog_name.empty()) ? pk_catalog_name : fk_catalog_name; if (catalog_name.empty() || (pk_catalog_name_len == 0 && fk_catalog_name_len == 0)) { - LOG(ERROR) << "FetchForeignKeysFromDataSource:: Catalog name for both " + LOG(ERROR) << "FetchFKResultSetFromTableMetaData:: Catalog name for both " "primary and foreign keys cannot be empty."; auto status_record = StatusRecord{SQLStates::k_HY090(), @@ -106,7 +213,7 @@ odbc_internal::StatusRecordOr FetchForeignKeysFromDataSource( } if (!pk_catalog_name.empty() && !fk_catalog_name.empty() && pk_catalog_name != fk_catalog_name) { - LOG(ERROR) << "FetchForeignKeysFromDataSource:: PK and FK catalog names " + LOG(ERROR) << "FetchFKResultSetFromTableMetaData:: PK and FK catalog names " "need to be the same."; auto status_record = StatusRecord{SQLStates::k_HYC00(), @@ -119,7 +226,7 @@ odbc_internal::StatusRecordOr FetchForeignKeysFromDataSource( (!pk_schema_name.empty()) ? pk_schema_name : fk_schema_name; if (schema_name.empty() || (pk_schema_name_len == 0 && fk_schema_name_len == 0)) { - LOG(ERROR) << "FetchForeignKeysFromDataSource:: Schema name for both " + LOG(ERROR) << "FetchFKResultSetFromTableMetaData:: Schema name for both " "primary and foreign keys cannot be empty."; auto status_record = StatusRecord{SQLStates::k_HY090(), @@ -130,7 +237,7 @@ odbc_internal::StatusRecordOr FetchForeignKeysFromDataSource( } if (!pk_schema_name.empty() && !fk_schema_name.empty() && pk_schema_name != fk_schema_name) { - LOG(ERROR) << "FetchForeignKeysFromDataSource:: PK and FK schema names " + LOG(ERROR) << "FetchFKResultSetFromTableMetaData:: PK and FK schema names " "need to be the same."; auto status_record = StatusRecord{SQLStates::k_HYC00(), @@ -141,8 +248,9 @@ odbc_internal::StatusRecordOr FetchForeignKeysFromDataSource( } if ((pk_table_name.empty() && fk_table_name.empty()) || (pk_table_name_len == 0 && fk_table_name_len == 0)) { - LOG(ERROR) << "FetchForeignKeysFromDataSource:: Both Primary and Foreign " - "key table names cannot be empty."; + LOG(ERROR) + << "FetchFKResultSetFromTableMetaData:: Both Primary and Foreign " + "key table names cannot be empty."; auto status_record = StatusRecord{ SQLStates::k_HY009(), "Both Primary and Foreign key table names cannot be empty"}; @@ -150,71 +258,78 @@ odbc_internal::StatusRecordOr FetchForeignKeysFromDataSource( return status_record; } if (stmt_handle.GetConnectionHandle() == nullptr) { - LOG(ERROR) << "FetchForeignKeysFromDataSource:: Connection handle is null."; + LOG(ERROR) + << "FetchFKResultSetFromTableMetaData:: Connection handle is null."; auto status_record = StatusRecord{SQLStates::k_HY013(), "Internal connection handle is null"}; stmt_handle.GetDiagnostics().AddStatusRecord(status_record); return status_record; } - // Construct named query for foreign keys. - std::string foreign_keys_query(kBasicForeignKeysQueryPrefix); - foreign_keys_query - .append(" AND pk_catalog = @") // PrimaryKey catalog - .append(kNamedCatalogParam) - .append(" AND pk_dataset = @") // PrimaryKey dataset - .append(kNamedSchemaParam) - .append(" AND key_column_usage.table_catalog = @") // ForeignKey catalog - .append(kNamedCatalogParam) - .append(" AND key_column_usage.table_schema = @") // ForeignKey dataset - .append(kNamedSchemaParam); - if (!pk_table_name.empty()) { - foreign_keys_query.append(" AND pk_references.pk_table LIKE @"); - foreign_keys_query.append(kNamedPKTableParam); + + ConnectionHandle& conn_handle = *(stmt_handle.GetConnectionHandle()); + ResultSet result_set; + result_set.row_schema.resize(kForeignKeysMap.size()); + for (auto const& [_, schema] : kForeignKeysMap) { + result_set.row_schema[schema.col_index] = schema; } - if (!fk_table_name.empty()) { - foreign_keys_query.append(" AND key_column_usage.table_name LIKE @"); - foreign_keys_query.append(kNamedFKTableParam); - } - foreign_keys_query.append(" ").append(kBasicForeignKeysQuerySuffix); - // Construct named query params - std::map named_query_params; - named_query_params.insert({kNamedCatalogParam, catalog_name}); - named_query_params.insert({kNamedSchemaParam, schema_name}); - if (!pk_table_name.empty()) { - named_query_params.insert({kNamedPKTableParam, pk_table_name}); + + std::string lookup_table = + !pk_table_name.empty() ? pk_table_name : fk_table_name; + auto table_metadata_status = + FetchBQTableData(conn_handle, catalog_name, schema_name, lookup_table); + if (!table_metadata_status) { + return table_metadata_status.GetStatusRecord(); } - if (!fk_table_name.empty()) { - named_query_params.insert({kNamedFKTableParam, fk_table_name}); + + auto const& pk_columns = + table_metadata_status->table_constraints.primary_key.columns; + // case : When both pk & fk table provided + if (!pk_table_name.empty() && !fk_table_name.empty()) { + auto row_status = CreateFKResultRows( + conn_handle, catalog_name, schema_name, fk_table_name, pk_catalog_name, + pk_schema_name, pk_table_name, pk_columns, false); + if (!row_status) { + return row_status.GetStatusRecord(); + } + + result_set.rows.insert(result_set.rows.end(), + std::make_move_iterator(row_status->begin()), + std::make_move_iterator(row_status->end())); + return result_set; } - auto query_param_status = ConstructStringQueryParameters(named_query_params); - if (!query_param_status) { - LOG(ERROR) - << "FetchForeignKeysFromDataSource::ConstructStringQueryParameters:: " - << query_param_status.GetStatusRecord().message; - auto status_record = query_param_status.GetStatusRecord(); - stmt_handle.GetDiagnostics().AddStatusRecord(status_record); - return status_record; + + // case: either pk or fk table provided + Options opts; + auto table_status = + conn_handle.GetClient()->ListAllTables(catalog_name, schema_name, opts); + if (!table_status) { + return table_status.GetStatusRecord(); } - // Construct post query request. - auto post_query_request_status = ConstructNamedParametersPostQueryRequest( - catalog_name, schema_name, foreign_keys_query, *query_param_status); - if (!post_query_request_status) { - LOG(ERROR) << "FetchForeignKeysFromDataSource::" - "ConstructNamedParametersPostQueryRequest:: " - << post_query_request_status.GetStatusRecord().message; - auto status_record = post_query_request_status.GetStatusRecord(); - stmt_handle.GetDiagnostics().AddStatusRecord(status_record); - return status_record; + + bool has_pk_table_only = !pk_table_name.empty(); + std::vector>> futures; + for (auto const& table : *table_status) { + futures.emplace_back(std::async( + std::launch::async, + [&conn_handle, catalog_name, schema_name, table, pk_catalog_name, + pk_schema_name, pk_table_name, pk_columns, has_pk_table_only]() { + return CreateFKResultRows( + conn_handle, catalog_name, schema_name, + table.table_reference.table_id, pk_catalog_name, pk_schema_name, + pk_table_name, pk_columns, has_pk_table_only); + })); } - // Fetch BQ Data using the post query request above. - auto status_record_or = FetchBQData(stmt_handle, *post_query_request_status); - if (!status_record_or) { - LOG(ERROR) << "FetchForeignKeysFromDataSource::FetchBQData:: " - << status_record_or.GetStatusRecord().message; - stmt_handle.GetDiagnostics().AddStatusRecord( - status_record_or.GetStatusRecord()); + + for (auto& future : futures) { + auto row_status = future.get(); + if (!row_status) { + continue; + } + result_set.rows.insert(result_set.rows.end(), + std::make_move_iterator(row_status->begin()), + std::make_move_iterator(row_status->end())); } - return status_record_or; + return result_set; } } // namespace google::cloud::odbc_bq_driver_internal diff --git a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h index b5b35aa68d..e42aa73565 100644 --- a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h +++ b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h @@ -56,7 +56,14 @@ static std::map const kForeignKeysMap = { {"DEFERRABILITY", ColumnSchema{13, BQDataType::kInt64}}, }; -odbc_internal::StatusRecordOr FetchForeignKeysFromDataSource( +StatusRecordOr CreateFKResultRows( + ConnectionHandle& conn_handle, std::string const& catalog_name, + std::string const& schema_name, std::string const& table_name, + std::string const& pk_catalog_name, std::string const& pk_schema_name, + std::string const& pk_table_name, std::vector pk_key_columns, + bool const& has_pk_table_only = false); + +odbc_internal::StatusRecordOr FetchFKResultSetFromTableMetaData( StatementHandle& stmt_handle, std::string const& pk_catalog_name, int pk_catalog_name_len, std::string const& pk_schema_name, int pk_schema_name_len, std::string const& pk_table_name, @@ -64,7 +71,6 @@ odbc_internal::StatusRecordOr FetchForeignKeysFromDataSource( int fk_catalog_name_len, std::string const& fk_schema_name, int fk_schema_name_len, std::string const& fk_table_name, int fk_table_name_len); - } // namespace google::cloud::odbc_bq_driver_internal #endif // CPP_BIGQUERY_ODBC_GOOGLE_CLOUD_ODBC_BQ_DRIVER_INTERNAL_ODBC_SQL_FOREIGN_KEYS_H diff --git a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys_test.cc b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys_test.cc index 69753375fe..2aba7116d6 100644 --- a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys_test.cc +++ b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys_test.cc @@ -34,7 +34,7 @@ int const kFKTableLen = kFKTable.length(); TEST(FetchForeignKeys, failureEmptyCatalogName) { StatementHandle handle; - auto status_record_or = FetchForeignKeysFromDataSource( + auto status_record_or = FetchFKResultSetFromTableMetaData( handle, "", kCatalogLen, kDataset, kDatasetLen, kPKTable, kPKTableLen, "", kCatalogLen, kDataset, kDatasetLen, kFKTable, kFKTableLen); @@ -46,7 +46,7 @@ TEST(FetchForeignKeys, failureEmptyCatalogName) { } TEST(FetchForeignKeys, failureEmptyCatalogNameLen) { StatementHandle handle; - auto status_record_or = FetchForeignKeysFromDataSource( + auto status_record_or = FetchFKResultSetFromTableMetaData( handle, kCatalog, 0, kDataset, kDatasetLen, kPKTable, kPKTableLen, kCatalog, 0, kDataset, kDatasetLen, kFKTable, kFKTableLen); @@ -59,7 +59,7 @@ TEST(FetchForeignKeys, failureEmptyCatalogNameLen) { TEST(FetchForeignKeys, failureEmptySchemaName) { StatementHandle handle; - auto status_record_or = FetchForeignKeysFromDataSource( + auto status_record_or = FetchFKResultSetFromTableMetaData( handle, kCatalog, kCatalogLen, "", kDatasetLen, kPKTable, kPKTableLen, kCatalog, kCatalogLen, "", kDatasetLen, kFKTable, kFKTableLen); @@ -71,7 +71,7 @@ TEST(FetchForeignKeys, failureEmptySchemaName) { } TEST(FetchForeignKeys, failureEmptySchemaNameLen) { StatementHandle handle; - auto status_record_or = FetchForeignKeysFromDataSource( + auto status_record_or = FetchFKResultSetFromTableMetaData( handle, kCatalog, kCatalogLen, kDataset, 0, kPKTable, kPKTableLen, kCatalog, kCatalogLen, kDataset, 0, kFKTable, kFKTableLen); @@ -84,7 +84,7 @@ TEST(FetchForeignKeys, failureEmptySchemaNameLen) { TEST(FetchForeignKeys, failureDifferentPrimaryForeignCatalog) { StatementHandle handle; - auto status_record_or = FetchForeignKeysFromDataSource( + auto status_record_or = FetchFKResultSetFromTableMetaData( handle, kCatalog, kCatalogLen, kDataset, kDatasetLen, kPKTable, kPKTableLen, "fk-catalog", 10 /* fk catalog len*/, kDataset, kDatasetLen, kFKTable, kFKTableLen); @@ -99,7 +99,7 @@ TEST(FetchForeignKeys, failureDifferentPrimaryForeignCatalog) { TEST(FetchForeignKeys, failureDifferentPrimaryForeignSchema) { StatementHandle handle; - auto status_record_or = FetchForeignKeysFromDataSource( + auto status_record_or = FetchFKResultSetFromTableMetaData( handle, kCatalog, kCatalogLen, kDataset, kDatasetLen, kPKTable, kPKTableLen, kCatalog, kCatalogLen, "fk-dataset", 10 /* fk dataset len*/, kFKTable, kFKTableLen); @@ -114,7 +114,7 @@ TEST(FetchForeignKeys, failureDifferentPrimaryForeignSchema) { TEST(FetchForeignKeys, failureEmptyTableName) { StatementHandle handle; - auto status_record_or = FetchForeignKeysFromDataSource( + auto status_record_or = FetchFKResultSetFromTableMetaData( handle, kCatalog, kCatalogLen, kDataset, kDatasetLen, "", 0, kCatalog, kCatalogLen, kDataset, kDatasetLen, "", 0); @@ -127,7 +127,7 @@ TEST(FetchForeignKeys, failureEmptyTableName) { } TEST(FetchForeignKeys, failureEmptyTableNameLen) { StatementHandle handle; - auto status_record_or = FetchForeignKeysFromDataSource( + auto status_record_or = FetchFKResultSetFromTableMetaData( handle, kCatalog, kCatalogLen, kDataset, kDatasetLen, kPKTable, 0, kCatalog, kCatalogLen, kDataset, kDatasetLen, kFKTable, 0); @@ -141,7 +141,7 @@ TEST(FetchForeignKeys, failureEmptyTableNameLen) { TEST(FetchForeignKeys, FailureNullConnectionhandle) { StatementHandle handle; - auto status_record_or = FetchForeignKeysFromDataSource( + auto status_record_or = FetchFKResultSetFromTableMetaData( handle, kCatalog, kCatalogLen, kDataset, kDatasetLen, kPKTable, kPKTableLen, kCatalog, kCatalogLen, kDataset, kDatasetLen, kFKTable, kFKTableLen); diff --git a/google/cloud/odbc/bq_driver/odbc_driver_metadata.cc b/google/cloud/odbc/bq_driver/odbc_driver_metadata.cc index 5d17ce2085..ece939cddd 100644 --- a/google/cloud/odbc/bq_driver/odbc_driver_metadata.cc +++ b/google/cloud/odbc/bq_driver/odbc_driver_metadata.cc @@ -35,7 +35,7 @@ using google::cloud::odbc_bq_driver_internal::DescriptorType; using google::cloud::odbc_bq_driver_internal::DSResults; using google::cloud::odbc_bq_driver_internal::FetchBQSQLProceduresData; using google::cloud::odbc_bq_driver_internal::FetchBQTablesData; -using google::cloud::odbc_bq_driver_internal::FetchForeignKeysFromDataSource; +using google::cloud::odbc_bq_driver_internal::FetchFKResultSetFromTableMetaData; using google::cloud::odbc_bq_driver_internal::GetResultSetForDatasets; using google::cloud::odbc_bq_driver_internal::GetResultSetForProjects; using google::cloud::odbc_bq_driver_internal::GetResultSetForTables; @@ -350,25 +350,14 @@ SQLRETURN SQLForeignKeysInternal( } StatementHandle& handle = *(*handle_result); - // First fetch the foreign keys from data source. - StatusRecordOr ds_status_record_or = - FetchForeignKeysFromDataSource( - handle, ToCharStr(pk_catalog_name), pk_catalog_name_len, - ToCharStr(pk_schema_name), pk_schema_name_len, - ToCharStr(pk_table_name), pk_table_name_len, - ToCharStr(fk_catalog_name), fk_catalog_name_len, - ToCharStr(fk_schema_name), fk_schema_name_len, - ToCharStr(fk_table_name), fk_table_name_len); - if (!ds_status_record_or) { - LOG(ERROR) << "SQLForeignKeys::FetchForeignKeysFromDataSource:: " - << ds_status_record_or.GetStatusRecord().message; - return LogAndReturnCode(handle, ds_status_record_or); - } - // Process the DSResults and convert to ResultSet. - StatusRecordOr rs_status_record_or = - ProcessQueryResults(*ds_status_record_or); + auto rs_status_record_or = FetchFKResultSetFromTableMetaData( + handle, ToCharStr(pk_catalog_name), pk_catalog_name_len, + ToCharStr(pk_schema_name), pk_schema_name_len, ToCharStr(pk_table_name), + pk_table_name_len, ToCharStr(fk_catalog_name), fk_catalog_name_len, + ToCharStr(fk_schema_name), fk_schema_name_len, ToCharStr(fk_table_name), + fk_table_name_len); if (!rs_status_record_or) { - LOG(ERROR) << "SQLForeignKeys::ProcessQueryResults:: " + LOG(ERROR) << "SQLForeignKeys::FetchFKResultSetFromTableMetaData:: " << rs_status_record_or.GetStatusRecord().message; return LogAndReturnCode(handle, rs_status_record_or); } diff --git a/google/cloud/odbc/integration_tests/odbc_driver_tests/examples/catalog_performance_example.cc b/google/cloud/odbc/integration_tests/odbc_driver_tests/examples/catalog_performance_example.cc index 83296b0bc4..941d7d3c75 100644 --- a/google/cloud/odbc/integration_tests/odbc_driver_tests/examples/catalog_performance_example.cc +++ b/google/cloud/odbc/integration_tests/odbc_driver_tests/examples/catalog_performance_example.cc @@ -18,11 +18,11 @@ #include "google/cloud/odbc/testing/odbc_utils/connection.h" #include "google/cloud/odbc/testing/odbc_utils/statement.h" #include +#include #include #include #include #include - namespace google::cloud::odbc_tests { class CatalogPerformanceHtapiParamTest : public ::testing::TestWithParam { diff --git a/google/cloud/odbc/testing/odbc_utils/catalog.cc b/google/cloud/odbc/testing/odbc_utils/catalog.cc index 2a37e4b52b..8b69da3223 100644 --- a/google/cloud/odbc/testing/odbc_utils/catalog.cc +++ b/google/cloud/odbc/testing/odbc_utils/catalog.cc @@ -448,6 +448,7 @@ RowWiseResults Catalog::GetForeignKeys(std::shared_ptr const& conn, columns[i].buffer_length, &(columns[i].str_len)); CheckError(status, "SQLBindCol", conn); } + auto start = std::chrono::high_resolution_clock::now(); if (!pk_table.empty() && !fk_table.empty()) { if (use_ansi) { @@ -548,6 +549,12 @@ RowWiseResults Catalog::GetForeignKeys(std::shared_ptr const& conn, static_cast(fk_table.length())); } } + auto end = std::chrono::high_resolution_clock::now(); + auto duration_us = + std::chrono::duration_cast(end - start); + + std::cout << "SQLForeignKeys took " << duration_us.count() / 1000 << " ms" + << std::endl; CheckError(status, "SQLForeignKeys", conn, use_ansi); while (true) { From 27d739e882ff178a33a197885a0c45b7b77a410b Mon Sep 17 00:00:00 2001 From: Neeraj Dwivedi Date: Thu, 23 Jul 2026 16:35:03 +0530 Subject: [PATCH 2/3] new changes --- .../internal/odbc_sql_foreign_keys.cc | 121 ++++++++++++------ .../internal/odbc_sql_foreign_keys.h | 4 +- .../odbc/bq_driver/odbc_driver_metadata.cc | 2 - 3 files changed, 85 insertions(+), 42 deletions(-) diff --git a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc index fff0fccd13..94862e8ea9 100644 --- a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc +++ b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc @@ -134,11 +134,13 @@ StatusRecordOr CreateFKResultRows( ConnectionHandle& conn_handle, std::string const& catalog_name, std::string const& schema_name, std::string const& table_name, std::string const& pk_catalog_name, std::string const& pk_schema_name, - std::string const& pk_table_name, std::vector pk_key_columns, + std::string const& pk_table_name, std::string const& lookup_table, + std::vector key_columns, std::vector fk_col_obj, bool const& has_pk_table_only) { + StatusRecord status; ResultSetRows result_rows; - if (table_name == pk_table_name && catalog_name == pk_catalog_name && - schema_name == pk_schema_name) { + if ((table_name == pk_table_name || table_name == lookup_table) && + catalog_name == pk_catalog_name && schema_name == pk_schema_name) { return result_rows; } @@ -148,42 +150,69 @@ StatusRecordOr CreateFKResultRows( return table_status.GetStatusRecord(); } auto const& metadata = *table_status; - auto const& foreign_keys = metadata.table_constraints.foreign_keys; - for (auto const& fk_key : foreign_keys) { - bool matches_pk_table = false; - if (has_pk_table_only) { - matches_pk_table = (fk_key.referenced_table.table_id == pk_table_name); - } else { - auto const& referenced_table = fk_key.referenced_table; + auto is_in_key_columns = [&](std::string const& col_name) { + return std::find(key_columns.begin(), key_columns.end(), col_name) != + key_columns.end(); + }; + auto add_row = [&](std::string const& p_cat, std::string const& p_sch, + std::string const& p_tbl, std::string const& p_col, + std::string const& f_cat, std::string const& f_sch, + std::string const& f_tbl, std::string const& f_col, + int& ord_pos) -> bool { + auto row_status = CreateResultSetForForeignKeys( + p_cat, p_sch, p_tbl, p_col, f_cat, f_sch, f_tbl, f_col, ord_pos); - matches_pk_table = (referenced_table.table_id == pk_table_name); - } - if (!matches_pk_table) { - continue; + if (!row_status) { + status = row_status.GetStatusRecord(); + return false; } + result_rows.emplace_back(std::move(*row_status)); + ++ord_pos; + return true; + }; + + // case: FK table only + if (!has_pk_table_only) { + auto const& pk_keys = metadata.table_constraints.primary_key.columns; int ord_pos = 1; - for (auto const& column_reference : fk_key.column_references) { - auto pk_column_it = - std::find(pk_key_columns.begin(), pk_key_columns.end(), - column_reference.referenced_column); - if (pk_column_it == pk_key_columns.end()) { - ++ord_pos; - continue; + for (auto const& fk_col : fk_col_obj) { + if (fk_col.referenced_table.table_id == table_name) { + for (auto const& pk_key : pk_keys) { + if (!is_in_key_columns(pk_key)) { + continue; + } + + if (!add_row(catalog_name, schema_name, table_name, pk_key, + catalog_name, schema_name, lookup_table, pk_key, + ord_pos)) { + return status; + } + } } + } + return result_rows; + } - auto row_status = CreateResultSetForForeignKeys( - pk_catalog_name, pk_schema_name, pk_table_name, - column_reference.referenced_column, catalog_name, schema_name, - table_name, column_reference.referencing_column, ord_pos); + auto const& foreign_keys = metadata.table_constraints.foreign_keys; - if (!row_status) { - return row_status.GetStatusRecord(); + for (auto const& fk_key : foreign_keys) { + if (fk_key.referenced_table.table_id != pk_table_name) { + continue; + } + + int ord_pos = 1; + for (auto const& col_ref : fk_key.column_references) { + if (!is_in_key_columns(col_ref.referenced_column)) { + continue; + } + if (!add_row(pk_catalog_name, pk_schema_name, pk_table_name, + col_ref.referenced_column, catalog_name, schema_name, + table_name, col_ref.referencing_column, ord_pos)) { + return status; } - result_rows.emplace_back(std::move(*row_status)); - ++ord_pos; } } return result_rows; @@ -281,13 +310,16 @@ StatusRecordOr FetchFKResultSetFromTableMetaData( return table_metadata_status.GetStatusRecord(); } - auto const& pk_columns = - table_metadata_status->table_constraints.primary_key.columns; + auto key_cols = table_metadata_status->table_constraints.primary_key.columns; + auto fk_col_obj = table_metadata_status->table_constraints.foreign_keys; + bool has_pk_table_only = (!pk_table_name.empty() && fk_table_name.empty()); + // case : When both pk & fk table provided if (!pk_table_name.empty() && !fk_table_name.empty()) { auto row_status = CreateFKResultRows( conn_handle, catalog_name, schema_name, fk_table_name, pk_catalog_name, - pk_schema_name, pk_table_name, pk_columns, false); + pk_schema_name, pk_table_name, lookup_table, key_cols, fk_col_obj, + true); if (!row_status) { return row_status.GetStatusRecord(); } @@ -298,7 +330,17 @@ StatusRecordOr FetchFKResultSetFromTableMetaData( return result_set; } - // case: either pk or fk table provided + // case: when fk table provided only + if (pk_table_name.empty() && !fk_table_name.empty()) { + key_cols.clear(); + for (auto const& fk : + table_metadata_status->table_constraints.foreign_keys) { + for (auto const& col_ref : fk.column_references) { + key_cols.push_back(col_ref.referencing_column); + } + } + } + Options opts; auto table_status = conn_handle.GetClient()->ListAllTables(catalog_name, schema_name, opts); @@ -306,17 +348,18 @@ StatusRecordOr FetchFKResultSetFromTableMetaData( return table_status.GetStatusRecord(); } - bool has_pk_table_only = !pk_table_name.empty(); std::vector>> futures; for (auto const& table : *table_status) { futures.emplace_back(std::async( std::launch::async, [&conn_handle, catalog_name, schema_name, table, pk_catalog_name, - pk_schema_name, pk_table_name, pk_columns, has_pk_table_only]() { - return CreateFKResultRows( - conn_handle, catalog_name, schema_name, - table.table_reference.table_id, pk_catalog_name, pk_schema_name, - pk_table_name, pk_columns, has_pk_table_only); + pk_schema_name, pk_table_name, lookup_table, key_cols, fk_col_obj, + has_pk_table_only]() { + return CreateFKResultRows(conn_handle, catalog_name, schema_name, + table.table_reference.table_id, + pk_catalog_name, pk_schema_name, + pk_table_name, lookup_table, key_cols, + fk_col_obj, has_pk_table_only); })); } diff --git a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h index e42aa73565..0c11186042 100644 --- a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h +++ b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h @@ -26,6 +26,7 @@ #include namespace google::cloud::odbc_bq_driver_internal { +using ::google::cloud::bigquery_v2_minimal_internal::ForeignKey; // Executes a BQ query and fetches the foreign key results and // populates the DSResults, as mentioned below: @@ -60,7 +61,8 @@ StatusRecordOr CreateFKResultRows( ConnectionHandle& conn_handle, std::string const& catalog_name, std::string const& schema_name, std::string const& table_name, std::string const& pk_catalog_name, std::string const& pk_schema_name, - std::string const& pk_table_name, std::vector pk_key_columns, + std::string const& pk_table_name, std::string const& lookup_table, + std::vector key_cols, std::vector fk_col_obj, bool const& has_pk_table_only = false); odbc_internal::StatusRecordOr FetchFKResultSetFromTableMetaData( diff --git a/google/cloud/odbc/bq_driver/odbc_driver_metadata.cc b/google/cloud/odbc/bq_driver/odbc_driver_metadata.cc index ece939cddd..3733e27374 100644 --- a/google/cloud/odbc/bq_driver/odbc_driver_metadata.cc +++ b/google/cloud/odbc/bq_driver/odbc_driver_metadata.cc @@ -32,7 +32,6 @@ using google::cloud::odbc_bq_driver_internal::ConnectionHandle; using google::cloud::odbc_bq_driver_internal::CreateResultSetForTableTypes; using google::cloud::odbc_bq_driver_internal::DescriptorHandle; using google::cloud::odbc_bq_driver_internal::DescriptorType; -using google::cloud::odbc_bq_driver_internal::DSResults; using google::cloud::odbc_bq_driver_internal::FetchBQSQLProceduresData; using google::cloud::odbc_bq_driver_internal::FetchBQTablesData; using google::cloud::odbc_bq_driver_internal::FetchFKResultSetFromTableMetaData; @@ -52,7 +51,6 @@ using google::cloud::odbc_bq_driver_internal::LogAndReturnCode; using google::cloud::odbc_bq_driver_internal::PopulateSupportedODBC2Functions; using google::cloud::odbc_bq_driver_internal::PopulateSupportedODBC3Functions; using google::cloud::odbc_bq_driver_internal::ProcessProcedures; -using google::cloud::odbc_bq_driver_internal::ProcessQueryResults; using google::cloud::odbc_bq_driver_internal::ProcessTableResults; using google::cloud::odbc_bq_driver_internal::ResultSet; using google::cloud::odbc_bq_driver_internal::SanitizeIdentifierArgument; From 57558f5b39a80a45476fe1cd390ce5ed08b4d9af Mon Sep 17 00:00:00 2001 From: Neeraj Dwivedi Date: Thu, 23 Jul 2026 18:25:46 +0530 Subject: [PATCH 3/3] fix clang-idy issue --- .../cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc | 4 ++-- .../cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h | 3 ++- .../examples/catalog_performance_example.cc | 2 +- google/cloud/odbc/testing/odbc_utils/catalog.cc | 7 ------- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc index 94862e8ea9..bb1b26a40b 100644 --- a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc +++ b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.cc @@ -135,8 +135,8 @@ StatusRecordOr CreateFKResultRows( std::string const& schema_name, std::string const& table_name, std::string const& pk_catalog_name, std::string const& pk_schema_name, std::string const& pk_table_name, std::string const& lookup_table, - std::vector key_columns, std::vector fk_col_obj, - bool const& has_pk_table_only) { + std::vector const& key_columns, + std::vector const& fk_col_obj, bool const& has_pk_table_only) { StatusRecord status; ResultSetRows result_rows; if ((table_name == pk_table_name || table_name == lookup_table) && diff --git a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h index 0c11186042..c2a87db01a 100644 --- a/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h +++ b/google/cloud/odbc/bq_driver/internal/odbc_sql_foreign_keys.h @@ -62,7 +62,8 @@ StatusRecordOr CreateFKResultRows( std::string const& schema_name, std::string const& table_name, std::string const& pk_catalog_name, std::string const& pk_schema_name, std::string const& pk_table_name, std::string const& lookup_table, - std::vector key_cols, std::vector fk_col_obj, + std::vector const& key_columns, + std::vector const& fk_col_obj, bool const& has_pk_table_only = false); odbc_internal::StatusRecordOr FetchFKResultSetFromTableMetaData( diff --git a/google/cloud/odbc/integration_tests/odbc_driver_tests/examples/catalog_performance_example.cc b/google/cloud/odbc/integration_tests/odbc_driver_tests/examples/catalog_performance_example.cc index 941d7d3c75..83296b0bc4 100644 --- a/google/cloud/odbc/integration_tests/odbc_driver_tests/examples/catalog_performance_example.cc +++ b/google/cloud/odbc/integration_tests/odbc_driver_tests/examples/catalog_performance_example.cc @@ -18,11 +18,11 @@ #include "google/cloud/odbc/testing/odbc_utils/connection.h" #include "google/cloud/odbc/testing/odbc_utils/statement.h" #include -#include #include #include #include #include + namespace google::cloud::odbc_tests { class CatalogPerformanceHtapiParamTest : public ::testing::TestWithParam { diff --git a/google/cloud/odbc/testing/odbc_utils/catalog.cc b/google/cloud/odbc/testing/odbc_utils/catalog.cc index 8b69da3223..2a37e4b52b 100644 --- a/google/cloud/odbc/testing/odbc_utils/catalog.cc +++ b/google/cloud/odbc/testing/odbc_utils/catalog.cc @@ -448,7 +448,6 @@ RowWiseResults Catalog::GetForeignKeys(std::shared_ptr const& conn, columns[i].buffer_length, &(columns[i].str_len)); CheckError(status, "SQLBindCol", conn); } - auto start = std::chrono::high_resolution_clock::now(); if (!pk_table.empty() && !fk_table.empty()) { if (use_ansi) { @@ -549,12 +548,6 @@ RowWiseResults Catalog::GetForeignKeys(std::shared_ptr const& conn, static_cast(fk_table.length())); } } - auto end = std::chrono::high_resolution_clock::now(); - auto duration_us = - std::chrono::duration_cast(end - start); - - std::cout << "SQLForeignKeys took " << duration_us.count() / 1000 << " ms" - << std::endl; CheckError(status, "SQLForeignKeys", conn, use_ansi); while (true) {