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

MSSQL columnInfo() had hard coded schema name #1585

Merged
merged 2 commits into from Jul 26, 2016

Conversation

Projects
None yet
3 participants
@nickwhiteley
Contributor

nickwhiteley commented Jul 20, 2016

columnInfo() for MSSQL has the default schema (dbo) hard coded. It should take the database and schema as parameters like the Postgres version does.

@elhigu

This comment has been minimized.

Collaborator

elhigu commented Jul 21, 2016

/home/travis/build/tgriesser/knex/src/dialects/mssql/query/compiler.js
162:9 error 'bindings' is never modified, use 'const' instead prefer-const
✖ 1 problem (1 error, 0 warnings)

@elhigu

This comment has been minimized.

Collaborator

elhigu commented Jul 21, 2016

Not knowing the dialect, changes looks fine I suppose :)

@nickwhiteley

This comment has been minimized.

Contributor

nickwhiteley commented Jul 25, 2016

The eslint error looks incorrect as the variable 'bindings' can be modified under the if condition. Does anyone know if this can be forced through or is it "better" to use const/var to get eslint to pass?

@wubzz

This comment has been minimized.

Collaborator

wubzz commented Jul 25, 2016

@nickwhiteley It's odd, I know. Short answer is change it to const.

Longer answer: An Object of any kind when defined as a constant simply means it cannot be re-assigned to anything else. It can still have properties assigned to itself, but cannot be re-assigned. For instance:

const bindings = [1,2,3];
bindings.push(4); //Ok, bindings [1,2,3,4]
bindings = 'String'; //Error thrown.

const bindings = {col1: 'val1'};
bindings.col2 = 'val2'; //Ok, bindings = {col1: 'val1', col2: 'val2'}
bindings = 'String'; //Error thrown.

In this case the bindings is originally assigned as an Array, and is not re-assigned later on, this is why it should be constant instead. ESLint picks up on this.

@nickwhiteley

This comment has been minimized.

Contributor

nickwhiteley commented Jul 25, 2016

I'll change it.
It's counter intuitive (to me) so thanks for the explanation. Always good to learn.

@wubzz wubzz merged commit f969d52 into tgriesser:master Jul 26, 2016

1 check passed

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

tgriesser added a commit that referenced this pull request Aug 9, 2016

Merge branch 'master' into gh-pages
* master:
  release 0.11.10
  Prep for release
  Add CHANGELOG.md (#1615)
  Added padding to avoid header sticking to the example usage.
  Updated docs to note schema builder is getter.
  Remove unused files
  Uint8Array data (e.g. Postgres "bytea" type) erroneously considered "undefined" by recent 0.11.7 update. (#1601)
  Revert "[docs] document multi-column order-by"
  [docs] document multi-column order-by
  MSSQL columnInfo() had hard coded schema name (#1585)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment