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

MSSQL columnInfo() had hard coded schema name #1585

Merged
merged 2 commits into from Jul 26, 2016
Merged

Conversation

@nickwhiteley
Copy link
Contributor

@nickwhiteley 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
Copy link
Member

@elhigu 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
Copy link
Member

@elhigu elhigu commented Jul 21, 2016

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

@nickwhiteley
Copy link
Contributor Author

@nickwhiteley 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
Copy link
Member

@wubzz 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
Copy link
Contributor Author

@nickwhiteley 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 knex:master Jul 26, 2016
1 check passed
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
* 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
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants