Skip to content

Conversation

@zikezhang
Copy link
Contributor

base on the code there https://github.com/phalcon/cphalcon/blob/master/phalcon/Mvc/Model/Query.zep#L477

getSingleResult()     retrun ModelInterface.

@headio
Copy link
Owner

headio commented Apr 18, 2023

Hi,

"fetchColumn" was intended to be used for returning scalars, rather than complete objects or a mixture of scalars and complete objects (Phalcon\Mvc\Model\Resultset\Complex).

Example:

$criteria = $this->repository
->createCriteria()
->columns(['c' => 'COUNT(*)'])
;
$result = $this->repository->fetchColumn($criteria);

$criteria = $this->repository
->createCriteria()
->columns(['c' => 'MAX(position)'])
;
$result = $this->repository->fetchColumn($criteria);

Agreed one could change the return type to: Phalcon\Mvc\Model\Row|null instead of "mixed".

Apart from the above, we can't set the return type to: Phalcon\Mvc\ModelInterface, because in actual fact the
the implementation of "getSingleResult" in contract Phalcon\Mvc\Model\QueryInterface returns "mixed".

Under the hood, getSingleResult() calls "execute()" (L264); this method returns "mixed" (hence my mixed declaration in fetchColumn), because it handles many types of queries. When the PHQL is parsed, "executeSelect" (L502) is called, which in turn returns either a ResultsetInterface or an Array.

If you call "->setUniqueRow(true)" it invokes "getFirst()" on the resultset, which in turn returns: ModelInterface|Row|null depending on the query (see below).

// returns a Phalcon\Mvc\Model\Row instance
$manager->createQuery('SELECT r.id FROM Robots AS r')->execute()->getFirst();

// returns a Phalcon\Mvc\Model\ModelInterface
$manager->createQuery('SELECT * FROM Robots')->execute()->getFirst();

There are many problems like this in Phalcon interfaces; they need to be rewritten inline with the current concrete implementations.

What queries are you using with fetchColumn? Let me know perhaps I can help.

@zikezhang
Copy link
Contributor Author

Hi headio,

In the class

Repository/Traits/CacheableTrait

public function fetchColumn() always returns null because of

$resultset = $query->getSingleResult();

$resultset is not instanceof Row.

@headio
Copy link
Owner

headio commented Apr 20, 2023

Hi headio,

In the class

Repository/Traits/CacheableTrait

public function fetchColumn() always returns null because of

$resultset = $query->getSingleResult();

$resultset is not instanceof Row.

Hi,

Run the following integration test from the project directory:

vendor/bin/codecept run integration Repository::canFetchNumberOfRecords -vvv -f

or the cache version:

vendor/bin/codecept run integration Repository::canCacheFetchRecordCount -vvv -f

and you will see both tests return (int) 5 as expected—both tests call the method: fetchColumn().

Moreover, if you open src/Repository/QueryRepository.php (concrete implementations without cache) and goto L71 (inside the method fetchColumn) and place:
var_dump($result);
die;

and re-run the test, you will see:

object(Phalcon\Mvc\Model\Row)#806 (1) {
  ["c"]=>
  int(5)
}

See attached screen shot.
test

Show me your code; it may well return null depending on what you are querying.

Regards,

@zikezhang
Copy link
Contributor Author

I really appreciate your taking the time to explain it.
so my code is below:

   public function getQueryByColumn(string $column, $value) : QueryInterface
    {
        return $this->createCriteria()
                ->eq($column, $value)
                //->eq('status', 'published')
                ->createBuilder()
                ->getQuery()
            ;
    }

    public function getByColumn(string $column,$value) : ?array
    {
        return $this->getCacheByQuery($this->getQueryByColumn($column,$value));
    }

    public function getCacheByQuery(QueryInterface $query) : ?array
    {
        $resultset = $this->fromCache($query,function() use ($query){
            return $query->setUniqueRow(true)->getSingleResult();
        });
        if ($resultset instanceof ModelInterface) {
            return $resultset->toArray();
        }
        return null;
    }

The reason I put getQueryByColumn() in the middle, because I wannt get the sql from the $query. to delete single cache by the key(not the whole module).

@headio
Copy link
Owner

headio commented Apr 25, 2023

No worries!

I will take a look this afternoon (currently 9am) and get back to you.

@headio
Copy link
Owner

headio commented Apr 26, 2023

Hi,
Generally speaking, I think managing cache keys is inherently difficult, it requires much thought about the desired system and the cost of managing it. What 'phalcon-service-layer' offers is hassle-free way of caching queries and not having to worry about cache invalidation—whenever a model (Entity) changes, or is removed, all related queries (this also includes related entity queries too) are invalidated. Naturally, you can cache a query for a desired lifetime using the method "remember()", so something like this from the service layer:

public function getPublished(): ModelInterface
{
    // cached for 60 seconds
    $model = $this->repository->remember(60)->findFirstByStatus("published");
    return model;
}

The above will return what you are doing with your implementation.

or using a "DateInterval" like:

// cached for 5 days
$this->repository->remember(new DateInterval("P5D"))->findFirstByStatus("published");

Looking at your code, if you swap "getSingleResult()" with "execute()" it will return the first row from the result set. This will be an instance of "ModelInterface" (see below). You don't need to call "getSingleResult()" if you have already called "setUniqueRow(true) on the query instance.

public function getCacheByQuery(QueryInterface $query) : ?array
    {
        $resultset = $this->fromCache($query,function() use ($query){
            return $query->setUniqueRow(true)->execute();
        });
        if ($resultset instanceof ModelInterface) {
            return $resultset->toArray();
        }
        return null;
    }

However, you probably want to move "setUniqueRow(true)" to your "getQueryByColumn" method, so "getCacheByQuery" can handle "findFirst" and "find" queries.

Find first record:

$query = $criteria->createBuilder()->getQuery()->setUniqueRow(true);
$model = $query->execute();

Find many:

$query = $criteria->createBuilder()->getQuery();
$model = $query->execute();

If you want to know what cache key was generated for a query, you would have to use the "appendCache" method on the query instance instead of "fromCache".

$repository = (new Repository());
$criteria = $repository->createCriteria()->eq('status', 'published');
$query = $criteria->createBuilder()->getQuery();
$constraint = $repository->appendCache(
    $query,
    60,
);

$lifetime =  $constraint->getCacheOptions()['lifetime']; // 60
$key =  $constraint->getCacheOptions()['key']; // system generated

$result = $query->execute();

Try out the QueryRepository methods, I have written without the cache to see your results. Then just as the cacheableTrait to cache everything or use "no cache()" and "remember()" methods for fine-grain control.

// Repository
Page extends QueryRepository
{
    public function newInstance(): ModelInterface
    {
        $modelName = $this->getModel();
        $model = new $modelName();

        return $model;
    }

    protected function getModelName(): string
    {
        return 'App\\Domain\\Model\\Page';
    }
}

// Service
class User extends Injectable implements ServiceInterface
{
    public function __construct(private PageInterface $repository)
    {
    }

    /**
     * Fetch first record by primary key from cache or storage.
     */
    public function getModel(int $id): ModelInterface
    {
        return $this->repository->findByPk($id);
    }

    /**
     * Return a collection of models by query criteria from
     * cache or storage.
     */
    public function getModels(CriteriaInterface $criteria): ResultsetInterface
    {
        return $this->repository->find($criteria);
    }

    /**
     * Fetch first published record from cache or storage.
     */
    public function getPublished(): ModelInterface
    {
        return $this->repository->findFirstByStatus("published");
    }

}

Hope that helps.


    

@zikezhang
Copy link
Contributor Author

Thank you very much @headio! you really did a great job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants