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.

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

Add getSingleScalarResultOrNull method #11296

Open
wants to merge 4 commits into
base: 3.4.x
Choose a base branch
from
Open

Add getSingleScalarResultOrNull method #11296

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2b27394
Add getSingleScalarResultOrNull method
arkhamvm Feb 23, 2024
1e64365
getSingleScalarResultOrNull unit tests
arkhamvm Feb 23, 2024
96e2881
getSingleScalarResultOrNull unit tests
arkhamvm Feb 23, 2024
a300312
remove unnecessary import
arkhamvm Feb 23, 2024
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
18 changes: 18 additions & 0 deletions src/AbstractQuery.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,24 @@ public function getSingleScalarResult(): mixed
return $this->getSingleResult(self::HYDRATE_SINGLE_SCALAR);
}

/**
* Gets the single scalar result of the query, returns null if result is empty.
*
* Alias for getSingleResult(HYDRATE_SINGLE_SCALAR) and try/catch NoResultException.
*
* @return bool|float|int|string|null The scalar result.
*
* @throws NonUniqueResultException If the query result is not unique.
*/
public function getSingleScalarResultOrNull(): mixed
{
try {
return $this->getSingleResult(self::HYDRATE_SINGLE_SCALAR);
} catch (NoResultException) {
Copy link
Member

@greg0ire greg0ire Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

We shouldn't use exceptions for flow control, they're expensive.

Copy link
Author

@arkhamvm arkhamvm Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@greg0ire what do you mean? getOneOrNullResult for examle already use NoResultException. Or you point is to completely remove NoResultException ?

Copy link
Member

Choose a reason for hiding this comment

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

Oh right we already do this. But let's not add more code with the same bad practice. I mean, why is no result considered exceptional in the first place?

Copy link
Member

@greg0ire greg0ire Oct 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

But I also think you can completely remove it. Look at the code of getSingleResult.

        if ($this->_hydrationMode !== self::HYDRATE_SINGLE_SCALAR && ! $result) {
            throw new NoResultException();
        }

So maybe it's just the comment on line 797 that is wrong? Can you find a way to call getSingleScalarResult() and get a NoResultException?

The line was added in 4781dc0

This comment was marked as resolved.

Sign in to view

Copy link
Member

Choose a reason for hiding this comment

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

After reading the thread at #7871 I am now confused as to what the right way would be. That $hydrationMode vs $this->hydrationMode thing that I didn't spot is hard to grasp.

Copy link
Member

Choose a reason for hiding this comment

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

My bad, this behavior is actually proven by tests:

orm/tests/Tests/ORM/Functional/QueryTest.php

Lines 465 to 470 in e7efded

public function testGetSingleScalarResultThrowsExceptionOnNoResult(): void
{
$this->expectException(NoResultException::class);
$this->_em->createQuery('select a.id from Doctrine\Tests\Models\CMS\CmsArticle a')
->getSingleScalarResult();
}

return null;
}
}

/**
* Sets a query hint. If the hint name is not recognized, it is silently ignored.
*
Expand Down
63 changes: 63 additions & 0 deletions tests/Tests/ORM/Functional/QueryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,13 @@ public function testGetSingleScalarResultThrowsExceptionOnNoResult(): void
->getSingleScalarResult();
}

public function testGetSingleScalarResultOrNullNotThrowsExceptionOnNoResult(): void
{
$this->expectNotToPerformAssertions();
$this->_em->createQuery('select a.id from Doctrine\Tests\Models\CMS\CmsArticle a')
->getSingleScalarResultOrNull();
}

public function testGetSingleScalarResultThrowsExceptionOnSingleRowWithMultipleColumns(): void
{
$user = new CmsUser();
Expand All @@ -446,6 +453,29 @@ public function testGetSingleScalarResultThrowsExceptionOnSingleRowWithMultipleC
->getSingleScalarResult();
}

public function testGetSingleScalarResultOrNullThrowsExceptionOnSingleRowWithMultipleColumns(): void
{
$user = new CmsUser();
$user->name = 'Javier';
$user->username = 'phansys';
$user->status = 'developer';

$this->_em->persist($user);

$this->_em->flush();
$this->_em->clear();

$this->expectException(NonUniqueResultException::class);
$this->expectExceptionMessage(
'The query returned a row containing multiple columns. Change the query or use a different result function'
. ' like getScalarResult().',
);

$this->_em->createQuery('select u from Doctrine\Tests\Models\CMS\CmsUser u')
->setMaxResults(1)
->getSingleScalarResultOrNull();
}

public function testGetSingleScalarResultThrowsExceptionOnNonUniqueResult(): void
{
$user = new CmsUser();
Expand Down Expand Up @@ -479,6 +509,39 @@ public function testGetSingleScalarResultThrowsExceptionOnNonUniqueResult(): voi
->getSingleScalarResult();
}

public function testGetSingleScalarResultOrNullThrowsExceptionOnNonUniqueResult(): void
{
$user = new CmsUser();
$user->name = 'Guilherme';
$user->username = 'gblanco';
$user->status = 'developer';

$article1 = new CmsArticle();
$article1->topic = 'Doctrine 2';
$article1->text = 'This is an introduction to Doctrine 2.';
$user->addArticle($article1);

$article2 = new CmsArticle();
$article2->topic = 'Symfony 2';
$article2->text = 'This is an introduction to Symfony 2.';
$user->addArticle($article2);

$this->_em->persist($user);
$this->_em->persist($article1);
$this->_em->persist($article2);

$this->_em->flush();
$this->_em->clear();

$this->expectException(NonUniqueResultException::class);
$this->expectExceptionMessage(
'The query returned multiple rows. Change the query or use a different result function like getScalarResult().',
);

$this->_em->createQuery('select a.id from Doctrine\Tests\Models\CMS\CmsArticle a')
->getSingleScalarResultOrNull();
}

public function testModifiedLimitQuery(): void
{
for ($i = 0; $i < 5; $i++) {
Expand Down
Loading