Skip to content

Feature: Add Real/Float4 type#6467

Closed
berkut1 wants to merge 4 commits intodoctrine:3.9.xfrom
berkut1:3.9.x
Closed

Feature: Add Real/Float4 type#6467
berkut1 wants to merge 4 commits intodoctrine:3.9.xfrom
berkut1:3.9.x

Conversation

@berkut1
Copy link
Copy Markdown
Contributor

@berkut1 berkut1 commented Jul 8, 2024 •
edited
Loading

Q A
Type feature
Version 3.9 and 4.1

Summary

Hello,

As I described in this issue #6466 (comment), there is a strange behavior with types in Doctrine, especially after migrating from DC2Type.
One of the problems is that when type comments like DC2Type were used, many checks were seemingly ignored, which allowed the use of the REAL (float4) type. In fact, this is still possible now because there are scattered checks for REAL but they are not fully implemented.

For example, Doctrine won't notice a problem in this case:

CREATE TABLE test_tbl (
   attr REAL NOT NULL, 
);
#[ORM\Table(name: "test_tbl")]
#[ORM\Entity]
class TestEn
{
    private ?int $id = null;
    #[ORM\Column(name: 'attr', type: Types::FLOAT, nullable: false)]
    private float $attr;
}

However, it will try to revert it to DOUBLE PRECISION if we specify DEFAULT, but even here there is a nuance. If we specify it like this, Doctrine will accept the REAL type. (float in quotas in the entity and without them in the migration)

CREATE TABLE test_tbl (
   attr REAL DEFAULT 55.8 NOT NULL, 
);
#[ORM\Column(name: 'attr', type: Types::FLOAT, nullable: false, options: ['default' => '55.8'])]
private float $attr;

Therefore, in this PR, I want to add support for the REAL type, which is supported by most DBMSs, which will fix that weird behavior for float types, and this will also allow people to transition from DC2Type comments without the need to reassign types in mapping_types. It also seems odd to me that Doctrine supports SMALLINT, INT, BIGINT, but only supports float8 and not float4.

Thanks.

@berkut1
Copy link
Copy Markdown
Contributor Author

berkut1 commented Jul 8, 2024 •
edited
Loading

MySQL, as always, has its own naming conventions; for them, REAL and DOUBLE PRECISION is float8, and FLOAT is float4. I tried to fix this to pass the tests.

UPD:

Because MySQL does things differently from others, the type name REAL might confuse MySQL users. :)

If you are satisfied with the PR, it might be appropriate to rename Types::REAL to Types::SINGLE_FLOAT and accordingly rename the other new functions. What do you think?

P.S
https://mariadb.com/kb/en/float/
https://mariadb.com/kb/en/double/

@berkut1
Copy link
Copy Markdown
Contributor Author

berkut1 commented Jul 10, 2024 •
edited
Loading

Oracle DBMS also supports this type in its own way :). According to the documentation, the FLOAT type differs only in precision.
https://docs.oracle.com/en/database/oracle/oracle-database/12.2/sqlqr/Data-Types.html
Section: Converting to Oracle Data Types

Checked DBMS:

  • PostgreSQL (represents REAL as float4 or real)
  • MySQL/MariaDB (Feature: Add Real/Float4 type #6467 (comment))
  • MSSQL (represents REAL as real)
  • Oracle (represents REAL as float(63))
  • IBM DB2 (represents REAL as real)
  • SQLite (represents REAL as real)

@PowerKiKi PowerKiKi mentioned this pull request Jul 12, 2024
Open
@berkut1 berkut1 mentioned this pull request Jul 15, 2024
Open
@derrabus
Copy link
Copy Markdown
Member

derrabus commented Jul 16, 2024

Thank you. Please mind PHPCS.

We currently develop new features on the 4.1 branch. Can you elaborate why you this this change should be merged to 3.9 instead?

@berkut1
Copy link
Copy Markdown
Contributor Author

berkut1 commented Jul 16, 2024

Oh, because of the existence of the 3.9 branch, I just couldn't decide where to send this new feature first.
Hmm, do I need to recreate this PR for the 4.1 branch only?

@derrabus
Copy link
Copy Markdown
Member

derrabus commented Jul 16, 2024

Unless you can make a very strong case for why we should add this feature to DBAL 3, I'd prefer shipping your feature with DBAL 4 only. No need to recreate the PR, a simple rebase should be enough.

@berkut1
Copy link
Copy Markdown
Contributor Author

berkut1 commented Jul 16, 2024

Hmm... it was a mistake to make changes in the main branch. :) I tried to isolate my changes in a separate branch and then rebase them onto 4.1, but I'm having issues resolving merge conflicts. The git program doesn't want to accept them, and it's becoming messy. I think it's better if I recreate this PR, if you don't mind.

@greg0ire
Copy link
Copy Markdown
Member

greg0ire commented Jul 16, 2024 •
edited
Loading

You can build a new branch, reset this branch to the new one and force push. No need for a new PR

@derrabus
Copy link
Copy Markdown
Member

derrabus commented Jul 16, 2024

I think it's better if I recreate this PR, if you don't mind.

Sure, whatever works for you.

@berkut1
Copy link
Copy Markdown
Contributor Author

berkut1 commented Jul 16, 2024

@greg0ire
I tried, but Git suggests very strange conflict resolutions, deleting files that shouldn't be deleted. It would be faster and safer to recreate the PR.

@berkut1 berkut1 closed this Jul 16, 2024
@berkut1 berkut1 deleted the 3.9.x branch July 16, 2024 11:25
@berkut1 berkut1 mentioned this pull request Jul 16, 2024
Merged
6 tasks
derrabus pushed a commit that referenced this pull request Jul 22, 2024
@berkut1
Add SmallFloat type (#6471)
6ebcb77 
<!-- Fill in the relevant information below to help triage your pull
request. -->

|      Q       |   A
|------------- | -----------
| Type         | feature

#### Summary

Recreated PR #6467 for branch 4.1.

This PR adds support for the `REAL` type for all DBMS, thereby resolving
issues with partial support and potential bugs related to the `REAL`
type in SQL schemas during migration creation. I've tried to explain
this issue in detail through the `INET` type here
#6466.

Checked DBMS:

- [x] PostgreSQL (represents `REAL` as `float4` or `real`)
- [x] MySQL/MariaDB
(#6467 (comment))
- [x] MSSQL (represents `REAL` as `real`)
- [x] Oracle (represents `REAL` as `float(63)`) [doc
](https://docs.oracle.com/en/database/oracle/oracle-database/12.2/sqlqr/Data-Types.html)
- [x] IBM DB2 (represents `REAL` as `real`)
- [x] SQLite (represents `REAL` as `real`)
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

New Feature Type Mapping Types

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@berkut1 @derrabus @greg0ire