Discuz 7.2坑爹集锦-SQL篇 -update 2012.02.09

Discuz 7.2坑爹集锦-SQL篇

 

 

DZ使用的是MySQL的MyISAM引擎，特点是简单快速，非常适合网络扁平数据。当数据量超过一定规模（大概300万），数据关联复杂（表连接增多）后性能急剧下降。并且在高读写并发时锁表严重（MyISAM是表锁，InnoDB有行锁），甚至导致表损坏。DZ7.2代码中SQL写法存在不标准的问题，虽然不影响执行但对维护迁移是个问题。对数据类型检查也不严格，比如int字段插入的数据可能为空字符串，让mysql的兼容性来实现到0的自动转换。至于查询优化，这个因数据不同而实际变化很大没有一个完美的解决，不过优化做不好也不要拖后腿呀：有些SQL低级错误对数据库性能影响不小。也许SQL代码是由对数据库不了解的PHP程序员写的，不过也应该有懂数据库的人员来审查SQL相关代码的吧。有些低级失误很让人无语：本来可以用PHP代码完成的事情却要丢给数据库做，虽然节省了PHP代码不过却导致DB负载大幅度增加。

总体来说一个系统最慢的一环是在数据库，根源在于磁盘IO能力。数据库性能、反应决定了整个系统的负载能力。所以应该尽快结束数据库操作释放数据库资源，也避免PHP等待过久造成502错误（尤其是fastcgi模式）




---------------------


类型：        条件缺失
坑爹指数：    ★★★
代码：        member.php=64

    $order = isset($order) && in_array($order, array('credits','gender','username')) ? $order : '';

 

代码：        member.php=90

            switch($order) {
                case 'credits': $orderadd = "ORDER BY credits DESC"; break;
                case 'gender':     $orderadd = "ORDER BY gender DESC"; break;
                case 'username': $orderadd = "ORDER BY username DESC"; break;
                default: $orderadd = 'ORDER BY uid'; $order = 'uid'; break;
            }

 

点评：        统计选项->会员列表无法根据注册日期排序。
FIX：        line64修改为

$order = isset($order) && in_array($order, array('credits','gender','username', 'regdate')) ? $order : '';

 

line90:

            switch($order) {
                case 'credits': $orderadd = "ORDER BY credits DESC"; break;
                case 'gender':     $orderadd = "ORDER BY gender DESC"; break;
                case 'username': $orderadd = "ORDER BY username DESC"; break;
                case 'regdate': $orderadd = " ORDER BY regdate DESC"; break;    // ADD
                default: $orderadd = 'ORDER BY uid'; $order = 'uid'; break;
            }

 

---------------------


类型：        类型错误
坑爹指数：    ★★
代码：        admin/forums.inc.php~1289

    $query = $db->query("SELECT * FROM {$tablepre}threadtypes WHERE typeid IN ($typeids) AND special='' ORDER BY displayorder");

 

点评：        牛头不对马嘴，special字段明明是int类型却去搜索''空字符串，还好这个表不会大，不然坑死人不偿命

---------------------


类型：        负载分配
坑爹指数：    ★★★★★
代码：        admin/attach.inc.php=169

$db->query("UPDATE {$tablepre}threads SET attachment='0' WHERE tid IN ($tids)".($attachtids ? " AND tid NOT IN ($attachtids)" : NULL));

 

代码：        admin/attach.inc.php=176

$db->query("UPDATE {$tablepre}posts SET attachment='0' WHERE pid IN ($pids)".($attachpids ? " AND pid NOT IN ($attachpids)" : NULL));

 

