Project

General

Profile

Actions

Bug #34152

closed

stdWrap function numRows() fails in 4.6 due to wrong SELECT clause

Added by Alain almost 13 years ago. Updated about 6 years ago.

Status:
Closed
Priority:
Should have
Category:
Frontend
Target version:
-
Start date:
2012-02-21
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
4.5
PHP Version:
5.3
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

Hello,

this piece of TS does not work anymore since an update from 4.5.11 to 4.6.4:

    ATagParams.cObject = TEXT
    ATagParams.cObject {
            value = class="hasSubpage" 
        if.isTrue.numRows {
                table = pages
            select {
                pidInList.field = uid
                where = doktype IN (1,199)
            }
        }
    }

While tracking tslib_cObj::getQuery(), I found out that the sanitizeSelectPart() is introcuced since 4.6. This uses the PHP function strpos() to check if wildcard is set in the SELECT clause, which leads to wrong SQL query in the special case of tslib_cObj::numRows(), where 'count(*)' is set in selectFields property.

The retrieved SQL query would be:

SELECT     count(*), pages.uid as uid, pages.pid as pid, pages.t3ver_state as t3ver_state
FROM     pages
WHERE     pages.pid IN (6) AND nav_hide!=1 AND doktype!=5 AND doktype!=6 AND pages.deleted=0 AND pages.hidden=0 AND pages.starttime<=1329827460 AND (pages.endtime=0 OR pages.endtime>1329827460) AND NOT pages.t3ver_state>0 AND pages.doktype<200 AND (pages.fe_group='' OR pages.fe_group IS NULL OR pages.fe_group='0' OR FIND_IN_SET('0',pages.fe_group) OR FIND_IN_SET('-1',pages.fe_group))

but the expected SQL query should be:

SELECT     count(*)
FROM     pages
WHERE     pages.pid IN (6) AND nav_hide!=1 AND doktype!=5 AND doktype!=6 AND pages.deleted=0 AND pages.hidden=0 AND pages.starttime<=1329827460 AND (pages.endtime=0 OR pages.endtime>1329827460) AND NOT pages.t3ver_state>0 AND pages.doktype<200 AND (pages.fe_group='' OR pages.fe_group IS NULL OR pages.fe_group='0' OR FIND_IN_SET('0',pages.fe_group) OR FIND_IN_SET('-1',pages.fe_group))

I would suggest to exclude "count(*)" in the if statement of tslib_cObj::sanitizeSelectPart():

    protected function sanitizeSelectPart($selectPart, $table) {
        // pattern matching parts
        $matchStart = '/(^\s*|,\s*|' . $table . '\.)';
        $matchEnd = '(\s*,|\s*$)/';

        $necessaryFields = array('uid', 'pid');
        $wsFields = array('t3ver_state');

-        if (isset($GLOBALS['TCA'][$table]) && strpos($selectPart, '*')!== FALSE) {
+        if (isset($GLOBALS['TCA'][$table]) && strpos($selectPart, '*')!== FALSE && $selectPart != 'count(*)') {
            foreach ($necessaryFields as $field) {
                $match = $matchStart . $field . $matchEnd;
                if (!preg_match($match, $selectPart)) {
                    $selectPart .= ', ' . $table . '.' . $field . ' as ' . $field;
                }
            }

            if ($GLOBALS['TCA'][$table]['ctrl']['versioningWS']) {
                foreach ($wsFields as $field) {
                    $match = $matchStart . $field . $matchEnd;
                    if (!preg_match($match, $selectPart)) {
                        $selectPart .= ', ' . $table . '.' . $field . ' as ' . $field;
                    }
                }
            }
        }
        return $selectPart;
    }

Thank you and best regards,

Alain


Related issues 4 (0 open4 closed)

Related to TYPO3 Core - Bug #17284: no content in workspace preview when using select.selectFieldsClosedBenni Mack2007-05-07

Actions
Related to TYPO3 Core - Bug #29783: getQuery: fields explicilty added on select <table>.* : Error: ambigious parameter uidClosedBjörn Pedersen2011-10-11

Actions
Related to TYPO3 Core - Bug #39634: Solved Bug #34152 results in a new interpretation of my TypoScriptRejectedMichael Stucki2012-08-09

Actions
Related to TYPO3 Core - Bug #43114: sanitizeSelectPart - select CONTENT using DISTINCTClosed2012-11-19

Actions
Actions #1

Updated by Stefan Galinski almost 13 years ago

Sounds like you already solved the bug. Can you create a patch against the master branch and open a new review request please? I also added Helmut as watcher as this seems to be security related!?

http://wiki.typo3.org/Contribution_Walkthrough_Tutorials

Actions #2

Updated by Alain almost 13 years ago

Hello,

thanks for your feedback. I am not yet familiar with git & gerrit so I need time to learn more about, sorry. At the moment, I would appreciate if someone else can create a patch for this bug.

Best regards,

Alain

Actions #3

Updated by Gerrit Code Review almost 13 years ago

  • Status changed from New to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #4

Updated by Georg Ringer almost 13 years ago

hi,

i tested this with current master

page.20 = TEXT
page.20 {
          value = class="hasSubpage" 
    if.isTrue.numRows {
            table = pages
      select {
        pidInList.field = uid
        where = doktype IN (1,199)
      }
    }
}

and it puts out class="hasSubpage" if there is a subpage, IMO this works. can you check the master?

Actions #5

Updated by Gerrit Code Review almost 13 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #6

Updated by Wouter Wolters almost 13 years ago

Tested in master and 4.6 installation and the given example is working.

Actions #7

Updated by Alain almost 13 years ago

I have cloned the master branch from repository git://git.typo3.org/TYPO3v4/Core.git to my local machine and I can reproduce it in 4.7-dev. I'm now not sure if this issue is system specific. I'm running Debian Etch, MySQL 5.0.32, PHP 5.3.4. I had never problem with this constellation since 4.2 until 4.5.

Actions #8

Updated by Markus Klein almost 13 years ago

Alain, in the ticket you say TYPO3-Version 4.6.
Now you say "..never had problem ... until 4.5"

To clarify things: Affected versions 4.5+ according to you?

Actions #9

Updated by Alain almost 13 years ago

the affected version is 4.6.4.

To clarify what I meant by "I had never problem with this constellation since 4.2 until 4.5", let me say in german:

Seit 4.2 bis einschließlich 4.5 hatte ich nie Probleme mit meiner System-Konstellation. Das o.g. TS-Snippet funktioniert bei mir in 4.5.11 tadellos.
Ich hatte in meinem englischen Satz das Wort inclusively vergessen, sorry.

The TS snippet that I've mentionned above, works great in 4.5.11. But since I updated to 4.6.4, I am confronted with this issue.

Meanwhile, I have tested the retrieved SQL query on shell command and I get an error:

mysql> use typo3db;
Database changed

mysql> SELECT count(*), pages.uid as uid, pages.pid as pid, pages.t3ver_state as t3ver_state FROM pages WHERE pages.pid IN (6) AND nav_hide!=1 AND doktype!=5 AND doktype!=6 AND pages.deleted=0 AND pages.hidden=0 AND pages.starttime<=1329827460 AND (pages.endtime=0 OR pages.endtime>1329827460) AND NOT pages.t3ver_state>0 AND pages.doktype<200 AND (pages.fe_group='' OR pages.fe_group IS NULL OR pages.fe_group='0' OR FIND_IN_SET('0',pages.fe_group) OR FIND_IN_SET('-1',pages.fe_group));

ERROR 1140 (42000): Mixing of GROUP columns (MIN(),MAX(),COUNT(),...) with no GROUP columns is illegal if there is no GROUP BY clause
Actions #10

Updated by Alain almost 13 years ago

Patch Set 2 available at http://review.typo3.org/9158 solves this issue. Tested on my current 4.6 installation and on master 4.7-dev.

Actions #11

Updated by Maik Matthias over 12 years ago

Same problem with TYPO3 version 4.5.13. The provided patch works fine with 4.5.13 too. IMHO fixing this bug is a "must have" because the numRows function is very common.

Actions #12

Updated by TO_Webmaster no-lastname-given over 12 years ago

Yeah, please fix this in 4.5.

I also suggest to check for other aggregate functions, too.

Actions #13

Updated by Marcus Krause over 12 years ago

We're also affected, using 4-5 branch (LTS).

Actions #14

Updated by Gerrit Code Review over 12 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #15

Updated by Ernesto Baschny over 12 years ago

  • Category set to Frontend
  • TYPO3 Version changed from 4.6 to 4.5

This problem also affects TYPO3_4-5, since the fix #17284 was also backported to the 4.5 branch and released with 4.5.13. So it needs to be fixed there as well (regression).

Please note that the numRows problem only seem to affect MySQL 5.0, as with MySQL 5.1 and later this is not considered wrong SQL anymore, which is why the problem is probably not that "wide-spread".

MySQL 5.0:

mysql> select count(*),uid from pages;
ERROR 1140 (42000): Mixing of GROUP columns (MIN(),MAX(),COUNT(),...) with no GROUP columns is illegal if there is no GROUP BY clause

MySQL 5.1:

mysql> select count(*),uid from pages;
-------------+ | count(*) | uid |
-------------+ | 769 | 1 |
-------------+
1 row in set (0.00 sec)
Actions #16

Updated by Manfred Egger over 12 years ago

Please consider to make

$selectPart != 'count(*)'
case insensitive because
count(*)
is often written as
COUNT(*)

Actions #17

Updated by Gerrit Code Review over 12 years ago

Patch set 4 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #18

Updated by Gerrit Code Review over 12 years ago

Patch set 5 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #19

Updated by Gerrit Code Review over 12 years ago

Patch set 6 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #20

Updated by Gerrit Code Review over 12 years ago

Patch set 7 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #21

Updated by Gerrit Code Review over 12 years ago

Patch set 8 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #22

Updated by Gerrit Code Review over 12 years ago

Patch set 9 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #23

Updated by Fedir RYKHTIK over 12 years ago

4.5 +1

patch works fine

Actions #24

Updated by Jan Simbera over 12 years ago

4.5 (4.5.17) +1

patch working

Actions #25

Updated by BenBaghira no-lastname-given over 12 years ago

4.5 (4.5.17) +1

patch working

Actions #26

Updated by Gerrit Code Review over 12 years ago

Patch set 10 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #27

Updated by Ernesto Baschny over 12 years ago

  • Assignee set to Ernesto Baschny
  • % Done changed from 0 to 50

Ok, the latest patch 10 now adds a unit test for the "getQuery" method which has been broken and fixed over and over. It includes test cases for the relevant issues (#17284, #29783, #34152).

Please review so that we can get that into the whole "chain" of releases (4.5 .. 6.0) soon.

Thanks!

Actions #28

Updated by Gerrit Code Review over 12 years ago

Patch set 11 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #29

Updated by Gerrit Code Review over 12 years ago

Patch set 12 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #30

Updated by Gerrit Code Review over 12 years ago

Patch set 13 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158

Actions #31

Updated by Ernesto Baschny over 12 years ago

Please note that the "rowNums" problem due to wrong SQL only pops up with MySQL 5.0 (e.g. debian lenny), so you won't be able to reproduce the bug in MySQL 5.1 (debian squeeze).

Test with this snippet:

page.1 = TEXT
page.1.value = TEST
page.1.if.isTrue.numRows {
table = pages
select {
pidInList.field = uid
where = doktype IN (1,199)
}
}

You should see "TEST" on top of your page if the current page has subpages. If you never see TEST, it means the SQL failed.

Actions #32

Updated by Gerrit Code Review over 12 years ago

Patch set 1 for branch TYPO3_4-7 has been pushed to the review server.
It is available at http://review.typo3.org/12997

Actions #33

Updated by Gerrit Code Review over 12 years ago

Patch set 1 for branch TYPO3_4-6 has been pushed to the review server.
It is available at http://review.typo3.org/12998

Actions #34

Updated by Gerrit Code Review over 12 years ago

Patch set 1 for branch TYPO3_4-5 has been pushed to the review server.
It is available at http://review.typo3.org/12999

Actions #35

Updated by Ernesto Baschny over 12 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 50 to 100
Actions #36

Updated by Gerrit Code Review over 12 years ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch TYPO3_4-7 has been pushed to the review server.
It is available at http://review.typo3.org/13202

Actions #37

Updated by Ernesto Baschny over 12 years ago

  • Status changed from Under Review to Resolved
Actions #38

Updated by Gerrit Code Review over 12 years ago

  • Status changed from Resolved to Under Review

Patch set 1 for branch TYPO3_4-6 has been pushed to the review server.
It is available at http://review.typo3.org/13309

Actions #39

Updated by Stefan Froemken over 12 years ago

Since you have pushed the solution for this report to current TYPO3 4.6.11 there are problems with TypoScript-parts using "select". Here is my problem which I have posted into the core list, too:

The field "uid" is not the "uid" out of the select-part anymore. My TS returns following records:

224 Tceforms // not linked
225 Ctrl // not linked
69 fluid // not linked
14 Warum jQuery? // linked
2 Home // linked
255 Form // not linked
252 Buttons // not linked

But the Link "Home" shows to Record "index.php?id=40" instead of "2"
and the link "Warum jQuery?" shows to record index.php?id=8" instead of "14"

Here is my TypoScript:

10 = CONTENT
10 {
table = tt_content
select {
orderBy = MAX DESC
groupBy = tt_content.pid
pidInList = 1
recursive = 10
max = 7
where = nav_hide = 0
selectFields = pages.uid, pages.title
leftjoin = pages ON (tt_content.pid = pages.uid)
}

renderObj = TEXT
renderObj {
field = title
required = 1
typolink.parameter.field = uid // maybe here is the problem
typolink.ATagParams = class=internal-link
wrap = &lt;li&gt;|&lt;/li&gt;
}
wrap = &lt;h2&gt;Meine zuletzt bearbeiteten Inhalte&lt;/h2&gt;&lt;ul&gt;|&lt;/ul&gt;
}

And here is the resulting Query:

SELECT pages.uid, pages.title
FROM tt_content
LEFT JOIN pages ON ( tt_content.pid = pages.uid )
WHERE nav_hide =0
GROUP BY tt_content.pid
ORDER BY MAX DESC
LIMIT 7

Actions #40

Updated by Markus Klein over 12 years ago

@Stefan: Not a bug! See #39634.

Actions #41

Updated by Oliver Hader over 12 years ago

  • Status changed from Under Review to Resolved

Closing issue due to notes in issue #39634 and accordant discussion in the core mailing list (thread "Announcing TYPO3 4.5.18, 4.6.11 and 4.7.3")

Actions #42

Updated by Benni Mack about 6 years ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF