Add Denodo integration handler#10432
Conversation
|
@quest-bot loot #10057 |
|
ZoranPandovski
left a comment
There was a problem hiding this comment.
Thanks. The only question I have is the usage of mysql dialect and mentioning denodo dialect. I've never used Denodo it looks like they have their own VQL?
cd521a6 to
777125f
Compare
|
@ritwickrajmakhal |
|
Done ✅ |
Co-authored-by: andrew <elkin.andr@gmail.com>
Co-authored-by: martyna-mindsdb <109554435+martyna-mindsdb@users.noreply.github.com>
0165378 to
09af05e
Compare
| def get_columns(self, table_name: str) -> Response: | ||
| """ | ||
| Get columns for specified table using parameterized query. | ||
| """ |
There was a problem hiding this comment.
Correctness: table_name is directly interpolated into the SQL string despite the docstring claiming it's parameterized — this allows SQL injection via a crafted table name.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In mindsdb/integrations/handlers/denodo_handler/denodo_handler.py at line 148, the `get_columns` method builds a SQL string by directly interpolating `table_name` and `self.database` using an f-string. This allows SQL injection. Fix by using parameterized query execution (pass parameters separately to `cursor.execute`) instead of string interpolation.
| logger.error(f"Error connecting to Denodo: {str(e)}") | ||
| raise ConnectionError(f"Failed to connect to Denodo: {str(e)}") | ||
|
|
||
| def disconnect(self) -> None: |
There was a problem hiding this comment.
Duplicate Code:
This function disconnect duplicates existing code.
📍 Original Location:
mindsdb/integrations/handlers/postgres_handler/postgres_handler.py:211-218
Function: disconnect
💡 Recommendation:
Remove disconnect from DenodoHandler entirely by inheriting from PostgresHandler. If the psycopg2 vs psycopg3 driver difference requires a custom disconnect, keep a minimal override that only addresses the driver-specific teardown.
Consider importing and reusing the existing function instead of duplicating the logic.
| - `user`: The username for the Denodo database. | ||
| - `password`: The password for the Denodo database. | ||
| - `host`: The hostname, IP address, or URL of the Denodo server. | ||
| - `port`: The port number for connecting to the Denodo server (default is `9999`). |
| password=self.connection_data.get("password"), | ||
| database=self.connection_data.get("database"), | ||
| ) | ||
| return self.connection |
There was a problem hiding this comment.
will be good to set self.is_connected=True

Description
This pull request introduces the integration of MindsDB with Denodo, a data virtualization platform. The changes include adding documentation, defining connection parameters, and implementing the Denodo handler to manage database interactions.
Fixes: #10057
Type of change
Additional Media:
2025-01-31.23-49-56.mp4
Checklist: