Bug #34152
closedstdWrap function numRows() fails in 4.6 due to wrong SELECT clause
100%
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
Updated by Stefan Galinski over 12 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!?
Updated by Alain over 12 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
Updated by Gerrit Code Review over 12 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
Updated by Georg Ringer over 12 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?
Updated by Gerrit Code Review over 12 years ago
Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/9158
Updated by Wouter Wolters over 12 years ago
Tested in master and 4.6 installation and the given example is working.
Updated by Alain over 12 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.
Updated by Markus Klein over 12 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?
Updated by Alain over 12 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
Updated by Alain over 12 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.
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.
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.
Updated by Marcus Krause over 12 years ago
We're also affected, using 4-5 branch (LTS).
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
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)
Updated by Manfred Egger over 12 years ago
Please consider to make
$selectPart != 'count(*)'case insensitive because
count(*)is often written as
COUNT(*)
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
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
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
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
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
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
Updated by BenBaghira no-lastname-given over 12 years ago
4.5 (4.5.17) +1
patch working
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
Updated by Ernesto Baschny over 12 years ago
- Assignee set to Ernesto Baschny
- % Done changed from 0 to 50
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
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
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
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.
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
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
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
Updated by Ernesto Baschny over 12 years ago
- Status changed from Under Review to Resolved
- % Done changed from 50 to 100
Applied in changeset 8e944f09c49ad51b03c7492f0f0647a7b9ab1c88.
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
Updated by Ernesto Baschny over 12 years ago
- Status changed from Under Review to Resolved
Applied in changeset b846f3d219e2280b2b967b7f1a3c017060815125.
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
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 = <li>|</li>
}
wrap = <h2>Meine zuletzt bearbeiteten Inhalte</h2><ul>|</ul>
}
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
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")