Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Update documentation for conflict to include table alias #6020

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jsk95
Copy link

@jsk95 jsk95 commented Mar 4, 2024

Problem

The current documentation for where after onConflict + merge does not seem correct.
Following the example, we receive the error column reference "x" is ambiguous.

Root cause

This is because in the where condition of a onConflict + merge, we have access to both the table and excluded aliases so specifying a column without being explicit results in that error.
https://www.postgresql.org/docs/current/sql-insert.html
https://dba.stackexchange.com/a/224167

We can actually see the table name being directly referenced in the test case that this scenario was created for https://github.com/knex/knex/pull/4148/files#diff-57a163701cfad49b7c6cbdfb102667277345ba8de7e575f6966269b0c0d8d3a0R1557

Solution

We could update with a custom where condition to actually use table name
https://github.com/knex/knex/blame/4ca3dd5bc28e0665c5bed55026fac2ec45489d81/lib/dialects/postgres/query/pg-querycompiler.js#L37

but implicitly choosing one or the other feels dangerous.

So this proposal is just to fix the documentation that shows the feature to work.

@coveralls
Copy link

Coverage Status

coverage: 92.843%. remained the same
when pulling 792a4ea on jsk95:patch-1
into 9659a20 on knex:master.

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.

None yet

2 participants