Skip to content

Typecast refactoring #260

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Jul 13, 2023
Merged

Typecast refactoring #260

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
1b72206
Typecast refactoring
Tigrov Jul 4, 2023
0335ea8
Fix test issues
Tigrov Jul 5, 2023
0c2a962
Fix test issues
Tigrov Jul 5, 2023
741e93f
Fix test issues
Tigrov Jul 5, 2023
e082299
Fix test issues
Tigrov Jul 5, 2023
74a7b50
Fix test issues
Tigrov Jul 5, 2023
9e455ca
Fix test issues
Tigrov Jul 6, 2023
09fd7cf
Rename `$columnSchema` to `$column`
Tigrov Jul 7, 2023
0c146f5
Update
Tigrov Jul 9, 2023
f792bcc
Update
Tigrov Jul 9, 2023
f1f9006
Add test `DEFAULT CURRENT_TIMESTAMP` for `text` field
Tigrov Jul 9, 2023
1ae0d8e
Add test `DEFAULT CURRENT_TIMESTAMP` for `text` field
Tigrov Jul 9, 2023
893f367
Fix tests issue
Tigrov Jul 9, 2023
942033f
Add line to changelog
Tigrov Jul 10, 2023
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
4 changes: 2 additions & 2 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ composer.phar

# phpunit itself is not needed
phpunit.phar
phpunit.result.cache

# local phpunit config
# local phpunit config and cache
/phpunit.xml
/.phpunit.result.cache

# NPM packages
/node_modules
Expand Down
4 changes: 2 additions & 2 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@

## 1.0.1 under development

- no changes in this release.
- Enh #260: Typecast refactoring (@Tigrov)

## 1.0.0 April 12, 2023

- Initial release.
- Initial release.
33 changes: 23 additions & 10 deletions src/Schema.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -493,19 +493,32 @@ protected function loadColumnSchema(array $info): ColumnSchemaInterface
}

$column->phpType($this->getColumnPhpType($column));
$column->defaultValue($this->normalizeDefaultValue($info['dflt_value'], $column));

if (!$column->isPrimaryKey()) {
if ($info['dflt_value'] === 'null' || $info['dflt_value'] === '' || $info['dflt_value'] === null) {
$column->defaultValue(null);
} elseif ($info['dflt_value'] === 'CURRENT_TIMESTAMP' && $column->getType() === 'timestamp') {
$column->defaultValue(new Expression('CURRENT_TIMESTAMP'));
} else {
$value = trim($info['dflt_value'], "'\"");
$column->defaultValue($column->phpTypecast($value));
}
return $column;
}

/**
* Converts column's default value according to {@see ColumnSchema::phpType} after retrieval from the database.
*
* @param string|null $defaultValue The default value retrieved from the database.
* @param ColumnSchemaInterface $column The column schema object.
*
* @return mixed The normalized default value.
*
* @psalm-suppress PossiblyNullArgument
*/
private function normalizeDefaultValue(?string $defaultValue, ColumnSchemaInterface $column): mixed
{
if ($column->isPrimaryKey()) {
return null;
}

return $column;
return match ($defaultValue) {
null, 'null', '' => null,
'CURRENT_TIMESTAMP', 'CURRENT_DATE', 'CURRENT_TIME' => new Expression($defaultValue),
Copy link
Contributor

@darkdef darkdef Jul 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Need add check for type of column. Without check type of column - with default value in column equal 'CURRENT_TIMESTAMP' (string), default value will be incorrect

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no special date type timestamp or datetime in SQlite.
https://www.sqlite.org/datatype3.html

For this purpose text type can be used.

Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/yiisoft/db-sqlite/blob/master/tests/Provider/SchemaProvider.php#L162

Copy link
Contributor

Choose a reason for hiding this comment

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

We have dbtype in schema of column, and need check type in condition

Copy link
Member Author

Choose a reason for hiding this comment

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

Added test with STRICT table where TIMESTAMP type is not allowed but TEXT type for storing date and time is okey.

CREATE TABLE "timestamp_default" (
  id INTEGER PRIMARY KEY,
  text_col TEXT NOT NULL DEFAULT 'CURRENT_TIMESTAMP',
  timestamp_text TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP
) STRICT;

Could you show a test where current changes without check for type of column will fail?

Copy link
Contributor

Choose a reason for hiding this comment

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

test will be more reliable. Simply add test for column text_col with assert: defaultValue not instaceof Expression

Copy link
Contributor

Choose a reason for hiding this comment

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

The current test is sufficient. Make it work without errors and that's it. Thank you

default => $column->phpTypecast(trim($defaultValue, "'\"")),
};
}

/**
Expand Down
44 changes: 44 additions & 0 deletions tests/Provider/SchemaProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,50 @@ public static function columns(): array
],
'animal',
],
[
[
'id' => [
'type' => 'integer',
'dbType' => 'integer',
'phpType' => 'integer',
'primaryKey' => true,
'allowNull' => true,
'autoIncrement' => true,
'enumValues' => null,
'size' => null,
'precision' => null,
'scale' => null,
'defaultValue' => null,
],
'text_col' => [
'type' => 'text',
'dbType' => 'text',
'phpType' => 'string',
'primaryKey' => false,
'allowNull' => false,
'autoIncrement' => false,
'enumValues' => null,
'size' => null,
'precision' => null,
'scale' => null,
'defaultValue' => 'CURRENT_TIMESTAMP',
],
'timestamp_text' => [
'type' => 'text',
'dbType' => 'text',
'phpType' => 'string',
'primaryKey' => false,
'allowNull' => false,
'autoIncrement' => false,
'enumValues' => null,
'size' => null,
'precision' => null,
'scale' => null,
'defaultValue' => new Expression('CURRENT_TIMESTAMP'),
],
],
'timestamp_default',
],
];
}

Expand Down
6 changes: 6 additions & 0 deletions tests/Support/Fixture/sqlite.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,12 @@ CREATE TABLE "notauto_pk" (
PRIMARY KEY (id_1, id_2)
);

CREATE TABLE "timestamp_default" (
id INTEGER PRIMARY KEY,
text_col TEXT NOT NULL DEFAULT 'CURRENT_TIMESTAMP',
timestamp_text TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP
); -- STRICT

CREATE VIEW "animal_view" AS SELECT * FROM "animal";

INSERT INTO "animal" ("type") VALUES ('yiiunit\data\ar\Cat');
Expand Down
1 change: 1 addition & 0 deletions tests/Support/Runtime/.gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/yiitest.sq3