点评：         懂点数据库的都知道除非万不得已否则应该避免使用“NOT IN”，使用的后果就是扫全表，如果数据量大磁盘性能再差点这一扫可是会扫出大菠萝的哟：UPDATE命令执行时间将会很长并且将导致长时间锁表从而阻塞住队列中的其他操作，最后导致SELECT都会严重阻塞，这时候网站基本就瘫痪了————页面刷新缓慢，发帖失去响应重复刷新结果就成复读机。所以应该均衡任务负荷，让数据库、PHP各自做擅长的工作而不是一股脑让一方完成所有。尤其数据库是整个系统中最慢那一块，应该避免让它陷入重负荷而是及时执行完毕释放资源否则它将会拖慢甚至拖垮系统。上面这语句其实可以分解成两步来执行：先执行一次SELECT查询取出数据作为要排除的部分与$tids数组做array_diff()运算，得出的结果再用作条件去执行UPDATE。虽然多了第一步查询，但这个查询是走搜索速度比全表扫快得多，总体下来性能提升明显。即便要在一条SQL命令中执行本可以使用子查询方式，不过mysql不支持EXCEPT/MINUS结果集操作……

---------------------------------------------------------------------------------------------

类型：        多余操作
坑爹指数：    ★★
代码：        admin/atttach.inc.php=165

$query = $db->query("SELECT tid FROM {$tablepre}attachments WHERE tid IN ($tids) GROUP BY tid ORDER BY pid DESC");

 

点评：         可使用“SELECT DISTINCT”来替代“GROUP BY”，可“ORDER BY”是啥意思呢？相关操作对结果集顺序并未有要求，多余的排序操作将会耗费CPU能力与内存占用，结果将增加数据库负载。只不过一般一个主题不会有海量附件，所以性能下降不明显。

---------------------------------------------------------------------------------------------

类型：        多余操作
坑爹指数：    ★★
代码：        recyclebin.inc.php=160

            do{
                $query = $db->query("SELECT f.name AS forumname, f.allowsmilies, f.allowhtml, f.allowbbcode, f.allowimgcode,
                    t.tid, t.fid, t.authorid, t.author, t.subject, t.views, t.replies, t.dateline,
                    p.message, p.useip, p.attachment, p.htmlon, p.smileyoff, p.bbcodeoff,
                    tm.uid AS moduid, tm.username AS modusername, tm.dateline AS moddateline, tm.action AS modaction
                    FROM {$tablepre}threads t
                    LEFT JOIN {$tablepre}posts p ON p.tid=t.tid AND p.first='1'
                    LEFT JOIN {$tablepre}threadsmod tm ON tm.tid=t.tid
                    LEFT JOIN {$tablepre}forums f ON f.fid=t.fid
                    WHERE t.displayorder='-1' $sql
                    GROUP BY t.tid ORDER BY t.dateline DESC LIMIT $ppp OFFSET ".(($pagetmp - 1) * $ppp));
                $pagetmp--;
            } while(!$query->rowCount() && $pagetmp);

 

点评：        "GROUP BY t.tid"是多余的，因为主表是threads tid是PK，上方line45还有一处类似。也许此段代码的大哥喜欢做菜。可厨艺不精，不知道什么时候该放什么调料，于是手边的调料瓶就都拿起来倒两下，只要味道不难吃这菜就算完成了。写代码也如此，估摸着写着写着忽然想起SQL还有“GROUP BY”的功能，随手拈来捣入SQL中搅和搅和，结果正确味道正好。遂顿悟，不会做菜的厨子不是个好程序猿 :D



---------------------------------------------------------------------------------------------

类型：        多余操作
坑爹指数：    ★
代码：        stats.php=217

        $query = $db->query("SELECT author, COUNT(*) AS posts FROM {$tablepre}posts WHERE dateline>='$timestamp'-86400 AND invisible='0' AND authorid>'0' GROUP BY author ORDER BY posts DESC LIMIT 1");

 

点评：        “AND authorid>'0'” 条件可以删除掉。这个条件毫无意义，只会让数据库在抓取row时过滤条件多一个结果却没差别。

---------------------------------------------------------------------------------------------


类型：        多余操作
坑爹指数：    ★★
代码：        include/requres.func.php

        $query = $db->query("SELECT t.tid,t.fid,t.readperm,t.author,t.authorid,t.subject,t.dateline,t.lastpost,t.lastposter,t.views,t.replies,t.highlight,t.digest,t.typeid,t.sortid
            $sqlfrom WHERE t.readperm='0'
            $sql
            AND t.displayorder>='0'
            AND t.fid>'0'            <--------
            $attachadd
            ORDER BY t.$orderby DESC
            LIMIT $items OFFSET $startrow "
            );

 

点评：        难道t.fid可以小于0？画蛇添足徒劳无功。可能这位老哥对于墨菲定律比较信服，越是怕fid小于0越是有可能出现，于是干脆把坑……唔，是地基挖深一些，避免出现意外 :)



