cphalcon icon indicating copy to clipboard operation
cphalcon copied to clipboard

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

Open FPEPOSHI opened this issue 1 year ago • 14 comments

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

  • Phalcon version: 5.8.0
  • PHP Version: 8.2.20
  • Operating System: Linux
  • Installation type: installing via package manager
  • Zephir version (if any):
  • Server: Apache
  • Other related info (Database, table schema): postgres db, redis for cache

FPEPOSHI avatar Aug 15 '24 09:08 FPEPOSHI

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

noone-silent avatar Aug 19 '24 02:08 noone-silent

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.

FPEPOSHI avatar Aug 19 '24 06:08 FPEPOSHI

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?

noone-silent avatar Aug 19 '24 08:08 noone-silent

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.

FPEPOSHI avatar Aug 19 '24 08:08 FPEPOSHI

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.

noone-silent avatar Aug 19 '24 08:08 noone-silent

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 avatar Aug 19 '24 09:08 FPEPOSHI

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

FPEPOSHI avatar Aug 19 '24 09:08 FPEPOSHI

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?

noone-silent avatar Aug 19 '24 09:08 noone-silent

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;

FPEPOSHI avatar Aug 19 '24 09:08 FPEPOSHI

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;

noone-silent avatar Aug 19 '24 09:08 noone-silent

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

FPEPOSHI avatar Aug 19 '24 09:08 FPEPOSHI

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.

noone-silent avatar Aug 20 '24 05:08 noone-silent

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 avatar Aug 20 '24 13:08 FPEPOSHI

@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.

FPEPOSHI avatar Sep 12 '24 10:09 FPEPOSHI