Skip to content

fix: join with table with column named "source"#5468

Merged
max-sixty merged 1 commit intoPRQL:mainfrom
gosupersimple:fix-join-fails-when-both-sides-have-column-named-source-5015
Oct 7, 2025
Merged

fix: join with table with column named "source"#5468
max-sixty merged 1 commit intoPRQL:mainfrom
gosupersimple:fix-join-fails-when-both-sides-have-column-named-source-5015

Conversation

@priithaamer
Copy link
Copy Markdown
Contributor

Attempting to fix #5015 by letting Claude to grind on it for 30 mins. My own understanding of the internals of PRQL is still very limited. Here's the report from Claude:

Root Cause

The problem occurred in prqlc/prqlc/src/semantic/resolver/functions.rs in the resolve_function_args function. When resolving relational arguments for functions like join:

  1. Arguments were being resolved one at a time in a loop
  2. After each argument was resolved, its columns were immediately added to the NS_THAT namespace
  3. This meant that when resolving the second relational argument, the first argument's columns were already visible in NS_THAT
  4. If both arguments referenced the same variable (like join a), the second reference would see the first one's columns, creating a circular dependency
  5. This caused column lookups to find both the actual column AND the function parameter from the _param namespace, resulting in ambiguity

The Fix

Changed the resolution order in resolve_function_args:

  • Before: Resolve argument → Add to namespace → Resolve next argument → Add to namespace
  • After: Resolve ALL arguments first → Then add all to namespaces

This ensures that when resolving variable references, they don't see partially-populated namespaces from other arguments being processed in the same function call.

Testing

The fix is minimal, surgical, and preserves all existing functionality while resolving the circular reference issue.

@priithaamer
Copy link
Copy Markdown
Contributor Author

@max-sixty hey, would be happy to get your eyes on this

@max-sixty
Copy link
Copy Markdown
Member

@claude can you confirm whether source is in our reserved keywords?

@claude
Copy link
Copy Markdown

claude Bot commented Oct 7, 2025

Claude encountered an error —— View job

Failed with exit code 128

I'll analyze this and get back to you.

@max-sixty
Copy link
Copy Markdown
Member

thanks @priithaamer ! looks good! I'll check whether source is in the reserved keywords already, I don't think that's going to help as I think it's a different part of the pipeline. so feel free to merge assuming that's correct!

@max-sixty
Copy link
Copy Markdown
Member

@claude try again?

@claude
Copy link
Copy Markdown

claude Bot commented Oct 7, 2025

Claude encountered an error —— View job

Failed with exit code 128

I'll analyze this and get back to you.

@max-sixty
Copy link
Copy Markdown
Member

(OK I guess Claude is broken)

I'll hit the button, thanks @priithaamer

@max-sixty max-sixty merged commit 9c01984 into PRQL:main Oct 7, 2025
36 checks passed
@priithaamer priithaamer deleted the fix-join-fails-when-both-sides-have-column-named-source-5015 branch October 7, 2025 18:07
@priithaamer
Copy link
Copy Markdown
Contributor Author

(OK I guess Claude is broken)

I'll hit the button, thanks @priithaamer

Thanks! Claude needs its me-time too. Will there be a release any time soon? If not, i'll resort to our internal build of the npm package.

@max-sixty
Copy link
Copy Markdown
Member

yes, we're a bit overdue for a release! I may try to finish the chumsky refactor so we're not pulling in two versions of chumsky. if I don't get that done in the next day or two, I'll do one; (feel free to remind me)

@max-sixty
Copy link
Copy Markdown
Member

Claude needs its me-time too.

lol

fyi I looked into this and it apparently doesn't work with forks (!?)

@priithaamer
Copy link
Copy Markdown
Contributor Author

Fwiw, i found a regression related to append-s which breaks many UNION ALL queries for us. I'll try to come up with minimal example and will provide the details. Maybe might be worth holding back the release.

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.

join fails when both sides have column named "source"

2 participants