---------------------------------------------------------------------------------------------


类型：        多余连接
坑爹指数：    ★★
代码：        include/post.func.php=602 updateforumcount()

extract($db->fetch_first("SELECT COUNT(*) AS threadcount, SUM(t.replies)+COUNT(*) AS replycount
FROM {$tablepre}threads t, {$tablepre}forums f
WHERE f.fid='$fid' AND t.fid=f.fid AND t.displayorder>='0'"));

 

点评：        其实没用到forums表的数据，对forums表的连接完全是多余的
FIX：

extract($db->fetch_first("SELECT COUNT(*) AS threadcount, SUM(replies)+COUNT(*) AS replycount
FROM {$tablepre}threads WHERE fid='$fid' AND displayorder>='0'"));

 

---------------------------------------------------------------------------------------------


类型：        条件模糊
坑爹指数：    ★
代码：        admin/counter.inc.php=80

$queryt = $db->query("SELECT uid FROM {$tablepre}members LIMIT $current, $pertask");

 

点评：        查询时SQL不严格未使用ORDER BY，导致结果集、结果顺序不确定。此页面多个SQL均存在这个问题, 会导致分页结果不可预料,尤其是提取帖子(精华)分页时！
FIX：

$queryt = $db->query("SELECT uid FROM {$tablepre}members ORDER BY uid LIMIT $current, $pertask");

 

---------------------------------------------------------------------------------------------


类型：        条件恶劣
坑爹指数：    ★★★★★
代码：        viewthread.php=354

        $specialadd2 .= "AND (dp.stand='0' OR dp.stand IS NULL OR p.first='1')";

 

代码：        viewthread.php=378

        $thread['replies'] = $sdb->result_first("SELECT COUNT(*) FROM {$tablepre}posts p LEFT JOIN {$tablepre}debateposts dp ON p.pid=dp.pid WHERE p.tid='$tid' AND (dp.stand='0' OR dp.stand IS NULL)");

 

代码：        include/task.func.php=134

        $nextnewbietaskid = intval($db->result_first("SELECT t.taskid FROM {$tablepre}tasks t LEFT JOIN {$tablepre}mytasks mt ON mt.taskid=t.taskid AND mt.uid='$discuz_uid' WHERE mt.taskid IS NULL AND t.available='2' AND t.newbietask='1' ORDER BY t.newbietask DESC LIMIT 1"));

 

点评：        会数据库的应该知道NULL值不会走索引，除非建立ISNULL索引，作NULL查询将会扫全表导致性能暴跌! DZ数据库建表风格是都采用NOT NULL约束，PHP代码风格也是不做NULL的判断。在字段已经明确NOT NULL约束条件下还采用(dp.stand='0' OR dp.stand IS NULL)这样条件，对mt.taskid不使用mt.taskid>0判断，如果不是临时工干的那就基本上是存心考古的……

FIX:

        $nextnewbietaskid = intval($db->result_first("SELECT t.taskid FROM {$tablepre}tasks t LEFT JOIN {$tablepre}mytasks mt ON mt.taskid=t.taskid AND mt.uid='$discuz_uid' WHERE mt.taskid = 0 AND t.available='2' AND t.newbietask='1' ORDER BY t.newbietask DESC LIMIT 1"));

 

---------------------------------------------------------------------------------------------


类型：        条件恶劣
坑爹指数：    ★★★★★
代码：        ucs/control/admin/pm.php~150 onclear()

    $uids = 0;

 

代码：        admin/prune.inc.php~220

    $forums = '0';

 

代码：        admin/prune.inc.php~230

    $uids = '-1';

 

代码：        viewthreads.php~220

    $attachpids = -1;

 

代码：        topicadmin.php~102 前台删除帖子

    $pids = 0;

 

代码：        topicadmin.php~109 前台删除帖子

    $pids .= ','.$post['pid'];

 

代码：        admin/threads.inc.php~622

    $tids = 0;

 

代码：        admin/forums.inc.php~1289

    $query = $db->query("SELECT * FROM {$tablepre}threadtypes WHERE typeid IN ($typeids) AND special='' ORDER BY displayorder");

 

代码：        modcp/moderate.inc.php~286

    WHERE pid IN (0,".implode(',', $pidarray).")");

 

代码：        admin/moderate.inc.php=727

    $db->query("UPDATE {$tablepre}posts SET invisible='0' WHERE pid IN (0,".implode(',', $pidarray).")");

 

代码：        include/misc.func.php~289

$db->query("UPDATE $tablepre$table SET $viewscol=$viewscol+'$views' WHERE $idcol IN (0$ids)" );

 

代码：

    $str = $comma = '';
    foreach (..) {
        $str .= $comma. 'something';
        $comma = ',';
    }

 

点评：        不知道为啥，对于搜索id，DZ代码风格是先给$id变量赋值个不可能的值（比如0或者''），然后在迭代中对此变量拼接字符串。这将会在两个方面影响性能。一，如果迭代结果并无真实id被追加，那么因为$id因为非空所以依旧会做一次无结果的查询。白白浪费数据库连接资源和PHP资源；二，即便有真实id需要查询，虽然$id包含了不可能值（比如0,-1）但这个不可能值依旧会被用作合法的查询条件值，结果是额外开销。我不确定是否会导致更多数据库性能开销：通常都是在PK上查询，走的索引自然是UNIQUE————一个值有匹配即停止对该值的继续查找判断，当最后一个值有索引匹配就停止搜索————如果存在一个合法的不可能的值将会导致数据库扫完整个索引来匹配该值！如果我对数据库索引搜索工作方式判断正确，那么DZ这个附加不可能值SQL条件的做法将是相当影响性能非常坑爹的，因为这种风格在DZ7.2代码中很常见。



---------------------------------------------------------------------------------------------


类型：        条件恶劣
坑爹指数：    ★★★★★
代码：        forumdisplay.php=317

    $forumstickycount = $stickycount = $stickytids = 0;

 

点评：        对tid搜索包含0, 版块精华SQL类似如下，将会导致扫全索引. 并且影响到即使没有全局置顶主题也会做同样查询，非常坑爹

  SELECT t.* FROM cdb_threads t
            WHERE t.tid IN (0,110) AND t.displayorder IN (2, 3, 4)
            ORDER BY displayorder DESC, lastpost DESC
            LIMIT  1 OFFSET 0

 

FIX：     在line338 if(($start_limit && $start_limit > $stickycount) || !$stickycount || $filterbool) {
之前加上过滤：

if ($stickytids) {
    $tarr = array();
    $stickytids = explode(',', $stickytids);
    foreach ($stickytids as $s_id) {
        $s_id = intval($s_id) && $s_id > 0 && $tarr[] = $s_id;
    }
    $stickytids = implode(',', $tarr);
    unset($tarr);
}
else {
    $stickytids = '';
}

 

然后line348:

   $querysticky = $sdb->query("SELECT t.* FROM {$tablepre}threads t
        WHERE t.tid IN ($stickytids) AND t.displayorder IN (2, 3, 4)
        ORDER BY displayorder DESC, $orderby $ascdesc
        LIMIT $start_limit, ".($stickycount - $start_limit < $tpp ? $stickycount - $start_limit : $tpp));

 

修改成：

if ($stickytids) {
        $querysticky = $sdb->query("SELECT t.* FROM {$tablepre}threads t
            WHERE t.tid IN ($stickytids) AND t.displayorder IN (2, 3, 4)
            ORDER BY displayorder DESC, $orderby $ascdesc
            LIMIT  ".($stickycount - $start_limit < $tpp ? $stickycount - $start_limit : $tpp). ' OFFSET '. $start_limit);
    }
    else {
        $querysticky = false;
    }

 

---------------------------------------------------------------------------------------------

 

类型：        流程问题
坑爹指数：    ★★★★★
代码：        search.php=166+
点评：        DZ搜索是在实际搜索前先对cdb_searchindex的进行查询来判断是否存存在flood以及是否存在相同搜索（条件），每个用户两次搜索间隔判断，每分钟服务器接受搜索阈值判断都是在此表上实现。此方式极大缺陷：是执行了查询之后再根据结果判断是否flood，而并非把flood与否作为条件去查询。也就是说无论是否flood，任何查询都会先走一次cdb_searchindex扫描————即使DZ系统提示你“两次搜索时间过短”让你待会儿再搜索，这只是减轻了对获得期望结果的数据库表的压力而丝毫不会减轻cdb_searchindex的压力！攻击者可以持续提交查询数据让cdb_searchindex表查询压力巨大从而影响数据库性能，尤其是在长时间运行的系统上，cdb_searchindex表缓存的查询数据越多越明显。

---------------------------------------------------------------------------------------------


类型：        多余连接
坑爹指数：    ★★★
代码：        include/requres.func.php~450

          case 'weekposts':
                $week = gmdate('w', $timestamp) - 1;
                $week = $week != -1 ? $week : 6;
                $historytime = mktime(0, 0, 0, date('m', $timestamp), date('d', $timestamp) - $week, date('Y', $timestamp));
                $sql = "SELECT DISTINCT (p.author) AS username,p.authorid AS uid,COUNT(p.pid) AS postnum FROM {$tablepre}posts p LEFT JOIN {$tablepre}memberfields mf ON mf.uid = p.authorid WHERE p.dateline>=$historytime GROUP BY p.authorid, p.author ORDER BY postnum DESC";

 

点评：        输出字段并未使用memberfields表中字段，貌似MySQL在这种情况下并不会因此而放弃连接操作，对此表的连接徒耗资源。

 

 

 

---------------------------------------------------------------------------------------------

类型：        逻辑错误
坑爹指数：    ★
代码：        admin/recyclebin.inc.php~145

            $threadcount = $db->result_first("SELECT count(*)
                FROM {$tablepre}threads t
                LEFT JOIN {$tablepre}threadsmod tm ON tm.tid=t.tid
                WHERE t.displayorder='-1' $sql");

 

点评：        此SQL未对threadsmod作如下do while()循环中类似的GROUP BY处理，如果一个回收站中主题被反复删除恢复那么就会出现“符合条件的回收站主题数”不为0但下方无列表显示这种情况

FIX1：        添加一条GROUP BY语句过滤threadsmod的结果。但限于MySQL不标准的GROUP BY语法，其结果（顺序）也许非正确

            $threadcount = $db->result_first("SELECT count(*)
                FROM {$tablepre}threads t
                LEFT JOIN {$tablepre}threadsmod tm ON tm.tid=t.tid
                GROUP BY tm.tid ORDER BY tm.dateline DESC, t.dateline DESC
                WHERE t.displayorder='-1' $sql");

 

 

 

 

版权曾经拥有，欢迎网上分享

转载请保留链接 http://waiting.iteye.com/blog/1378554