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

Calling destroy() with criteria on a model without a PK defined generates an error on MySQL #23

Open
dcaravana opened this issue Jan 28, 2015 · 9 comments · May be fixed by #57
Open

Calling destroy() with criteria on a model without a PK defined generates an error on MySQL #23

dcaravana opened this issue Jan 28, 2015 · 9 comments · May be fixed by #57
Labels
bug

Comments

@dcaravana
Copy link

Using sails@0.10.5 and sails-mysql@0.10.11.

This example:

Timeseries.destroy({ response_id: response_id, metric_id: metric_id }).exec(...

generates the following error:

Error: ER_BAD_FIELD_ERROR: Unknown column 'timeseries.undefined' in 'order clause'

The bug is in WhereBuilder.prototype.single and it's very easy to spot

...
  if(!hop(queryObject, 'sort')) {
    var childPK;

    _.keys(this.schema[this.currentTable].attributes).forEach(function(attr) {
      var expandedAttr = self.schema[self.currentTable].attributes[attr];
      if(!hop(expandedAttr, 'primaryKey')) return;
      childPK = expandedAttr.columnName || attr;
    });

    queryObject.sort = {};
    queryObject.sort[childPK] = -1;
  }
...

last two code lines become:

    if(childPK) {
      queryObject.sort = {};
      queryObject.sort[childPK] = -1;
    }

The workaround of adding explicit sorting to criteria doesn't work either, but helps debug since the syntax of the generated SQL statement is not accepted by MySQL:

DELETE `timeseries` 
FROM `TimeSeriesData` AS `timeseries`
WHERE `timeseries`.`response_id` = "2" AND `timeseries`.`metric_id` = "1"
ORDER BY `timeseries`.`response_id` ASC;

instead it should be:

DELETE FROM `TimeSeriesData`
WHERE `TimeSeriesData`.`response_id` = "2" AND `TimeSeriesData`.`metric_id` = "1"
ORDER BY `TimeSeriesData`.`response_id` ASC;

Resorting to a SQL statement through query() for now.
@CWyrtzen
Copy link

@dcaravana was this ever resolved?

@vkopitsa
Copy link

+1 sails@0.11.0 and sails-mysql@0.10.12

@devinivy devinivy linked a pull request Jul 14, 2015 that will close this issue
Open
@devinivy
Copy link
Contributor

PR posted: #57.

@yisyang
Copy link

yisyang commented Sep 13, 2015

👍 @devinivy
Until that is merged, defining an attribute with primaryKey: false will work as a temporary fix.

@devinivy devinivy added the bug label Sep 13, 2015
@sailsbot
Copy link
Collaborator

Thanks for posting, @dcaravana. I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

  • review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
  • create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

@devinivy
Copy link
Contributor

Reopening until #57 is merged.

@devinivy devinivy reopened this Oct 14, 2015
@dcaravana
Copy link
Author

Fix not verified (and still on sails 0.10.5), let me know if we should.

Thanks everyone.

@sailsbot
Copy link
Collaborator

Thanks for posting, @dcaravana. I'm a repo bot-- nice to meet you!

It has been 30 days since there have been any updates or new comments on this page. If this issue has been resolved, feel free to disregard the rest of this message. On the other hand, if you are still waiting on a patch, please:

  • review our contribution guide to make sure this submission meets our criteria (only verified bugs with documented features, please; no questions, commentary, or bug reports about undocumented features or unofficial plugins)
  • create a new issue with the latest information, including updated version details with error messages, failing tests, and a link back to the original issue. This allows GitHub to automatically create a back-reference for future visitors arriving from search engines.

Thanks so much for your help!

@devinivy
Copy link
Contributor

Bug with PR pending– reopening.

@devinivy devinivy reopened this Nov 14, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Do not enforce PK sort when table has no PK. balderdashy/waterline-sequel
6 participants
@dcaravana @vkopitsa @devinivy @yisyang @CWyrtzen @sailsbot