Skip to content

Commit

Permalink
Use bigint DBAL type for ID fields by default (#1223)
Browse files Browse the repository at this point in the history
  • Loading branch information
mvorisek authored May 29, 2024
1 parent 577e9c5 commit 6d3fdb3
Show file tree
Hide file tree
Showing 31 changed files with 238 additions and 195 deletions.
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

0 comments on commit 6d3fdb3

Please sign in to comment.