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

Use wrapIdentifier in columnInfo. fixes #2402 #2405

Merged
merged 3 commits into from Jan 2, 2018

Conversation

Projects
None yet
2 participants
@koskimas
Collaborator

koskimas commented Jan 2, 2018

Hi,

currently columnInfo doesn't respect the wrapIdentifier function if one is provided in the configuration. This PR fixes that.

fixes #2402

koskimas added some commits Jan 2, 2018

Make postgres client wrapIdentifierImpl independent of `this` context
 * Also fixes a bug where only single digit indices were accepted.
add customWrapIdentifier helper for client
 * customWrapIdentifier is one more level of abstraction for the wrapIdentifier "pipeline"
   that allows the custom wrapIdentifier to be called without calling the `wrapIdentifierImpl`
@elhigu

This comment has been minimized.

Collaborator

elhigu commented Jan 2, 2018

Thanks a lot for PR! I'm fixing the test suite right now... check this one after tests are running again.

return this.customWrapIdentifier(value, this.wrapIdentifierImpl);
},
customWrapIdentifier(value, origImpl) {

This comment has been minimized.

@elhigu

elhigu Jan 2, 2018

Collaborator

Kind of hacky, but works at least as long as user doesn not implement their custom quote style too in their wrapIdentifier configuration. Maybe knex would need this approach to be more official and to have separate APIs to override for quoting and for transforming identifiers. Anyways this is good enough for now.

This comment has been minimized.

@koskimas

koskimas Jan 2, 2018

Collaborator

Yeah, but there is no other way to do this right now.

@elhigu elhigu merged commit 45f5ffb into tgriesser:master Jan 2, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment