Bug #38838

False PageCacheTimeout / getFirstTimeValueForRecord calculation with start-/stoptimes on multiple content elements.

Added by Alexander Opitz almost 9 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Should have
Category:
Caching
Target version:
Start date:
2012-07-10
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
4.6
PHP Version:
Tags:
Complexity:
easy
Is Regression:
No
Sprint Focus:

Description

If you have 2 content elements with start-/stoptimes in the feature getFirstTimeValueForRecord returns false values.

Setup:

- A page with two content elements.
- first content element has a starttime but no endtime
- second content element has a endtime but no starttime

MySQL Statement issued:

SELECT MIN AS starttime, MIN AS endtime
FROM tt_content
WHERE pid =19325
AND (
starttime >1341908220
OR endtime >1341908220
)
AND tt_content.deleted =0
AND tt_content.t3ver_state <=0
AND tt_content.pid <> -1
AND tt_content.hidden =0
AND (
tt_content.fe_group = ''
OR tt_content.fe_group IS NULL
OR tt_content.fe_group = '0'
OR FIND_IN_SET( '0', tt_content.fe_group )
OR FIND_IN_SET( '-1', tt_content.fe_group )
)
... (simplified)

The where statement now selects 2 content elements.
- first
-- starttime: value in future
-- endtime: 0
- second
-- starttime: 0
-- endtime: value in future

The min function for starttime and endtime results in
- starttime: 0
- endtime: 0

The later if statement validates if this values are greater then $now and so getFirstTimeValueForRecord returns PHP_INT_MAX

Fix:

The min select statement should be:

SELECT MIN ) AS starttime, MIN ) AS endtime

so the 0 values won't be respected.


Files

a.diff (1.16 KB) a.diff Alexander Opitz, 2012-07-10 11:05

Related issues

Is duplicate of TYPO3 Core - Bug #35684: Cache duration wrongly calculated with associated recordsClosedFrancois Suter2012-04-04

Actions
#1

Updated by Alexander Opitz almost 9 years ago

Original:

SELECT MIN( starttime ) AS starttime, MIN( endtime ) AS endtime
FROM tt_content
WHERE pid =19325
AND (
starttime >1341908220
OR endtime >1341908220
)
AND tt_content.deleted =0
AND tt_content.t3ver_state <=0
AND tt_content.pid <> -1
AND tt_content.hidden =0
AND (
tt_content.fe_group = ''
OR tt_content.fe_group IS NULL
OR tt_content.fe_group = '0'
OR FIND_IN_SET( '0', tt_content.fe_group )
OR FIND_IN_SET( '-1', tt_content.fe_group )
)
AND (
tt_content.tx_aidadrecountryxs_countryxs = ''
OR (
tt_content.tx_aidadrecountryxs_includecountries =0
AND ! FIND_IN_SET( 1, tt_content.tx_aidadrecountryxs_countryxs )
)
OR (
tt_content.tx_aidadrecountryxs_includecountries =1
AND FIND_IN_SET( 1, tt_content.tx_aidadrecountryxs_countryxs )
)
)
AND (
(
tt_content.tx_aidadrecountryxs_fegroups_invert =0
AND tt_content.tx_aidadrecountryxs_fegroups = ''
)
OR (
tt_content.tx_aidadrecountryxs_fegroups_invert =1
AND tt_content.tx_aidadrecountryxs_fegroups != "" 
)
)
LIMIT 1 

Fixed Select:

SELECT MIN( if( starttime =0, NULL , starttime ) ) AS starttime, MIN( if( endtime =0, NULL , endtime ) ) AS endtime

#2

Updated by Alexander Opitz almost 9 years ago

Patch against Typo3 4.6.8 typo3 directory.
The problem may exists also in Typo3 4.5 and 4.7

#3

Updated by Susanne Moog almost 9 years ago

  • Status changed from New to Accepted
  • Complexity set to easy
#4

Updated by Francois Suter over 8 years ago

Beware that the code should be DBAL-compatible. Is that the case?

This is such a sensitive check, it would be great to have unit tests to cover it.

#5

Updated by Alexander Opitz over 8 years ago

How to check the DBAL compatibility?

#6

Updated by Alexander Opitz about 8 years ago

Short speed test on homepage of an introduction package installation:

Test: ab -c 100 -n 1000 http://domain/

Before:

Concurrency Level: 100
Time taken for tests: 18.564 seconds
Complete requests: 1000
Failed requests: 0
Write errors: 0
Total transferred: 10403000 bytes
HTML transferred: 9966000 bytes
Requests per second: 53.87 [#/sec] (mean)
Time per request: 1856.449 [ms] (mean)
Time per request: 18.564 [ms] (mean, across all concurrent requests)
Transfer rate: 547.24 [Kbytes/sec] received

After:

Concurrency Level: 100
Time taken for tests: 18.066 seconds
Complete requests: 1000
Failed requests: 0
Write errors: 0
Total transferred: 10403000 bytes
HTML transferred: 9966000 bytes
Requests per second: 55.35 [#/sec] (mean)
Time per request: 1806.581 [ms] (mean)
Time per request: 18.066 [ms] (mean, across all concurrent requests)
Transfer rate: 562.34 [Kbytes/sec] received

#7

Updated by Alexander Opitz about 8 years ago

The select query

SELECT
MIN(if( starttime = 0, NULL , starttime )) AS starttime, MIN(if( endtime = 0, NULL , endtime )) AS endtime
FROM
tt_content
WHERE
pid=1 AND (starttime>1362128340 OR endtime>1362128340) AND tt_content.deleted=0 AND tt_content.t3ver_state<=0 AND tt_content.pid<>-1 AND tt_content.hidden=0 AND (tt_content.fe_group='' OR tt_content.fe_group IS NULL OR tt_content.fe_group='0' OR FIND_IN_SET('0',tt_content.fe_group) OR FIND_IN_SET('-1',tt_content.fe_group))

worked in the DBAL Debug SQL check

#8

Updated by Gerrit Code Review about 8 years ago

  • Status changed from Accepted to Under Review

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

#9

Updated by Gerrit Code Review about 8 years ago

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

#10

Updated by Gerrit Code Review about 8 years ago

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

#11

Updated by Gerrit Code Review about 8 years ago

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

#12

Updated by Gerrit Code Review about 8 years ago

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

#13

Updated by Dmitry Dulepov over 7 years ago

  • Is Regression set to No

The issue is not reproducible in TYPO3 4.7 or newer with two content elements as described above. getFirstTimeValueForRecord() returns time correctly. There is no need for this change.

#14

Updated by Christian Kuhn over 6 years ago

  • Status changed from Under Review to New

The pending patch was blocked for a long time without any reaction by the blocking person.

I've now abandoned the pending patch and reset the issue back to new. The issue is still valid and should be re-pushed again with a functional test showing the correct result.

#15

Updated by Christian Kuhn over 6 years ago

  • TYPO3 Version changed from 4.6 to 6.2
#16

Updated by Alexander Opitz over 6 years ago

  • Assignee set to Alexander Opitz
  • TYPO3 Version changed from 6.2 to 4.6

The issue exists since 4.6 so no change of TYPO3 Version is needed.

#17

Updated by Alexander Opitz almost 6 years ago

  • Status changed from New to In Progress
  • Target version set to 7.5
#18

Updated by Benni Mack over 5 years ago

  • Target version changed from 7.5 to 7 LTS
#19

Updated by Gerrit Code Review over 5 years ago

  • Status changed from In Progress to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43613

#20

Updated by Gerrit Code Review over 5 years ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43613

#21

Updated by Gerrit Code Review over 5 years ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43613

#22

Updated by Gerrit Code Review over 5 years ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43613

#23

Updated by Gerrit Code Review over 5 years ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43613

#24

Updated by Gerrit Code Review over 5 years ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at http://review.typo3.org/43613

#25

Updated by Anonymous over 5 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#26

Updated by Riccardo De Contardi over 3 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF