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

feat(pg): implement QueryBuilder#updateFrom #5386

Merged
merged 2 commits into from Jul 6, 2023

Conversation

wms
Copy link
Contributor

@wms wms commented Nov 17, 2022

Documentation PR: knex/documentation#476

Introduces the updateFrom querybuilder method for PostgreSQL. This allows the creation of UPDATE <table> SET <data> FROM <name> queries, which may reference relations or CTEs outside of the target table.

I opted for creating a new QB method (updateFrom) to avoid conflict or weird interactions or unexpected changes to behaviour with the target table already being inferred from knex(table), knex.table(table), etc.

@coveralls
Copy link

coveralls commented Nov 17, 2022

Coverage Status

coverage: 92.792% (+0.002%) from 92.79% when pulling 460f4cf on wms:pg-update-from into 8e2944d on knex:master.

@wms
Copy link
Contributor Author

wms commented Nov 17, 2022

Sorry, looks like I've not fully understood how the testsuite works; its attempting to run pg-specific QB methods against other DB engines.

Is there documentation on how to correctly place my tests so that they only run against the correct DB engine(s) during CI?

Copy link
Collaborator

@OlivierCavadenti OlivierCavadenti left a comment

Choose a reason for hiding this comment

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

It's only for PG so in tests you need to skip the test for each other non-PG db.
BY the way, we are sure only PG support update from ? Cannot be added for other DB?

if (!isPostgreSQL(knex)) {
  return this.skip();
}

@wms
Copy link
Contributor Author

wms commented Nov 18, 2022

BY the way, we are sure only PG support update from ? Cannot be added for other DB?

I've not done an exhaustive search, but I know that (and am in need of support for) PostgreSQL handles this.

@OlivierCavadenti OlivierCavadenti merged commit 61ee533 into knex:master Jul 6, 2023
53 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.

None yet

3 participants