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

Use bigint DBAL type for ID fields by default #1223

Merged
merged 19 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/aggregates.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ in various ways to fine-tune aggregation. Below is one sample use:
$aggregate = new AggregateModel($orders);
$aggregate->addField('country');
$aggregate->setGroupBy(['country_id'], [
'count' => ['expr' => 'count(*)', 'type' => 'integer'],
'count' => ['expr' => 'count(*)', 'type' => 'bigint'],
'total_amount' => ['expr' => 'sum([amount])', 'type' => 'atk4_money'],
],
);
Expand Down
25 changes: 2 additions & 23 deletions docs/design.md
Original file line number Diff line number Diff line change
Expand Up @@ -374,26 +374,8 @@ $order = new Model_Order($db);
$order = $order->load(10);
```

In scenario above we loaded a specific record. Agile Data does not create a
separate object when loading, instead the same object is re-used. This is done
to preserve some memory.

So in the code above `$order` is not created for the record, but it can load
any record from the DataSet. Think of it as a "window" into a large table of
Orders:

```
$sum = 0;
$order = new Model_Order($db);
$order = $order->load(10);
$sum += $order->get('amount');

$order = $order->load(11);
$sum += $order->get('amount');

$order = $order->load(13);
$sum += $order->get('amount');
```
In scenario above we loaded a specific record. Agile Data creates a separate
object/entity when loading by cloning the original object/model.

You can iterate over the DataSet:

Expand All @@ -404,9 +386,6 @@ foreach (new Model_Order($db) as $order) {
}
```

You must remember that the code above will only create a single object and
iterating it will simply make it load different values.

At this point, I'll jump ahead a bit and will show you an alternative code:

```
Expand Down
3 changes: 0 additions & 3 deletions docs/fields.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,6 @@ $model->set('name', 'John');
echo $model->get('name'); // John
```

Just like you can reuse {php:class}`Model` to access multiple data records,
{php:class}`Field` object will be reused also.

## Purpose of Field

Implementation of Field in Agile Data is a very powerful and distinctive feature.
Expand Down
4 changes: 2 additions & 2 deletions docs/joins.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ but you wouldn't want that adding a new user would create a new country:

```
$user->addField('username');
$user->addField('country_id', ['type' => 'integer']);
$user->addField('country_id', ['type' => 'bigint']);
$jCountry = $user->join('country', ['weak' => true, 'prefix' => 'country_']);
$jCountry->addField('code');
$jCountry->addField('name');
Expand Down Expand Up @@ -110,7 +110,7 @@ $jCreditCard = $user->join('credit_card', [
'prefix' => 'cc_',
'masterField' => 'default_credit_card_id',
]);
$jCreditCard->addField('integer'); // creates cc_number
$jCreditCard->addField('bigint'); // creates cc_number
$jCreditCard->addField('name'); // creates cc_name
```

Expand Down
15 changes: 1 addition & 14 deletions docs/model.md
Original file line number Diff line number Diff line change
Expand Up @@ -216,20 +216,7 @@ $model->addField('name');
var_dump($model->getField('name'));
```

Other persistence framework will use "properties", because individual objects may impact
performance. In ATK Data this is not an issue, because "Model" is re-usable:

```
foreach (new User($db) as $user) {
// will be the same object every time!!
var_dump($user->getField('name'));

// this is also the same object every time!!
var_dump($user);
}
```

Instead, Field handles many very valuable operations which would otherwise fall on the
Field handles many very valuable operations which would otherwise fall on the
shoulders of developer (Read more here {php:class}`Field`)

:::{php:method} addField($name, $seed)
Expand Down
2 changes: 1 addition & 1 deletion docs/persistence.md
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ $m->onHook(Model::HOOK_BEFORE_SAVE, function (Model $entity) {
$arc = $this->withPersistence($entity->getApp()->archive_db);

// add some audit fields
$arc->addField('original_id', ['type' => 'integer'])->set($this->getId());
$arc->addField('original_id', ['type' => 'bigint'])->set($this->getId());
$arc->addField('saved_by')->set($this->getApp()->user);

$arc->saveAndUnload();
Expand Down
4 changes: 2 additions & 2 deletions docs/references.md
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ select * from user where id in
By passing options to hasOne() you can also differentiate field name:

```
$o->addField('user_id', ['type' => 'integer']);
$o->addField('user_id', ['type' => 'bigint']);
$o->hasOne('User', ['model' => $u, 'ourField' => 'user_id']);

