Skip to content
This repository has been archived by the owner on Aug 13, 2021. It is now read-only.

broken atribute columnName #88

Open
masitko opened this issue Apr 10, 2016 · 11 comments
Open

broken atribute columnName #88

masitko opened this issue Apr 10, 2016 · 11 comments
Labels

Comments

@masitko
Copy link

masitko commented Apr 10, 2016

After updating to version 0.6.2 (from 0.5.x, using sails-postgresql adapter) there is an error when lifting sails:

error: error: A hook (`orm`) failed to load!
error: error: Error (E_UNKNOWN) :: Encountered an unexpected error
error: column author.createdUser does not exist
    at Connection.parseE (/var/www/html/ccentre/backend/node_modules/sails-postgresql/node_modules/pg/lib/connection.js:539:11)
    at Connection.parseMessage (/var/www/html/ccentre/backend/node_modules/sails-postgresql/node_modules/pg/lib/connection.js:366:17)
    at Socket.<anonymous> (/var/www/html/ccentre/backend/node_modules/sails-postgresql/node_modules/pg/lib/connection.js:105:22)
    at emitOne (events.js:90:13)
    at Socket.emit (events.js:182:7)
    at readableAddChunk (_stream_readable.js:147:16)
    at Socket.Readable.push (_stream_readable.js:111:10)
    at TCP.onread (net.js:523:20)

the error seems to be caused by problem in select.js around line 90:

attributes.forEach(function(key) {
    // Default schema to {} in case a raw DB column name is sent.  This shouldn't happen
    // after https://github.com/balderdashy/waterline/commit/687c869ad54f499018ab0b038d3de4435c96d1dd
    // but leaving here as a failsafe.
    var schema = self.schema[self.currentTable].definition[key] || {};
//    console.log(schema);
    if(!schema) return;
    if(hop(schema, 'collection')) return;
    selectKeys.push({ table: self.currentTable, key: schema.columnName || key });
  });
@sailsbot
Copy link
Collaborator

@masitko Thanks for posting, we'll take a look as soon as possible. In the meantime, if you haven’t already, please carefully read the issue contribution guidelines and double-check for any missing information above. In particular, please ensure that this issue is about a stability or performance bug with a documented feature; and make sure you’ve included detailed instructions on how to reproduce the bug from a clean install. Finally, don’t forget to include the version of Node.js you tested with, as well as your version of Sails or Waterline, and of any relevant standalone adapters/generators/hooks.

Thank you!

@masitko
Copy link
Author

masitko commented Apr 10, 2016

To fix the error line 92 of file https://github.com/balderdashy/waterline-sequel/blob/master/sequel/select.js needs to be changed from

    var schema = self.schema[self.currentTable].definition[key] || {};

to

    var schema = self.schema[self.currentTable].attributes[key] || {};

@masitko
Copy link
Author

masitko commented Apr 10, 2016

Pull request:
9b6d199

@particlebanana
Copy link
Contributor

@masitko it looks like it's actually a combination of a few things. I submitted 2 PR's that should solve this problem I'd like you to test before I go ahead and publish them.

First was an issue in Waterline where the auto-migrations were not normalizing the select values to use column names when lifting. The other issue was my completely horrible code in this repo.

Both of those PR's are merged so let me know what you see. I ran through some manual tests and they pass all the adapter tests.

@masitko
Copy link
Author

masitko commented Apr 12, 2016

@particlebanana it wasn't fixed I'm afraid. Still

error: error: A hook (`orm`) failed to load!
error: error: Error (E_UNKNOWN) :: Encountered an unexpected error
error: column userlogin.user does not exist
    at Connection.parseE (/var/www/html/ccentre/backend/node_modules/pg/lib/connection.js:539:11)
    at Connection.parseMessage (/var/www/html/ccentre/backend/node_modules/pg/lib/connection.js:366:17)
    at Socket.<anonymous> (/var/www/html/ccentre/backend/node_modules/pg/lib/connection.js:105:22)
    at emitOne (events.js:90:13)
    at Socket.emit (events.js:182:7)
    at readableAddChunk (_stream_readable.js:147:16)
    at Socket.Readable.push (_stream_readable.js:111:10)
    at TCP.onread (net.js:523:20)

