Closed matthewjumpsoffbuildings closed 9 months ago
Theres some issues with the types of some of the methods coming up in the unit tests, hopefully if the general ideas of this PR work, you dont mind sorting those, I put some ugly forced type docs in to get the phpstan stage passing, all of it needs a cleanup
I was looking through the underlying implementation and it looks like it would be easier if you implement a Json
class which implements ValueInterface and pass that as binging.
Coversion takes place here.
Right so perhaps I could create a custom Cast class that uses the ValueInterface? That would be much simpler, this was a very hacky attempt
Though I notice this in the comments:
* Note: This interface may become internal in the next major version change
Which may be concerning?
In that case I think it's best to throw a PR to them to add a JSON type so it's handled internally. Seems like they already have one for PgJsonb.
Just checking, how do I use these google type interfaces anyway? Am I on the right track to assume its via Casts?
Also I need JSON working yesterday haha, I am not sure how long a PR will take with google
Are you familiar with the ValueMapper class? Apparently this is a thing:
$jsonValue = new PgJsonb('{"key": "value"}');
$valueMapper = new ValueMapper($db, true);
$mappedValue = $valueMapper->encode($jsonValue);
$result = $db->execute('INSERT INTO MyTable (JsonColumn) VALUES (@jsonValue)', [
'parameters' => [
'jsonValue' => $mappedValue
]
]);
I feel like somewhere in the internals of this library, this ValueMapping should be done?
ValueMapper is not accessible from our end.
I've already attempted to make it accessible (https://github.com/googleapis/google-cloud-php/pull/5700).
This was their response.
ValueMapperInterface isn't very easy to implement for a developer as-is. It was meant to be an internal class and not exposed externally. Making it public will mean we will have to support it for back-compat (Google's other language libraries also don't seem to expose it). It also confuses users with a similar name class Spanner/src/ValueInterface.php.
Just checking, how do I use these google type interfaces anyway? Am I on the right track to assume its via Casts?
I had to do this for Numeric types. You will have to convert to a ValueInterface when inserting/updating by overriding Model::getDirtryForUpdate and Model::getAttributesForInsert.
Holy moly i figured it out and its even easier - no changes required to your code at all.
If I make a custom cast like so:
namespace App\Casts;
use Illuminate\Contracts\Database\Eloquent\CastsAttributes;
use Google\Cloud\Spanner\PgJsonb;
use Google\Cloud\Spanner\V1\TypeAnnotationCode;
class SpannerJsonCast implements CastsAttributes
{
public function get($model, $key, $value, $attributes)
{
return json_decode($attributes[$key], true);
}
public function set($model, $key, $value, $attributes)
{
return [$key => new SpannerJsonValue($value)];
}
}
// Just copy the PgJsonb ValueInterface class
class SpannerJsonValue extends PgJsonb
{
// override the PgJsonb classes typeAnnotation TypeAnnotationCode::PG_JSONB
public function typeAnnotation()
{
return TypeAnnotationCode::TYPE_ANNOTATION_CODE_UNSPECIFIED;
}
}
Then in my model I use that cast like so:
protected $casts = [
'ColumnName' => SpannerJsonCast::class,
];
Then it just works! This should be added to the docs, it solves any possible type issues on insert/update, not just JSON
You said
I had to do this for Numeric types.
I am noticing that if I have an INT64 column, then try to update/insert to it via an Eloquent model, even with a cast to integer, it throws an error from spanner saying it cant write strings to int columns... I tried making another custom cast class and that solved it, I am wondering where the implementation for your Numeric type solution is? Or has it not been released?
it throws an error from spanner saying it cant write strings to int columns
Doesn't happen to me.
If you pass a form input, which is a string, this will happen.
You have to cast it to an int first like $model->number = (int) $value
.
I am wondering where the implementation for your Numeric type solution is? Or has it not been released?
This is not released to public because it's not a very clean solution.
Doesn't happen to me. If you pass a form input, which is a string, this will happen. You have to cast it to an int first like
$model->number = (int) $value
.
I am using an Eloquent model, with $casts = [ 'column' => 'int']
, and the form is actually a Laravel Nova form, using a Number field. None of this works, always seem to get the 'cannot insert string into int64' error from spanner. The only thing that did work was making a custom cast class that uses an extension of the Numeric value type
Then override Model::getDirtryForUpdate and Model::getAttributesForInsert and cast it to an int?
Also, please provide as much context as possible so we don't have to go back and forth.
Custom type casting will be improved once google implements https://github.com/googleapis/google-cloud-php/issues/5838. Closing this for now.
This is a first attempt to resolve https://github.com/colopl/laravel-spanner/issues/149
The first step is to add a
$types
array to both Models and Builders, and to automatically inject a Models types array into every new Builder instance for that model.The next step is overriding the base update/insert methods of the Builder, to loop through each value being passed in, and using a function
checkForType()
that attempts to apply the same conventions as the Parameterizer class, to generate an array of bound types eg['p2' => Database::TYPE, ...]
.The final step is adding an additional optional
$types = []
argument to Connectionupdate
/insert
/statement
/affectingStatement
methods, so that it can be passed to the$transaction->executeUpdate()
as an additionaltypes
argument alongsideparameters
I am still not super familiar with how all the internals work here, likely there are several improvements that could be made, but hopefully this is a good jumping off point