$o->load(1)->ref('User')['name'];
Expand Down Expand Up @@ -492,7 +492,7 @@ No condition will be applied by default so it's all up to you:
$m->addReference('Archive', ['model' => static function (Persistence $persistence) use ($m) {
$archive = new $m(null, ['table' => $m->table . '_archive']);

$m->addField('original_id', ['type' => 'integer']);
$m->addField('original_id', ['type' => 'bigint']);

if ($m->isLoaded())) {
$archive->addCondition('original_id', $m->getId());
Expand Down
22 changes: 11 additions & 11 deletions docs/typecasting.md
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,17 @@ a different type.

### Supported types

- 'string' - for storing short strings, such as name of a person. Normalize will trim the value.
- 'text' - for storing long strings, suchas notes or description. Normalize will trim the value.
- 'boolean' - normalize will cast value to boolean.
- 'integer' - normalize will cast value to integer.
- 'atk4_money' - normalize will round value with 4 digits after dot.
- 'float' - normalize will cast value to float.
- 'date' - normalize will convert value to DateTime object.
- 'datetime' - normalize will convert value to DateTime object.
- 'time' - normalize will convert value to DateTime object.
- 'json' - no normalization by default
- 'object' - no normalization by default
- `string` - for storing short strings, such as name of a person. Normalize will trim the value.
- `text` - for storing long strings, suchas notes or description. Normalize will trim the value.
- `boolean` - normalize will cast value to boolean.
- `smallint`, `integer`, `bigint` - normalize will cast value to integer.
- `atk4_money` - normalize will round value with 4 digits after dot.
- `float` - normalize will cast value to float.
- `date` - normalize will convert value to DateTime object.
- `datetime` - normalize will convert value to DateTime object.
- `time` - normalize will convert value to DateTime object.
- `json` - no normalization by default
- `object` - no normalization by default

### Types and UI

Expand Down
2 changes: 2 additions & 0 deletions src/Field.php
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ public function normalize($value)
}

break;
case 'smallint':
case 'integer':
case 'bigint':
case 'float':
case 'decimal':
case 'atk4_money':
Expand Down
2 changes: 1 addition & 1 deletion src/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ protected function init(): void

if ($this->idField) {
if (!$this->hasField($this->idField)) {
$this->addField($this->idField, ['type' => 'integer']);
$this->addField($this->idField, ['type' => 'bigint']);
}
$this->getIdField()->required = true;
$this->getIdField()->system = true;
Expand Down
2 changes: 1 addition & 1 deletion src/Model/Join.php
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ protected function createFakeForeignModel(): Model
}
}
if ($fakeModel->idField !== $this->foreignField) {
$fakeModel->addField($this->foreignField, ['type' => 'integer']);
$fakeModel->addField($this->foreignField, ['type' => 'bigint']);
}

return $fakeModel;
Expand Down
79 changes: 52 additions & 27 deletions src/Persistence.php
Original file line number Diff line number Diff line change
Expand Up @@ -340,14 +340,16 @@ public function typecastLoadRow(Model $model, array $row): array
/**
* @param mixed $value
*
* @return ($value is scalar ? scalar : mixed)
* @return ($value is scalar ? scalar|null : mixed)
*/
private function _typecastPreField(Field $field, $value, bool $fromLoad)
{
if (is_string($value)) {
switch ($field->type) {
case 'boolean':
case 'smallint':
case 'integer':
case 'bigint':
$value = preg_replace('~\s+|,~', '', $value);

break;
Expand All @@ -359,30 +361,56 @@ private function _typecastPreField(Field $field, $value, bool $fromLoad)
break;
}

switch ($field->type) {
case 'boolean':
case 'integer':
case 'float':
case 'decimal':
case 'atk4_money':
if ($value === '') {
if ($value === '') {
// TODO should be handled by DBAL types itself like "json" type already does
// https://github.com/doctrine/dbal/blob/4.0.2/src/Types/JsonType.php#L55
switch ($field->type) {
case 'boolean':
case 'smallint':
case 'integer':
case 'bigint':
case 'float':
case 'decimal':
case 'atk4_money':
case 'datetime':
case 'date':
case 'time':
case 'object':
$value = null;
} elseif (!is_numeric($value)) {
throw new Exception('Must be numeric');
}

break;
break;
}
} else {
switch ($field->type) {
case 'boolean':
case 'smallint':
case 'integer':
case 'bigint':
case 'float':
case 'decimal':
case 'atk4_money':
if (!is_numeric($value)) {
throw new Exception('Must be numeric');
}

break;
}
}
} elseif ($value !== null) {
switch ($field->type) {
case 'string':
case 'text':
case 'boolean':
case 'smallint':
case 'integer':
case 'bigint':
case 'float':
case 'decimal':
case 'atk4_money':
if (is_bool($value)) {
throw new Exception('Must not be bool type');
if ($field->type !== 'boolean') {
throw new Exception('Must not be bool type');
}
} elseif (is_int($value)) {
if ($fromLoad) {
$value = (string) $value;
Expand Down Expand Up @@ -482,13 +510,10 @@ protected function _typecastSaveField(Field $field, $value)
{
$value = $this->_typecastPreField($field, $value, false);

if (in_array($field->type, ['json', 'object'], true) && $value === '') { // TODO remove later
return null;
}

// native DBAL DT types have no microseconds support
if (in_array($field->type, ['datetime', 'date', 'time'], true)
&& str_starts_with(get_class(Type::getType($field->type)), 'Doctrine\DBAL\Types\\')) {
if ($value !== null && in_array($field->type, ['datetime', 'date', 'time'], true)
&& str_starts_with(get_class(Type::getType($field->type)), 'Doctrine\DBAL\Types\\')
) {
if ($value === '') {
return null;
} elseif (!$value instanceof \DateTimeInterface) {
Expand All @@ -507,6 +532,7 @@ protected function _typecastSaveField(Field $field, $value)
}

$res = Type::getType($field->type)->convertToDatabaseValue($value, $this->getDatabasePlatform());

if (is_resource($res) && get_resource_type($res) === 'stream') {
$res = stream_get_contents($res);
}
Expand All @@ -526,14 +552,10 @@ protected function _typecastLoadField(Field $field, $value)
{
$value = $this->_typecastPreField($field, $value, true);

// TODO casting optionally to null should be handled by type itself solely
if ($value === '' && in_array($field->type, ['boolean', 'integer', 'float', 'decimal', 'datetime', 'date', 'time', 'json', 'object'], true)) {
return null;
}

// native DBAL DT types have no microseconds support
if (in_array($field->type, ['datetime', 'date', 'time'], true)
&& str_starts_with(get_class(Type::getType($field->type)), 'Doctrine\DBAL\Types\\')) {
if ($value !== null && in_array($field->type, ['datetime', 'date', 'time'], true)
&& str_starts_with(get_class(Type::getType($field->type)), 'Doctrine\DBAL\Types\\')
) {
$format = ['date' => 'Y-m-d', 'datetime' => 'Y-m-d H:i:s', 'time' => 'H:i:s'][$field->type];
if (str_contains($value, '.')) { // time possibly with microseconds, otherwise invalid format
$format = preg_replace('~(?<=H:i:s)(?![. ]*u)~', '.u', $format);
Expand All @@ -556,7 +578,10 @@ protected function _typecastLoadField(Field $field, $value)
}

$res = Type::getType($field->type)->convertToPHPValue($value, $this->getDatabasePlatform());
if (is_resource($res) && get_resource_type($res) === 'stream') {

if ($field->type === 'bigint' && $res === (string) (int) $res) { // once DBAL 3.x support is dropped, it should no longer be needed
$res = (int) $res;
} elseif (is_resource($res) && get_resource_type($res) === 'stream') {
$res = stream_get_contents($res);
}

Expand Down
6 changes: 4 additions & 2 deletions src/Persistence/Array_.php
Original file line number Diff line number Diff line change
Expand Up @@ -306,10 +306,12 @@ public function generateNewId(Model $model)

$type = $model->idField
? $model->getIdField()->type
: 'integer';
: 'bigint';

switch ($type) {
case 'smallint':
case 'integer':
case 'bigint':
$nextId = ($this->maxSeenIdByTable[$model->table] ?? 0) + 1;
$this->maxSeenIdByTable[$model->table] = $nextId;

Expand All @@ -319,7 +321,7 @@ public function generateNewId(Model $model)

break;
default:
throw (new Exception('Unsupported id field type. Array supports type=integer or type=string only'))
throw (new Exception('Unsupported ID field type'))
->addMoreInfo('type', $type);
}

Expand Down
4 changes: 2 additions & 2 deletions src/Persistence/Sql/Join.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ protected function init(): void
// TODO this mutates the owner model/joins!
if (!$this->reverse && !$this->getOwner()->hasField($this->masterField)) {
$owner = $this->hasJoin() ? $this->getJoin() : $this->getOwner();
$field = $owner->addField($this->masterField, ['type' => 'integer', 'system' => true, 'readOnly' => true]);
$field = $owner->addField($this->masterField, ['type' => 'bigint', 'system' => true, 'readOnly' => true]);
$this->masterField = $field->shortName; // TODO this mutates the join!
} elseif ($this->reverse && !$this->getOwner()->hasField($this->foreignField) && $this->hasJoin()) {
$owner = $this->getJoin();
$field = $owner->addField($this->foreignField, ['type' => 'integer', 'system' => true, 'readOnly' => true, 'actual' => $this->masterField]);
$field = $owner->addField($this->foreignField, ['type' => 'bigint', 'system' => true, 'readOnly' => true, 'actual' => $this->masterField]);
$this->foreignField = $field->shortName; // TODO this mutates the join!
}
}
Expand Down
9 changes: 9 additions & 0 deletions src/Persistence/Sql/Sqlite/PlatformTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,20 @@

namespace Atk4\Data\Persistence\Sql\Sqlite;

use Atk4\Data\Persistence\Sql\PlatformFixColumnCommentTypeHintTrait;
use Doctrine\DBAL\Schema\ForeignKeyConstraint;
use Doctrine\DBAL\Schema\TableDiff;

trait PlatformTrait
{
// remove (and related property) once https://github.com/doctrine/dbal/pull/6411 is merged and released
use PlatformFixColumnCommentTypeHintTrait;

/** @var list<string> */
private $requireCommentHintTypes = [
'bigint',
];

public function __construct()
{
$this->disableSchemaEmulation(); // @phpstan-ignore method.deprecated
Expand Down
Loading
Loading