when trying to lift Sails.

Please have a look at file select.js around line 90:

 var attributes = queryObject.select || Object.keys(this.schema[this.currentTable].definition);
  delete queryObject.select;

attributes.forEach(function(key) {
    // Default schema to {} in case a raw DB column name is sent.  This shouldn't happen
    // after https://github.com/balderdashy/waterline/commit/687c869ad54f499018ab0b038d3de4435c96d1dd
    // but leaving here as a failsafe.
    var schema = self.schema[self.currentTable].definition[key] || {};
    if(!schema) return;
    if(hop(schema, 'collection')) return;
    selectKeys.push({ table: self.currentTable, key: schema.columnName || key });
  });

and what self.schema[self.currentTable] contains:

{ attributes: 
   { ip: { type: 'string', required: true },
     host: { type: 'string', required: true },
     agent: { type: 'text', required: true },
     browser: { type: 'string', defaultsTo: 'Unknown' },
     browserVersion: { type: 'string', defaultsTo: 'Unknown' },
     browserFamily: { type: 'string', defaultsTo: 'Unknown' },
     os: { type: 'string', defaultsTo: 'Unknown' },
     osVersion: { type: 'string', defaultsTo: 'Unknown' },
     osFamily: { type: 'string', defaultsTo: 'Unknown' },
     count: { type: 'integer', defaultsTo: 1 },
     id: 
      { type: 'integer',
        autoIncrement: true,
        primaryKey: true,
        unique: true },
     createdAt: { type: 'datetime', default: 'NOW' },
     updatedAt: { type: 'datetime', default: 'NOW' },
     user: 
      { columnName: 'userId',
        type: 'integer',
        foreignKey: true,
        references: 'user',
        on: 'id',
        onKey: 'id' } },
  definition: 
   { ip: { type: 'string' },
     host: { type: 'string' },
     agent: { type: 'text' },
     browser: { type: 'string', defaultsTo: 'Unknown' },
     browserVersion: { type: 'string', defaultsTo: 'Unknown' },
     browserFamily: { type: 'string', defaultsTo: 'Unknown' },
     os: { type: 'string', defaultsTo: 'Unknown' },
     osVersion: { type: 'string', defaultsTo: 'Unknown' },
     osFamily: { type: 'string', defaultsTo: 'Unknown' },
     count: { type: 'integer', defaultsTo: 1 },
     id: 
      { type: 'integer',
        autoIncrement: true,
        primaryKey: true,
        unique: true },
     createdAt: { type: 'datetime' },
     updatedAt: { type: 'datetime' },
     userId: 
      { type: 'integer',
        model: 'user',
        foreignKey: true,
        alias: 'user' } },
  meta: { junctionTable: false },
  tableName: 'userlogin',
  connection: [ 'postgresql' ] }

As we can see definition key for column user is already updated to userId.
Code mentioned above iterates through schema keys and extracts values from definition using the same (wrong in this case) keys

var schema = self.schema[self.currentTable].definition[key] || {};

When columnName is set schema = {}

@masitko
Copy link
Author

masitko commented Apr 12, 2016

@particlebanana it does lift after updating waterline to the latest master.
There is lots of errors during migration but it works with a clean database.
Will have a look at it tomorrow.

@masitko
Copy link
Author

masitko commented Apr 12, 2016

@particlebanana it lifted once with clean database but after loading fixtures I got some validation errors:

error: error: A hook (`orm`) failed to load!
error: error: Error (E_VALIDATION) :: 1 attribute is invalid
    at WLValidationError.WLError (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/error/WLError.js:25:15)
    at new WLValidationError (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/error/WLValidationError.js:19:28)
    at _afterValidating (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/query/validate.js:53:23)
    at allValidationsChecked (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/core/validations.js:288:12)
    at /var/www/html/ccentre/backend/node_modules/async/lib/async.js:52:16
    at Object.async.forEachOf.async.eachOf (/var/www/html/ccentre/backend/node_modules/async/lib/async.js:236:30)
    at Object.async.forEach.async.each (/var/www/html/ccentre/backend/node_modules/async/lib/async.js:209:22)
    at Validator.validate (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/core/validations.js:167:9)
    at /var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/query/validate.js:42:25
    at /var/www/html/ccentre/backend/node_modules/async/lib/async.js:718:13
    at Immediate.iterate [as _onImmediate] (/var/www/html/ccentre/backend/node_modules/async/lib/async.js:262:13)
    at processImmediate [as _immediateCallback] (timers.js:383:17)

Invalid attributes sent to Passport:
 • protocol
   • `undefined` should be a alphanumeric (instead of "null", which is a object)
   • "required" validation rule failed for input: null
Specifically, it threw an error.  Details:
 undefined

This seems to be an old problem, I'm not sure if this i's related to my models definition/fixtures or changes in waterline 0.11.x -> 0.12.x latest master.

@particlebanana
Copy link
Contributor

var schema = self.schema[self.currentTable].definition[key] || {};

That is the expected behavior. By the time it gets to there the select statement should contain the normalized column name values. The patch in Waterline fixes this where the auto migrations were not running the same code as normal .find/.create. The last validation issue you posted may have something to do with this, now the migrations run through the .create(). I'm wondering if this is the correct behavior or not.

@masitko
Copy link
Author

masitko commented Apr 12, 2016

I'm not sure if this works correctly, it may be related to my broken/out of date dependencies but:

I can lift sails only first time with clean database and fixtures are being loaded OK.
There is lots of errors

error: error: Must specify an `id` when calling `Model.room(id)`

which I never seen before and relations between models seem to be broken.

After that every attempt to lift ends with an error and waterline tries to migrate my database despite I didn't change anything:

Waterline encountered a fatal error when trying to perform the `alter` auto-migration strategy.
In a couple of seconds, the data (cached in memory) will be logged to stdout.
(a failsafe put in place to preserve development data)

In the mean time, here's the error:

Error (E_VALIDATION) :: 2 attributes are invalid
    at WLValidationError.WLError (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/error/WLError.js:25:15)
    at new WLValidationError (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/error/WLValidationError.js:19:28)
    at _afterValidating (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/query/validate.js:53:23)
    at allValidationsChecked (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/core/validations.js:288:12)
    at /var/www/html/ccentre/backend/node_modules/async/lib/async.js:52:16
    at Object.async.forEachOf.async.eachOf (/var/www/html/ccentre/backend/node_modules/async/lib/async.js:236:30)
    at Object.async.forEach.async.each (/var/www/html/ccentre/backend/node_modules/async/lib/async.js:209:22)
    at Validator.validate (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/core/validations.js:167:9)
    at /var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/query/validate.js:42:25
    at /var/www/html/ccentre/backend/node_modules/async/lib/async.js:718:13
    at Immediate.iterate [as _onImmediate] (/var/www/html/ccentre/backend/node_modules/async/lib/async.js:262:13)
    at processImmediate [as _immediateCallback] (timers.js:383:17)

Invalid attributes sent to Book:
 • title
   • `undefined` should be a string (instead of "null", which is a object)
   • "required" validation rule failed for input: null
Specifically, it threw an error.  Details:
 undefined
 • releaseDate
   • `undefined` should be a date (instead of "null", which is a object)
   • "required" validation rule failed for input: null
Specifically, it threw an error.  Details:
 undefined

================================
Data backup:
================================

[ { id: 1 },
  { id: 2 },
  { id: 3 },
  { id: 4 },
  { id: 5 },
  { id: 6 },
  { id: 7 },
  { id: 8 },
  { id: 9 },
  { id: 10 },
  { id: 11 },
  { id: 12 },
  { id: 13 },
  { id: 14 },
  { id: 15 },
  { id: 17 },
  { id: 16 },
  { id: 26 },
  { id: 36 },
  { id: 46 },
  { id: 18 },
  { id: 28 },
  { id: 38 },
  { id: 48 },
  { id: 19 },
  { id: 29 },
  { id: 42 },
  { id: 20 },
  { id: 30 },
  { id: 43 },
  { id: 21 },
  { id: 31 },
  { id: 39 },
  { id: 22 },
  { id: 32 },
  { id: 40 },
  { id: 23 },
  { id: 33 },
  { id: 41 },
  { id: 24 },
  { id: 34 },
  { id: 44 },
  { id: 25 },
  { id: 35 },
  { id: 45 },
  { id: 27 },
  { id: 37 },
  { id: 47 } ]
error: error: A hook (`orm`) failed to load!
error: Error (E_VALIDATION) :: 2 attributes are invalid
    at WLValidationError.WLError (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/error/WLError.js:25:15)
    at new WLValidationError (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/error/WLValidationError.js:19:28)
    at _afterValidating (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/query/validate.js:53:23)
    at allValidationsChecked (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/core/validations.js:288:12)
    at /var/www/html/ccentre/backend/node_modules/async/lib/async.js:52:16
    at Object.async.forEachOf.async.eachOf (/var/www/html/ccentre/backend/node_modules/async/lib/async.js:236:30)
    at Object.async.forEach.async.each (/var/www/html/ccentre/backend/node_modules/async/lib/async.js:209:22)
    at Validator.validate (/var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/core/validations.js:167:9)
    at /var/www/html/ccentre/backend/node_modules/sails-hook-orm/node_modules/waterline/lib/waterline/query/validate.js:42:25
    at /var/www/html/ccentre/backend/node_modules/async/lib/async.js:718:13
    at Immediate.iterate [as _onImmediate] (/var/www/html/ccentre/backend/node_modules/async/lib/async.js:262:13)
    at processImmediate [as _immediateCallback] (timers.js:383:17)

Let me know if I can help in any way.

@mr47
Copy link

mr47 commented Apr 18, 2016

@masitko @particlebanana Guys after hours of debug, i found that magic wrong goes here with 99% not in waterline-shema or waterline itself.
First of all queryObject.select always undefined, but it seems its should pass to queryObject.select. So when using columnName it don't selects field and when comes time to processChildren we gen someting like this

^^^^^^^^^^^^^^^^^^^^MySQL.processChildren.groupedRecords^^^^^^^^^^^^^^
{ undefined:
   [ RowDataPacket {
       id: 1,
       comment: '2',
       createdAt: Mon Apr 18 2016 23:41:20 GMT+0300 (FLE Daylight Time),
       updatedAt: Mon Apr 18 2016 23:41:20 GMT+0300 (FLE Daylight Time) } ] }
^^^^^^^^^^^^^^^^^^^^groupedRecords^^^^^^^^^^^^^^

So we see that it should get an field place_id but when build sql its don't select one more field that used to map result to an collection.
And more raw logs with some internal names:

MySQL.populateBuffers:
  SELECT `places`.`id`, `places`.`rawData`, `places`.`createdAt`, `places`.`updatedAt` FROM `places` AS `places`   LIMIT 30 OFFSET 0

MySQL.processChildren:
 (SELECT `placecomments`.`id`,`placecomments`.`comment`,`placecomments`.`createdAt`,`placecomments`.`updatedAt` FROM `placecomments` AS `placecomments` WHERE `place_id` = 148864955158827  ORDER BY `placecomments`.`id` ASC LIMIT 30) UNION ALL (SELECT `placecomments`.`id`,`placecomments`.`comment`,`placecomments`.`createdAt`,`placecomments`.`updatedAt` FROM `placecomments` AS `placecomments` WHERE `place_id` = 148864955158828  ORDER BY `placecomments`.`id` ASC LIMIT 30)

MySQL.processChildren.instructions: 

{ 
  parent: 'places',
  parentKey: 'id',
  child: 'placecomments',
  childKey: 'place_id',
  select: [ 'id', 'comment', 'createdAt', 'updatedAt', 'place_id' ],
  alias: 'comments',
  removeParentKey: false,
  model: false,
  collection: true,
  criteria: { where: {}, limit: 30, sort: { id: 1 } } 
}
^^^^^^^^^^^^^^^^^^^^MySQL.processChildren.result^^^^^^^^^^^^^^

I dont know if it helps but additional info always need.
PS: for this solution is don't using a columnNames, and it should works if get error do few restarts and it will remove all data from tables. To fix problem with migrate: "alter" refer here

@atiertant
Copy link

i don't think the real problem is in sequel, i think adapter should not receive other thing than columnName by waterline...the bug should be report in waterline repo because all adapter should have the same problem if they don't play with attributes ...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

5 participants