You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Preserve duplicate column values in Postgres, MySQL, and SQLite connectors - #491
What: queries returning two columns with the same name (SELECT c.id, o.id …, SELECT * over a join) gave silently wrong values on Postgres, MySQL and SQLite, so text-to-SQL answered from them. Frames are now built from row tuples and cursor.description. Risk: low. Frames can now contain repeated column names; every consumer of agent-SQL results was checked (see below). Look at:_fetch_frame in postgres.py, the MySQL cursor, and SQLite row building. Each is a few lines. Verified: the new tests fail on the old code. The opt-in live MySQL suite passed against MySQL 8.4. Est. review: ~10 min.
Summary
When a query returns two columns with the same name (SELECT c.id, o.id, c.name, o.name …, or SELECT * over a join), the Postgres, MySQL and SQLite connectors returned wrong data with no error. Each row was built as a dict keyed by column name, so one value overwrote the other before the DataFrame existed:
Connector
Before
After
Postgres
columns id, name, row 100, first order
id, id, name, name / 1, 100, Ada, first order
MySQL
100, 100, first order, first order
1, 100, Ada, first order
SQLite
1, 1, Ada, Ada
1, 100, Ada, first order
Text-to-SQL then answered from the wrong values without retrying. The fix builds frames from row tuples plus cursor.description. DuckDB was already correct.
Is it safe for downstream readers?
Frames from arbitrary agent SQL can now contain repeated column names, so I checked every reader of execute_chat_sql / _run_sql output:
sql_execution._run_sql already de-duplicates names (id, id__2) before serializing.
numeric_column_kinds and result_values_digest read by position.
db_probeto_dict(orient="records") still collapses duplicates, as it did before, inside a try.
Catalog metadata queries have unique column names, so they're unaffected.
Provenance
This re-lands an external contribution: upstream PR 422, authored by HUSEIN. Commit authorship is preserved. The code is identical, rebased onto current main. Only the commit title's issue reference was reworded, so pushing doesn't post to the public issue before review.
Verification
The new tests fail on the old connector code (6 failures) and pass here.
Live MySQL 8.4 (the opt-in AUTO_ONTOLOGY_LIVE_MYSQL=1 suite, which CI doesn't run): 6 of 6 pass. A duplicate-name join returns ['id','id','name','name'] [[1, 100, 'Ada', 'first order']].
Full suite against local Postgres: all pass except Kumo tests that need the kumo extra (they run in CI).
Merge checklist
Before merge, add the closing reference for upstream issue 408 to this description (the Fixes keyword plus the issue number).
After merge, close upstream PR 422 with a thank-you pointing here.
Summary by CodeRabbit
Bug Fixes
Query results now preserve distinct values when multiple columns share the same name.
SQL result data retains duplicate column labels, including for empty PostgreSQL query results.
When duplicate fields are serialized, they receive distinct keys so their values remain accessible.
No actionable comments were generated in the recent review.
Walkthrough
MySQL, PostgreSQL, and SQLite now construct query-result DataFrames from tuple rows and cursor column names. Tests cover duplicate column names and values, including their serialization by the text-to-SQL path.
Changes: Tuple-Based SQL Result Handling
Layer / File(s)
Summary
Connector tuple rows and regression tests auto_ontology/connectors/*.py, auto_ontology/connectors/tests/*
The MySQL and PostgreSQL connectors use tuple rows and cursor metadata to build DataFrames. SQLite builds DataFrame rows from tuples. Tests check duplicate column labels and values, empty PostgreSQL results, and tuple-based timeout fixtures.
Text-to-SQL result serialization auto_ontology/retrieval/text_to_sql/tests/test_sql_execution.py
A SQLite integration test checks duplicate labels and values from a joined query. It also checks that _run_sql serializes repeated keys with __2 suffixes and closes the connector.
This change keeps all values when a SQL query returns columns with the same name, instead of silently overwriting some of them. No merge-blocking risk was found.
Security architecture risk: Minimal · up to 64f74
Query results now preserve selected values without expanding database access. The inspected changes do not weaken timeout handling, connection ownership, or error containment, and no introduced security concern was established.
Retained concerns
No architecture-level concerns identified.
Security review details
Security Blast Radius
inferred — The changed behavior affects results returned by already configured database connections or the configured SQLite file. Preserving additional values from an already selected projection does not itself add a connection target, SQL capability, or privilege.
Trust Boundaries and Controls
observed — The changed connector paths continue to execute caller-supplied SQL and parameters using their configured connections. The row changes occur after statement execution and do not introduce a new authorization mechanism or bypass an existing check in these paths.
Resilience and Maintainability Implications
observed — The inspected lifecycle wrappers are unchanged: MySQL closes its per-call cursor and connection through finally blocks; PostgreSQL scopes capped execution to a transaction on a pooled connection; SQLite uses thread-local connections and clears its timeout progress handler in a finally block. Row conversion remains within these existing success and failure paths.
Pre-merge checks: 6
Check name
Status
Explanation
Title check
The title names the change: duplicate column values are preserved in the Postgres, MySQL, and SQLite connectors. The raw summary and objectives confirm that these connectors now build DataFrames from …
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Tests For Backend Behaviour Changes
The PR changes runtime behaviour under auto_ontology/. It alters how the Postgres, MySQL, and SQLite connectors build DataFrames from query rows (`auto_ontology/connectors/{postgres,mysql,sqlite}.py…
Migration Accompanies Schema Change
The check applies only when auto_ontology/dal/schema.py is modified. The changed-file inventory for refs/coderabbit/pre-merge-check/base..head lists seven modified files: connectors/mysql.py, `c…
Full details: Title check
Explanation
The title names the change: duplicate column values are preserved in the Postgres, MySQL, and SQLite connectors. The raw summary and objectives confirm that these connectors now build DataFrames from row tuples and cursor.description rather than dictionaries keyed by column name. The title is specific and matches the main change.
Full details: Tests For Backend Behaviour Changes
Explanation
The PR changes runtime behaviour under auto_ontology/. It alters how the Postgres, MySQL, and SQLite connectors build DataFrames from query rows (auto_ontology/connectors/{postgres,mysql,sqlite}.py). The PR also adds or updates tests in the same change set: - auto_ontology/connectors/tests/test_mysql.py: test_execute_preserves_duplicate_column_values (new). - auto_ontology/connectors/tests/test_postgres.py: test_fetch_frame_preserves_duplicate_column_values and test_fetch_frame_keeps_column_names_on_empty_result (new). - auto_ontology/connectors/tests/test_statement_timeouts.py: fake cursors updated to tuple rows. - auto_ontology/retrieval/text_to_sql/tests/test_sql_execution.py: test_sqlite_connector_preserves_duplicate_column_values_through_run_sql (new). The check asks for tests "under auto_ontology/tests or dev_tools". The new tests sit in package-level tests/ directories inside auto_ontology/. This is the repository's established layout. I read the check's intent as requiring tests in the auto_ontology/ tree, and these satisfy it. A strict path-only reading would fail this PR. That reading does not match how the repository organizes its tests. The PR description also explains test coverage and live MySQL verification. The check does not require that explanation, because tests were added.
Full details: Migration Accompanies Schema Change
Explanation
The check applies only when auto_ontology/dal/schema.py is modified. The changed-file inventory for refs/coderabbit/pre-merge-check/base..head lists seven modified files: connectors/mysql.py, connectors/postgres.py, connectors/sqlite.py, three connector test files, and retrieval/text_to_sql/tests/test_sql_execution.py. Neither auto_ontology/dal/schema.py nor any path under alembic/ appears in the diff. The check's trigger condition is not met, so the check is inapplicable and passes. No Alembic revision is required for this PR.
Finishing Touches
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reviewer guide
What: queries returning two columns with the same name (
SELECT c.id, o.id …,SELECT *over a join) gave silently wrong values on Postgres, MySQL and SQLite, so text-to-SQL answered from them. Frames are now built from row tuples andcursor.description.Risk: low. Frames can now contain repeated column names; every consumer of agent-SQL results was checked (see below).
Look at:
_fetch_frameinpostgres.py, the MySQL cursor, and SQLite row building. Each is a few lines.Verified: the new tests fail on the old code. The opt-in live MySQL suite passed against MySQL 8.4.
Est. review: ~10 min.
Summary
When a query returns two columns with the same name (
SELECT c.id, o.id, c.name, o.name …, orSELECT *over a join), the Postgres, MySQL and SQLite connectors returned wrong data with no error. Each row was built as a dict keyed by column name, so one value overwrote the other before the DataFrame existed:id, name, row100, first orderid, id, name, name/1, 100, Ada, first order100, 100, first order, first order1, 100, Ada, first order1, 1, Ada, Ada1, 100, Ada, first orderText-to-SQL then answered from the wrong values without retrying. The fix builds frames from row tuples plus
cursor.description. DuckDB was already correct.Is it safe for downstream readers?
Frames from arbitrary agent SQL can now contain repeated column names, so I checked every reader of
execute_chat_sql/_run_sqloutput:sql_execution._run_sqlalready de-duplicates names (id,id__2) before serializing.numeric_column_kindsandresult_values_digestread by position.db_probeto_dict(orient="records")still collapses duplicates, as it did before, inside atry.Catalog metadata queries have unique column names, so they're unaffected.
Provenance
This re-lands an external contribution: upstream PR 422, authored by HUSEIN. Commit authorship is preserved. The code is identical, rebased onto current
main. Only the commit title's issue reference was reworded, so pushing doesn't post to the public issue before review.Verification
AUTO_ONTOLOGY_LIVE_MYSQL=1suite, which CI doesn't run): 6 of 6 pass. A duplicate-name join returns['id','id','name','name'] [[1, 100, 'Ada', 'first order']].kumoextra (they run in CI).Merge checklist
Summary by CodeRabbit