Skip to content

Conversation

@nicosuave
Copy link
Member

Fixes a bug where lowercase or multiline FROM prevented alias injection. Adds a regression test for lowercase from with line breaks.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 565b715d8f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +841 to +843
fn insert_table_alias(sql: &str, table_name: &str, alias: &str) -> Option<String> {
let from_pos = find_top_level_keyword(sql, "FROM", 0)?;
let bytes = sql.as_bytes();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip comments when locating FROM for alias insertion

insert_table_alias relies on find_top_level_keyword to grab the first FROM at depth 0, but that helper does not skip SQL comments. If the query contains a -- ... from <table> or /* from <table> */ comment before the real FROM clause, this will insert _outer into the comment and return Some(_), so later expansion uses _outer even though the actual FROM clause was never updated. That yields correlated references like _outer.col against a query that has no _outer alias, causing SQL errors in any statement that needs outer correlation. Consider ignoring comments in find_top_level_keyword or searching for the first FROM token that is not inside a comment before inserting the alias.

Useful? React with 👍 / 👎.

@nicosuave nicosuave merged commit 63c677f into main Jan 9, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants