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

fix(HasAttributes) no cast when use expressions #6375

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

BinaryAlan
Copy link
Contributor

When the "ct" attribute of the Foo model is set to cast.

class Foo extends Model
{
    protected array $casts = ['id' => 'integer', 'ct' => 'integer'];
}

In the updateOrCreate method, an Expression expression is used.

Foo::updateOrCreate(['id' => 1], ['ct' => Db::raw('ct + 1')]);

The following warning will be issued

 WARNING  Object of class Hyperf\Database\Query\Expression could not be converted to int in vendor/hyperf/database/src/Model/Concerns/HasAttributes.php on line 883

@BinaryAlan BinaryAlan marked this pull request as ready for review December 11, 2023 11:02
@huangdijia
Copy link
Member

补一个单测?

Comment on lines +891 to +894
if ($value instanceof Expression) {
return $value;
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

测试过么?这么改可以?

Laravel 是这么处理的么?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

测试过了
Laravel也有这个问题,还没解决

@BinaryAlan
Copy link
Contributor Author

补一个单测?

已补

@limingxinleo
Copy link
Member

模型方法不能支持这种写法

后期会有各种问题

Db::raw('ct + 1')

@limingxinleo
Copy link
Member

你可以先给 Laravel 提交代码,如果 Laravel 合并了代码,我们再来讨论是否要合并吧

@BinaryAlan
Copy link
Contributor Author

模型方法不能支持这种写法

后期会有各种问题

Db::raw('ct + 1')

可以举个可能出现问题的例子吗?
模型其实已经事实上支持了Expression的用法,只是我在使用的过程中发现会有警告的抛出,才想着把它处理一下

@limingxinleo
Copy link
Member

现在的情况是模型都不支持这种写法。

一旦这里支持了,就需要所有的地方都支持,像你现在这种写法,还需要强制从数据库里 refresh 到最新结果,是不太合适的。

你可以先提交给 Laravel,我们看下他们是怎么处理的。

@limingxinleo
Copy link
Member

比如,模型事件里,直接拿修改后的数据,会是什么情况?

@BinaryAlan
Copy link
Contributor Author

@limingxinleo 模型的update方法实际使用的是database的Builder,如果修改vendor/hyperf/database/src/Model/Builder.phpupdateOrCreate方法,把model操作改成Builder操作,是否可行?

    public function updateOrCreate(array $attributes, array $values = [])
    {
        return tap($this->firstOrNew($attributes), function ($instance) use ($values) {
            // $instance->fill($values)->save(); 原来的代码
            $instance->newQuery()->update($values);  // 改动后的代码
        });
    }

@limingxinleo
Copy link
Member

尽量别改。我建议你先给 Laravel 提交 PR 比较好。

@BinaryAlan BinaryAlan marked this pull request as draft December 13, 2023 00:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants