phalcon / cphalcon

High performance, full-stack PHP framework delivered as a C extension.
https://phalcon.io
BSD 3-Clause "New" or "Revised" License
10.79k stars 1.96k forks source link

[BUG]: Model->save() triggers Insert instead of Update when records is existing one #16636

Open FPEPOSHI opened 3 months ago

FPEPOSHI commented 3 months ago

We're encountering intermittent issues with a specific Model (the only one affected). Occasionally, when attempting to execute ->save() or ->delete(), we encounter database errors such as:

"Unique violation: 7 ERROR: duplicate key value violates unique constraint."

This issue persists from version 3.4 up to 5.8.0 and only occurs on the production server. We haven't been able to reproduce it locally. The only temporary solution on the production server is clearing the Redis cache.

Previously, we encountered another error:

"A primary key must be defined in the model in order to perform the operation."

We resolved this by adding the following annotation to the model's $id:

/**
* @Primary
* @Identity
* @Column(type='integer', nullable=false)
*/
protected $id;

We suspect that the problem might be related to the metadata cache for this model. After reviewing the Zephir code, it seems the issue occurs at:

Model.zep::has(<MetaDataInterface> metaData, <AdapterInterface> connection), where the record is not recognized as existing in the database.

Since we can't reproduce the issue locally, I'm unable to provide code or specific examples. We've also tried clearing the Redis cache (ph-mm-reds-meta-[some namespace lowercase]\standingorderpattern and ph-mm-reds-map-[some namespace lowercase]\standingorderpattern) before querying the database, but the problem persists.

Details

noone-silent commented 3 months ago

Could you please post some example code, if it doesn't contain any secrets. Also would you be able to produce plain SQL?

FPEPOSHI commented 3 months ago

I could but it will not help because there is no special code/sql for this entity, its same as all other 100+ entities we have. And that's the struggle because we don't have special logic defined for this and its not reproducible, it occurs some rarely on production server only.

noone-silent commented 3 months ago

We need at least a little bit of code, to see what you're doing. For example, how do you load the entity, modify it and then update it.

What metaData storage do you use local? Memory? What happens if you change to redis?

FPEPOSHI commented 3 months ago

We user redis, same as in production server.

Sure, here is an example:

Sql:

create table if not exists some_table
(
    id           integer default nextval('some_table_id_seq'::regclass) not null primary key,
    total_amount numeric(19, 6)                                         not null
);

Model:

class Something extends \Phalcon\Mvc\Model {

    /**
     * @Primary
     * @Identity
     * @Column(type='integer', nullable=false)
     * @var int
     */
    protected $id;

    /** @var float */
    protected $totalAmount;

    public function initialize() {
        $this->setSchema('public');
        $this->setSource('some_table');
    }

    /**
     * @return int
     */
    public function getId() {
        return $this->id;
    }

    /**
     * @param int $id
     */
    public function setId(int $id): void {
        $this->id = $id;
    }

    public function setTotalAmount(float $totalAmount): void {
        $this->totalAmount = $totalAmount;
    }

    public function getTotalAmount() {
        return $this->totalAmount;
    }

    /**
     * Column map
     * @return array
     */
    public function columnMap() {
        return [
            'id' => 'id',
            'total_amount' => 'totalAmount',
        ];
    }
}

Usage:


$something = Something::findFirst(1);
$something->setTotalAmount(300);
$something->save(); // at this moment the: 'Unique violation: 7 ERROR: duplicate key value violates unique constraint' is thrown.
noone-silent commented 3 months ago

You have a mix of Annotation Parser and manual. Maybe this creates some Problems.

What is you metadata strategy?

Please see this documentation: https://docs.phalcon.io/latest/db-models-metadata/#manual

My guess is, that you're missing the getMetaData Method and there this part:

 MetaData::MODELS_IDENTITY_COLUMN => 'id'

Probably that's the error.

So basically stick to one metadata strategy annotations or manual.

FPEPOSHI commented 3 months ago

If I don't define $id like this:

/**
* @Primary
* @Identity
* @Column(type='integer', nullable=false)
* @var int
*/
protected $id;

another error will occur: A primary key must be defined in the model in order to perform the operation.

FPEPOSHI commented 3 months ago

We don't use custom metadata strategy. As I said at the beginning, out of +100 models we have, only one has this issue.

noone-silent commented 3 months ago

If you did not set any strategy, then the default is the database introspection, see here:

https://docs.phalcon.io/latest/db-models-metadata/#strategies

The default strategy to obtain the model's metadata is database introspection. Using this strategy, the information schema is used to identify the fields in a table, its primary key, nullable fields, data types, etc.

That means, the id column is not correctly detected as primary key (somehow). I'm not a postgres expert, but using the database introspection is maybe faulty.

Could you try to set the metadata strategy to annotations and try again?

FPEPOSHI commented 3 months ago

Now I got it, we use this strategy:

$serializerFactory = new SerializerFactory();

  $adapterFactory    = new \Phalcon\Cache\AdapterFactory($serializerFactory);
  $options = [
      'defaultSerializer' => 'Igbinary',
      'host' => $this->config->redis->host,
      'port' => $this->config->redis->port,
      'auth' => $this->config->redis->authKey
  ];

  $metaData = new \Phalcon\Mvc\Model\MetaData\Redis($adapterFactory, $options);

  return $metaData;
noone-silent commented 3 months ago

Now I got it, we use this strategy:

$serializerFactory = new SerializerFactory();

$adapterFactory    = new \Phalcon\Cache\AdapterFactory($serializerFactory);
$options = [
    'defaultSerializer' => 'Igbinary',
    'host' => $this->config->redis->host,
    'port' => $this->config->redis->port,
    'auth' => $this->config->redis->authKey
];

$metaData = new \Phalcon\Mvc\Model\MetaData\Redis($adapterFactory, $options);

return $metaData;

Try with this:

        $metadata->setStrategy(new \Phalcon\Model\MetaData\Strategy\Annotations());

Also add this to your Something:

        /**
         * @Primary
         * @Identity
     * @Column(type='integer', nullable=false, column="id")
     * @var int
     */
        protected $id;
    /** 
         * @Column(type="float", nullable=true, column="total_amount")
         * @var float
         */
    protected $totalAmount;
FPEPOSHI commented 3 months ago

But we don't want this, otherwise we would have to refactor all our models to this approach which we don't want to.

noone-silent commented 3 months ago

Ok, I THINK: The error is, that through the database introspection, Phalcon can't figure out, that your id field is the identity and primary key of the database.

For example take this:

id           integer default nextval('some_table_id_seq'::regclass) not null primary key

and also postgres could do this

id SERIAL PRIMARY KEY
// OR
id INTEGER PRIMARY KEY GENERATED ALWAYS AS IDENTITY

As I said earlier, I'm not an export in postgres.

FPEPOSHI commented 3 months ago

It didn't help, the column definition is as described:

For example take this:

id           integer default nextval('some_table_id_seq'::regclass) not null primary key
FPEPOSHI commented 2 months ago

@noone-silent any update on this?

We are still getting: Exception: Source related to this model does not have a primary key defined (in phalcon/Mvc/Model/Query/Builder.zep on line 792) and it works only when we clear redis cache and its happening only for one specific Model, the one we discussed above.