Skip to content

fix(core): preserve CTE names during schema prefixing in SQLDatabase.run_sql - #21028

Merged
AstraBert merged 2 commits into
run-llama:mainfrom
fury0928:fix/sql-wrapper-cte-support
Mar 17, 2026
Merged

fix(core): preserve CTE names during schema prefixing in SQLDatabase.run_sql#21028
AstraBert merged 2 commits into
run-llama:mainfrom
fury0928:fix/sql-wrapper-cte-support

Conversation

@fury0928

Copy link
Copy Markdown
Contributor

Description

Fix naive str.replace("FROM ", ...) in SQLDatabase.run_sql() that incorrectly schema-prefixes CTE (Common Table Expression) names, breaking queries like:

WITH my_cte AS (SELECT * FROM users) SELECT * FROM my_cte
-- Before (broken): ... SELECT * FROM myschema.my_cte
-- After (fixed):   ... SELECT * FROM my_cte

Replace with regex-based _add_schema_prefix() method that:

  1. Extracts CTE names from WITH [RECURSIVE] name AS ( clauses (including comma-separated CTEs)
  2. Uses regex to match FROM/JOIN table references
  3. Skips CTE names and already schema-qualified identifiers when prefixing

Fixes #19889

Version Bump?

  • No (llama-index-core is exempt per PR template)

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • I added new unit tests to cover this change

10 new tests covering:

  • Simple FROM/JOIN prefixing
  • Single CTE name preservation
  • Multiple CTEs (WITH a AS (...), b AS (...))
  • Recursive CTEs (WITH RECURSIVE)
  • Already schema-qualified names (other_schema.table)
  • Multi-word JOINs (LEFT JOIN)
  • Subqueries (FROM (SELECT ...))
  • CTE + real table in same query
  • Case-insensitive matching

All 16 tests pass (6 existing + 10 new). Pre-commit hooks, ruff, mypy all pass.

Suggested Checklist:

  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I ran uv run make format; uv run make lint to appease the lint gods

…run_sql

The naive str.replace("FROM ", ...) in run_sql() incorrectly schema-prefixes
CTE (Common Table Expression) names, breaking queries like:
  WITH my_cte AS (SELECT * FROM t) SELECT * FROM my_cte

Replace with regex-based _add_schema_prefix() that extracts CTE names from
WITH clauses and skips them (and already-qualified identifiers) when prefixing.

Fixes run-llama#19889
…run_sql

The naive str.replace("FROM ", ...) in run_sql() incorrectly schema-prefixes
CTE (Common Table Expression) names, breaking queries like:
  WITH my_cte AS (SELECT * FROM t) SELECT * FROM my_cte

Replace with regex-based _add_schema_prefix() that extracts CTE names from
WITH clauses and skips them (and already-qualified identifiers) when prefixing.

Fixes run-llama#19889
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 16, 2026
@fury0928

Copy link
Copy Markdown
Contributor Author

Hi @AstraBert thanks for your time on the review
Would you review this also if you have a moment?
Appreciate it

@AstraBert

Copy link
Copy Markdown
Contributor

Please do not ping maintainers as soon as you open a PR. We go through all PRs at our own pace

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Mar 17, 2026
@AstraBert
AstraBert merged commit b87f8bd into run-llama:main Mar 17, 2026
11 checks passed
claytonlin1110 pushed a commit to claytonlin1110/llama_index that referenced this pull request Mar 25, 2026
…run_sql (run-llama#21028)

* fix(core): preserve CTE names during schema prefixing in SQLDatabase.run_sql

The naive str.replace("FROM ", ...) in run_sql() incorrectly schema-prefixes
CTE (Common Table Expression) names, breaking queries like:
  WITH my_cte AS (SELECT * FROM t) SELECT * FROM my_cte

Replace with regex-based _add_schema_prefix() that extracts CTE names from
WITH clauses and skips them (and already-qualified identifiers) when prefixing.

Fixes run-llama#19889

* fix(core): preserve CTE names during schema prefixing in SQLDatabase.run_sql

The naive str.replace("FROM ", ...) in run_sql() incorrectly schema-prefixes
CTE (Common Table Expression) names, breaking queries like:
  WITH my_cte AS (SELECT * FROM t) SELECT * FROM my_cte

Replace with regex-based _add_schema_prefix() that extracts CTE names from
WITH clauses and skips them (and already-qualified identifiers) when prefixing.

Fixes run-llama#19889
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature Request]: Add CTE support in SQLWrapper

2 participants