Fixed table name resolution.

[khustup2]

🚀 🚀 Pull Request

Impact

• Bug fix (non-breaking change which fixes expected existing functionality)
• Enhancement/New feature (adds functionality without impacting existing logic)
• Breaking change (fix or feature that would cause existing functionality to change)

Description

Things to be aware of

Things to worry about

Additional Context

[Copilot]

Pull request overview

This PR fixes table name resolution issues when PostgreSQL adds hash suffixes to table names (e.g., after DDL operations). The fix ensures the extension uses actual table names from the PostgreSQL catalog instead of potentially stale cached metadata, preventing "table does not exist" errors in DuckDB.

Key changes:

• Retrieves qualified table names directly from PostgreSQL catalog during metadata loading
• Adds comprehensive test coverage for hash suffix scenarios
• Improves error handling and logging throughout the query execution path

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
cpp/deeplake_pg/table_storage.cpp	Adds get_qualified_table_name() helper and updates metadata loading to use actual catalog names
cpp/deeplake_pg/table_data_impl.hpp	Changes log levels from WARNING to DEBUG1 for routine operations
cpp/deeplake_pg/extension_init.cpp	Adds error handling for cleanup operations during transaction rollback
cpp/deeplake_pg/duckdb_executor.cpp	Improves error handling and adds debug logging for DuckDB operations
postgres/tests/py_tests/test_table_name_resolution.py	Adds comprehensive test suite for table name resolution with hash suffixes

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The include order has been changed. The standard convention is to include the corresponding header file first (duckdb_executor.hpp) before other project headers. This change moves it after other includes, which deviates from common C++ style guidelines where the matching header comes first to ensure it's self-contained.

Suggested change

	#include "duckdb_deeplake_convert.hpp"
	#include "duckdb_deeplake_scan.hpp"
	#include "duckdb_executor.hpp"
	#include "duckdb_executor.hpp"
	#include "duckdb_deeplake_convert.hpp"
	#include "duckdb_deeplake_scan.hpp"

[Copilot]

Removing error checking from execute_query means callers must now handle errors themselves. Since this function is still named execute_query, the removal of error handling makes its behavior less clear and shifts responsibility to all call sites. Consider renaming to something like send_query_unchecked to make the lack of error handling explicit, or keep the error checking here for consistency.

Suggested change

	return con->SendQuery(query);
	try {
	auto result = con->SendQuery(query);
	if (!result) {
	elog(ERROR, "DuckDB query execution failed: null result returned");
	return nullptr;
	}
	if (result->HasError()) {
	elog(ERROR, "DuckDB query execution failed: %s", result->GetError().c_str());
	}
	return result;
	} catch (const std::exception& e) {
	elog(ERROR, "DuckDB query execution threw an exception: %s", e.what());
	} catch (...) {
	elog(ERROR, "DuckDB query execution threw an unknown exception");
	}
	return nullptr;

[Copilot]

These cleanup operations are now wrapped in a try-catch that suppresses exceptions, but if one operation fails, the subsequent operations still execute. Consider whether partial cleanup could leave the system in an inconsistent state. If order matters, each operation might need individual error handling to ensure critical cleanup steps complete even if earlier ones fail.

Suggested change

	pg::query_info::cleanup();
	pg::table_storage::instance().reset_requested_columns();
	} catch (const std::exception& e) {
	// Log cleanup errors but don't let them prevent the original error from being reported
	elog(WARNING, "Error during transaction cleanup: %s", e.what());
	} catch (...) {
	elog(WARNING, "Unknown error during transaction cleanup");
	} catch (const std::exception& e) {
	// Log cleanup errors but don't let them prevent the original error from being reported
	elog(WARNING, "Error during transaction cleanup (rollback_all): %s", e.what());
	} catch (...) {
	elog(WARNING, "Unknown error during transaction cleanup (rollback_all)");
	}
	try {
	pg::query_info::cleanup();
	} catch (const std::exception& e) {
	elog(WARNING, "Error during transaction cleanup (query_info::cleanup): %s", e.what());
	} catch (...) {
	elog(WARNING, "Unknown error during transaction cleanup (query_info::cleanup)");
	}
	try {
	pg::table_storage::instance().reset_requested_columns();
	} catch (const std::exception& e) {
	elog(WARNING, "Error during transaction cleanup (reset_requested_columns): %s", e.what());
	} catch (...) {
	elog(WARNING, "Unknown error during transaction cleanup (reset_requested_columns)");

[sonarqubecloud]

Quality Gate passed

Issues
8 New issues
1 Accepted issue

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarQube Cloud