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 - Update - Postgres NULL value is not allowed on Phalcon 4.0.4 #14862

Closed
Gamblt opened this issue Feb 20, 2020 · 6 comments · Fixed by #14876
Closed

[BUG]: Model - Update - Postgres NULL value is not allowed on Phalcon 4.0.4 #14862

Gamblt opened this issue Feb 20, 2020 · 6 comments · Fixed by #14876
Assignees
Labels
bug A bug report status: medium Medium status: unverified Unverified

Comments

@Gamblt
Copy link

Gamblt commented Feb 20, 2020

When I try to save a NULL value on postgresql, it does not save;

$model->getMessages() "return field1,field2,field3.... is required

I've get model from database and try to save it but got required errors on NULL values.

It's similar to issue #14722

@Gamblt Gamblt added bug A bug report status: unverified Unverified labels Feb 20, 2020
@renatogabrielbr
Copy link
Contributor

I will try to help you, but you can tell me your postgres, phalcon and php version, I'm not having this issue. and if possible some code example and the table.

Thanks

@Jeckerson
Copy link
Member

@Gamblt Can you give more specific example with code?

@TimurFlush
Copy link

TimurFlush commented Feb 23, 2020

I confirm on Postgresql 12

PHP: 7.4
Phalcon (build from source):

Version => 4.0.4
Build Date => Feb 16 2020 16:40:11
Powered by Zephir => Version 0.12.16-$Id$

Model:

class User extends \Phalcon\Mvc\Model
{
    public ?int $id = null;
    public ?string $login = null;
    public ?string $password = null;
    public ?string $updated_at = null;
    
    public function initialize()
    {
         $this->setSource('users');
         $this->useDynamicUpdate(true);
    }
}

Schema

CREATE TABLE "users" (
    "id" SERIAL PRIMARY KEY,
    "login" CHARACTER VARYING(255) NOT NULL,
    "password" CHARACTER VARYING(255) NULL,
    "updated_at" timestamp NULL
);

To reproduce:

$user = new User();
$user->login = 'kek';
$user->save();
foreach ($user->getMessages() as $message) {
    echo $message->getMessage(); // password is required, updated_at is required
}

@Gamblt
Copy link
Author

Gamblt commented Feb 24, 2020

Yes. It's on Phalcon 4.0.4 from binary, PHP7.4 and Postgresql12.1

@maxc62
Copy link

maxc62 commented Feb 24, 2020

The problem is in the Adapter/Pdo/Postgresql class.
On line 490 we have

if field [5] == "NO" {
         let definition["notNull"] = true;
}

and that is correct
Then it feeds the "Db/Column" class but by default notNull is already at "true" (since commit of February 03: 4cb5887)
line 258: protected notNull = true;
If we replace Adapter/Pdo/Postgresql with

if field [5] == "YES" {
         let definition["notNull"] = false;
}

that fixes the problem

The fix already exists for Mysql: 8038065
Same for Sqlite 31a6fd0

@Jeckerson Jeckerson self-assigned this Feb 25, 2020
@Jeckerson Jeckerson linked a pull request Feb 25, 2020 that will close this issue
5 tasks
Jeckerson added a commit that referenced this issue Feb 25, 2020
Jeckerson added a commit that referenced this issue Feb 25, 2020
Jeckerson added a commit that referenced this issue Feb 25, 2020
Jeckerson added a commit that referenced this issue Feb 25, 2020
Jeckerson added a commit that referenced this issue Feb 26, 2020
Jeckerson added a commit that referenced this issue Feb 26, 2020
# Conflicts:
#	tests/_data/assets/schemas/pgsql.sql
#	tests/_data/fixtures/Migrations/OrdersProductsMigration.php
#	tests/_envs/pgsql.yml
Jeckerson added a commit that referenced this issue Feb 26, 2020
Jeckerson added a commit that referenced this issue Feb 27, 2020
#14862 - Swap condition if column allows null values in PostgreSQL
@Jeckerson
Copy link
Member

Fixed in #14876

@niden niden moved this to Released in Phalcon v5 Aug 25, 2022
@niden niden added this to Phalcon v5 Aug 25, 2022
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 status: unverified Unverified
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants