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

Models apparently have reserved keywords #11253

Closed
nsossonko opened this issue Dec 21, 2015 · 7 comments
Closed

Models apparently have reserved keywords #11253

nsossonko opened this issue Dec 21, 2015 · 7 comments
Labels
bug A bug report status: medium Medium

Comments

@nsossonko
Copy link
Contributor

The following keywords (when used as column names) were found to cause issues with phalcon's models (although I could not find a list in the docs). IMHO they should not cause issues and should be allowed to be used, but at least it should be documented:

group - causes an exception during PHQL parsing as it sees it as meaning GROUP.
transaction - meddles with transactions used when nesting models.
for - Causes error similar to group's.
source - Confuses phalcon to think we are declaring the table source.

Edit: I should note, this is using mysql backend.

@sergeyklay
Copy link
Contributor

@nsossonko Thanks for reporting. Can you please provide a small script to reproduce? Also please provide Phalcon & MySQL version. We'll try to deal

@nsossonko
Copy link
Contributor Author

Approximation of what we were doing:

class Parents {
  $id;
  public function initialize() {
    $this->hasMany(
      'id',
      'Childs',
      'parent',
      array(
        'alias'  => 'children'
      )
    );
  }
}

class Childs {
  $id;
  $parent;
  $source;
  $transaction;
  $for;
  $group;
  public function initialize() {
    $this->belongsTo(
      'parent',
      'Parents',
      'id',
      array(
        'alias'  => 'myparent'
      )
    );
  }
}

$parent = new Parents();
$child = new Childs();
$child->myparent = $parent;
$child->transaction = 'transaction';
$child->create();
/**
 * Issue with transaction...you'll have to debug because, IIRC,
 * it was a difficult one to figure out
 */

$child = new Childs();
$child->source = 'source';
$child->create(); // Error about no such table

$children = Childs::findByFor(1); // Error
$children = Childs::findByGroup(1); // Error

SQL:

create table `parents` (
  `id` int not null auto_increment,
  primary key (`id`)
);

create table `childs` (
  `id` int not null auto_increment,
  `parent` int default null,
  `source` varchar(20) default null,
  `transaction` varchar(20) default null,
  `for` int default null,
  `group` int default null,
  primary key (`id`)
);

@Jurigag
Copy link
Contributor

Jurigag commented Dec 21, 2015

I guess in columnmap you have to put transaction etc in backticks ?

Its not really a phalcon problem:
http://dev.mysql.com/doc/refman/5.7/en/keywords.html

Other frameworks just automatically escape it, and that would be nice option.

@nsossonko
Copy link
Contributor Author

It is a Phalcon problem, because it fails in Phalcon code, not in PDO or Mysql. Phalcon already escapes everything with backticks so this wouldn't be a problem at all on the Mysql side. It fails in these scenarios as described above.

@Jurigag
Copy link
Contributor

Jurigag commented Dec 22, 2015

So show us logs, and database log, cuz we dont really now whats error it is, beacause all those "model" properites what you provided all actually mysql reserved keywords, so its not phalcon problem, they are just seems no escaping.

@nsossonko
Copy link
Contributor Author

So I've subsequently found this: https://docs.phalconphp.com/en/latest/reference/phql.html#escaping-reserved-words which doesn't detail the actual reserved words for phql, but makes enough sense I guess. That would fix group and for. source and transaction though are still very much an issue. source is very easy to test, just make a model with an attribute named source (as indicated in the code samples) and you will see it will attempt to use the source attribute's value as the table name. transaction is also demoed in the code above but is a bit more involved to test.

sergeyklay added a commit that referenced this issue Mar 13, 2016
Model::find...By...() fields are now escaped properly (fixes #11253).
@sergeyklay
Copy link
Contributor

Fixed in 2.1.x branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug report status: medium Medium
Projects
None yet
Development

No branches or pull requests

4 participants