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

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

Open
FPEPOSHI opened this issue Aug 15, 2024 · 14 comments
Labels
bug A bug report status: unverified Unverified

Comments

@FPEPOSHI
Copy link

FPEPOSHI commented Aug 15, 2024

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 FPEPOSHI added bug A bug report status: unverified Unverified labels Aug 15, 2024
@noone-silent
Copy link
Contributor

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

@FPEPOSHI
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

noone-silent commented Aug 19, 2024

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
Copy link
Author

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
Copy link
Author

FPEPOSHI commented Aug 19, 2024

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
Copy link
Contributor

noone-silent commented Aug 19, 2024

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Author

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

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

No branches or pull requests

2 participants