We launched new forums in March 2019—join us there. In a hurry for help with your website? Get Help Now!
    • 11055 ☆ A M B ☆
    • 3,112 Posts
    Hi,

    Following my tweet, I found that getCount() method fails if the query has alias as aggregate function because it only counts the ID of the given object's class and ignores anything else.
    This leaves the HAVING condition looses its reference.

    I created this snippet example for you to test:
    <?php
    $searchString = $modx->getOption('searchString', $scriptProperties, 'hello world');
    
    $c = $modx->newQuery('modResource');
    $c->select(array('modResource.id', 'modResource.pagetitle'));
    $c->select(array(
        'score' => "MATCH (modResource.pagetitle, modResource.content) AGAINST ('{$searchString}' IN BOOLEAN MODE)"
    ));
    $c->having("(score > 0)");
    
    $count = $modx->getCount('modResource', $c);
    
    $c->prepare();
    $sql = $c->toSQL();
    echo __LINE__ . ' : $sql = ' . $sql . '<br />';
    
    return $count;


    It returned error:
    [2014-01-16 22:12:16] (ERROR in xPDO::getCount @ D:\localhost\example.com\www\core\xpdo\xpdo.class.php : 1024) Error 42S22 executing statement: 
    Array
    (
        [0] => 42S22
        [1] => 1054
        [2] => Unknown column 'score' in 'having clause'
    )
    


    So far, this is my fix to replace this getCount() method:
    <?php
    
        /**
         * Replacing MODX's getCount(), because it has bug on counting SQL with
         * aggregate functions and HAVING condition.<br>
         * Retrieves a count of xPDOObjects by the specified xPDOCriteria.
         *
         * @param string $className Class of xPDOObject to count instances of.
         * @param mixed $criteria Any valid xPDOCriteria object or expression.
         * @return integer The number of instances found by the criteria.
         * @see xPDO::getCount()
         */
        public function getQueryCount($className, $criteria= null) {
            $count= 0;
            if ($query= $this->modx->newQuery($className, $criteria)) {
                $expr= '*';
                if ($pk= $this->modx->getPK($className)) {
                    if (!is_array($pk)) {
                        $pk= array ($pk);
                    }
                    $expr= $this->modx->getSelectColumns($className, $query->getAlias(), '', $pk);
                }
                if (isset($query->query['columns'])) {
                    foreach ($query->query['columns'] as $k => $column) {
                        if (is_numeric($k)) {
                            unset($query->query['columns'][$k]);
                        }
                    }
                }
                $query->select("COUNT(DISTINCT {$expr})");
                if ($stmt= $query->prepare()) {
                    $tstart = microtime(true);
                    if ($stmt->execute()) {
                        $this->modx->queryTime += microtime(true) - $tstart;
                        $this->modx->executedQueries++;
                        $idx = count($query->query['columns']) - 1;
                        $idx = $idx >= 0 ? $idx : 0;
                        if ($results= $stmt->fetchAll(PDO::FETCH_COLUMN, $idx)) {
                            $count= reset($results);
                            $count= intval($count);
                        }
                    } else {
                        $this->modx->queryTime += microtime(true) - $tstart;
                        $this->modx->executedQueries++;
                        $this->modx->log(modX::LOG_LEVEL_ERROR, "Error " . $stmt->errorCode() . " executing statement: \n" . print_r($stmt->errorInfo(), true), '', __METHOD__, __FILE__, __LINE__);
                    }
                }
            }
            return $count;
        }


    Let me know if there's a better way.

    Cheers,

    Rico

    EDIT:
    This is still not correct method, since I found miscalculating in someway.

    This question has been answered by goldsky. See the first response.

    [ed. note: goldsky last edited this post 12 years, 8 months ago.]
      Rico
      Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
      MODx is great, but knowing how to use it well makes it perfect!

      www.virtudraft.com

      Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

      Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

      Maintainter/contributor of Babel

      Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
      • 11055 ☆ A M B ☆
      • 3,112 Posts
      I ended up to have it like this:

      <?php
          /**
           * Replacing MODX's getCount(), because it has bug on counting SQL with
           * aggregate functions and HAVING condition.<br>
           * Retrieves a count of xPDOObjects by the specified xPDOCriteria.
           *
           * @param string $className Class of xPDOObject to count instances of.
           * @param mixed $criteria Any valid xPDOCriteria object or expression.
           * @return integer The number of instances found by the criteria.
           * @see xPDO::getCount()
           */
          public function getQueryCount($className, $criteria= null) {
              $count= 0;
              if ($query= $this->modx->newQuery($className, $criteria)) {
                  $expr= '*';
                  if (!empty($query->query['having']) || !empty($query->query['groupby'])) {
                      if ($pk= $this->modx->getPK($className)) {
                          if (!is_array($pk)) {
                              $pk= array ($pk);
                          }
                          $expr= $this->modx->getSelectColumns($className, 'alias', '', $pk);
                      }
                      $query->prepare();
                      $sql = $query->toSQL();
                      $stmt= $this->modx->query("SELECT COUNT($expr) FROM ($sql) alias");
                  } else {
                      if ($pk= $this->modx->getPK($className)) {
                          if (!is_array($pk)) {
                              $pk= array ($pk);
                          }
                          $expr= $this->modx->getSelectColumns($className, $query->getAlias(), '', $pk);
                      }
                      if (isset($query->query['columns'])) $query->query['columns'] = array();
                      $query->select(array ("COUNT(DISTINCT {$expr})"));
                      $stmt= $query->prepare();
                  }
      
                  if ($stmt) {
                      $tstart = microtime(true);
                      if ($stmt->execute()) {
                          $this->modx->queryTime += microtime(true) - $tstart;
                          $this->modx->executedQueries++;
                          if ($results= $stmt->fetchAll(PDO::FETCH_COLUMN)) {
                              $count= reset($results);
                              $count= intval($count);
                          }
                      } else {
                          $this->modx->queryTime += microtime(true) - $tstart;
                          $this->modx->executedQueries++;
                          $this->modx->log(modX::LOG_LEVEL_ERROR, "Error " . $stmt->errorCode() . " executing statement: \n" . print_r($stmt->errorInfo(), true), '', __METHOD__, __FILE__, __LINE__);
                      }
                  }
              }
              return $count;
          }
      


      EDIT:
      concluded the adaptation. [ed. note: goldsky last edited this post 12 years, 8 months ago.]
        Rico
        Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
        MODx is great, but knowing how to use it well makes it perfect!

        www.virtudraft.com

        Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

        Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

        Maintainter/contributor of Babel

        Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
        • 22303 MODX Staff
        • 10,725 Posts
        @goldsky: Can you check this commit please? It was a PR aimed at solving getCount for queries containing a group by. I'm wondering if it might solve your issue as well.
          • 11055 ☆ A M B ☆
          • 3,112 Posts
          ok, just tested it, and it seems to be working.
          thanks for fix.

          i'll try to adapt your code, so my package can work for MODX's fallback versions.

          will that be merged on 2.2.11 (so I can define the version comparison)?

          thanks @opengeek
            Rico
            Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
            MODx is great, but knowing how to use it well makes it perfect!

            www.virtudraft.com

            Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

            Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

            Maintainter/contributor of Babel

            Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
            • 11055 ☆ A M B ☆
            • 3,112 Posts
            Just found another bug.
            This time, it failed to grab another function: Control Flow Functions.
            Both of my method and your PR.
            My case was about to add another selection
            IFNULL(`my_template_variable_contentvalue`.`value`, `my_template_variable`.`default_text`) AS `my_template_variable`


            I wonder if getCount() fails to the other functions as well. [ed. note: goldsky last edited this post 12 years, 8 months ago.]
              Rico
              Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
              MODx is great, but knowing how to use it well makes it perfect!

              www.virtudraft.com

              Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

              Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

              Maintainter/contributor of Babel

              Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
            • discuss.answer
              • 11055 ☆ A M B ☆
              • 3,112 Posts
              I can no longer rely on getCount() if there is function inside the Query.

              I don't like the performance of this, but it seems the most reliable one to count the Query which has function:

              <?php
              
                  /**
                   * Replacing MODX's getCount(), because it has bug on counting SQL with function.<br>
                   * Retrieves a count of xPDOObjects by the specified xPDOCriteria.
                   *
                   * @param string $className Class of xPDOObject to count instances of.
                   * @param mixed $criteria Any valid xPDOCriteria object or expression.
                   * @return integer The number of instances found by the criteria.
                   * @see xPDO::getCount()
                   */
                  public function getQueryCount($className, $criteria= null) {
                      $count= 0;
                      if ($query= $this->modx->newQuery($className, $criteria)) {
                          $expr= '*';
                          if ($pk= $this->modx->getPK($className)) {
                              if (!is_array($pk)) {
                                  $pk= array ($pk);
                              }
                              $expr= $this->modx->getSelectColumns($className, 'alias', '', $pk);
                          }
                          $query->prepare();
                          $sql = $query->toSQL();
                          $stmt= $this->modx->query("SELECT COUNT($expr) FROM ($sql) alias");
                          if ($stmt) {
                              $tstart = microtime(true);
                              if ($stmt->execute()) {
                                  $this->modx->queryTime += microtime(true) - $tstart;
                                  $this->modx->executedQueries++;
                                  if ($results= $stmt->fetchAll(PDO::FETCH_COLUMN)) {
                                      $count= reset($results);
                                      $count= intval($count);
                                  }
                              } else {
                                  $this->modx->queryTime += microtime(true) - $tstart;
                                  $this->modx->executedQueries++;
                                  $this->modx->log(modX::LOG_LEVEL_ERROR, "Error " . $stmt->errorCode() . " executing statement: \n" . print_r($stmt->errorInfo(), true), '', __METHOD__, __FILE__, __LINE__);
                              }
                          }
                      }
                      return $count;
                  }
              
                Rico
                Genius is one percent inspiration and ninety-nine percent perspiration. Thomas A. Edison
                MODx is great, but knowing how to use it well makes it perfect!

                www.virtudraft.com

                Security, security, security! | Indonesian MODx Forum | MODx Revo's cheatsheets | MODx Evo's cheatsheets

                Author of Easy 2 Gallery 1.4.x, PHPTidy, spieFeed, FileDownload R, Upload To Users CMP, Inherit Template TV, LexRating, ExerPlan, Lingua, virtuNewsletter, Grid Class Key, SmartTag, prevNext

                Maintainter/contributor of Babel

                Because it's hard to follow all topics on the forum, PING ME ON TWITTER @_goldsky if you need my help.
                • 22303 MODX Staff
                • 10,725 Posts
                getCount was added to make it easy to count the number of objects of a specific class. I merged the previous PR (yes, will be in 2.2.11) to support the group by counts as a courtesy, but you can get counts and any other aggregations manually very easily on xPDO. I do not see a reason to try and handle every type of aggregation in this method, especially those dependent on platform-dependent SQL functions.