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

ONLY keyword support for PostgreSQL #1874

Merged
merged 4 commits into from Jan 26, 2017

Conversation

Projects
None yet
3 participants
@tomaspinho
Contributor

tomaspinho commented Jan 20, 2017

Hello, we're requiring the ability to use the ONLY keyword in PostgreSQL and I have done a crude implementation of it in this PR.
Please discuss and provide any comments that would enable this or other PR to be merged that implements the same functionality.
Thank you!

adding support for ONLY keyword for PostgreSQL as an optional flag of…
… from() and into() through table(tableName, only)
@elhigu

This comment has been minimized.

Collaborator

elhigu commented Jan 22, 2017

Sounds like ONLY is actually not postgresql specific functionality, but comes from standard... any ideas if mysql or others support it?

Generally pull request looks good, change is so simple that I'm fine that there is no integration tests for this since the change is so simple.

One thing there is that I would like to change. Instead of true/false parameter would be more future proof to pass { only: true } object instead.

Could you make also pull request to documentation repository and add link to it here, so I can check that documentation will be

@elhigu

This comment has been minimized.

Collaborator

elhigu commented Jan 22, 2017

and thanks for contribution 👍 !

@tomaspinho

This comment has been minimized.

Contributor

tomaspinho commented Jan 22, 2017

Hey @elhigu! Thanks so much for the feedback. From Google searches, I would say Postgres is the only one supporting it, as other engines don't seem to support table inheritance in the same way (but I may be wrong, not a DBA).

I'll see if I can take a look at your request this coming week and update this PR accordingly.

@tomaspinho

This comment has been minimized.

Contributor

tomaspinho commented Jan 23, 2017

Hey @elhigu,

You may find the new docs here: knex/documentation#16
I've already implemented the options object, but the build seems to have failed in an integration test unrelated to this PR. Could you please make it run again? Thanks!

this._single.table = tableName;
this._single.only = options.only !== undefined ? options.only : false;

This comment has been minimized.

@wubzz

wubzz Jan 24, 2017

Collaborator

Try to prevent negated conditions where possible. Here it would make more sense with explicit checks this._single.only = options.only === true as it would also ensure the property only is always a boolean. :)

This comment has been minimized.

@tomaspinho

tomaspinho Jan 25, 2017

Contributor

taken care of ;) thanks!

@elhigu elhigu merged commit 7de07c9 into tgriesser:master Jan 26, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@elhigu

This comment has been minimized.

Collaborator

elhigu commented Jan 26, 2017

Thanks @tomaspinho

@tomaspinho

This comment has been minimized.

Contributor

tomaspinho commented Jan 27, 2017

Thank you, @elhigu 😄

elhigu added a commit to elhigu/knex that referenced this pull request Feb 15, 2017

ONLY keyword support for PostgreSQL (tgriesser#1874)
* adding support for ONLY keyword for PostgreSQL as an optional flag of from() and into() through table(tableName, only)

* passing only parameter from default function was missing

* changing only option to options object with only key

* better style per @wubzz suggestion
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment