From 39200596f48e44067021474adeefeeccf3dc3a13 Mon Sep 17 00:00:00 2001 From: RoyBenMoshe <112958533+RoyBenMoshe@users.noreply.github.com> Date: Tue, 25 Nov 2025 09:30:43 +0200 Subject: [PATCH] SCAN: restore original filter order (#14537) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In #14121, the SCAN filters order was changed, before #14121the order was - pattern, expiration and type, after #14121pattern became last, this break change broke the original behavior, which will cause scan with pattern also to remove the expired keys. This PR reorders the filters to be consistent with the original behavior and extends a test to cover this scenario. --- src/db.c | 28 +++++++++++++++------------- tests/unit/scan.tcl | 27 +++++++++++++++------------ 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/src/db.c b/src/db.c index 50b229d0e..b9a66efb0 100644 --- a/src/db.c +++ b/src/db.c @@ -1504,9 +1504,22 @@ void scanCallback(void *privdata, const dictEntry *de, dictEntryLink plink) { /* o and typename can not have values at the same time. */ serverAssert(!((data->type != LLONG_MAX) && o)); + kvobj *kv = NULL; if (!o) { /* If scanning keyspace */ - kvobj *kv = dictGetKV(de); - + kv = dictGetKV(de); + keyStr = kvobjGetKey(kv); + } else { + keyStr = dictGetKey(de); + } + + /* Filter element if it does not match the pattern. */ + if (data->pattern) { + if (!stringmatchlen(data->pattern, sdslen(data->pattern), keyStr, data->strlen(keyStr), 0)) { + return; + } + } + + if (!o) { /* Expiration check first - only for database keyspace scanning. * Use kv obj to avoid robj creation. */ if (expireIfNeeded(data->db, NULL, kv, 0) != KEY_VALID) @@ -1521,17 +1534,6 @@ void scanCallback(void *privdata, const dictEntry *de, dictEntryLink plink) { if (!objectTypeCompare(kv, data->type)) return; } - - keyStr = kvobjGetKey(kv); - } else { - keyStr = dictGetKey(de); - } - - /* Filter element if it does not match the pattern. */ - if (data->pattern) { - if (!stringmatchlen(data->pattern, sdslen(data->pattern), keyStr, data->strlen(keyStr), 0)) { - return; - } } if (o == NULL) { diff --git a/tests/unit/scan.tcl b/tests/unit/scan.tcl index 8ab14d3b1..6a092cb4e 100644 --- a/tests/unit/scan.tcl +++ b/tests/unit/scan.tcl @@ -164,25 +164,29 @@ proc test_scan {type} { r debug set-active-expire 1 } {OK} {needs:debug} - test "{$type} SCAN with expired keys with TYPE filter" { + test "{$type} SCAN with expired keys with TYPE filter and PATTERN filter" { r flushdb # make sure that passive expiration is triggered by the scan r debug set-active-expire 0 populate 1000 - r set foo bar - r pexpire foo 1 + r set key:foo bar + r pexpire key:foo 1 # add a hash type key - r hset hash f v - r pexpire hash 1 + r hset key:hash f v + r pexpire key:hash 1 + + # add a pattern key + r set boo far + r pexpire boo 1 after 2 set cur 0 set keys {} while 1 { - set res [r scan $cur type "string" count 10] + set res [r scan $cur type "string" match key* count 10] set cur [lindex $res 0] set k [lindex $res 1] lappend keys {*}$k @@ -191,12 +195,11 @@ proc test_scan {type} { assert_equal 1000 [llength $keys] - # make sure that expired key have been removed by scan command - assert_equal 1000 [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d] - # TODO: uncomment in redis 8.0 - # make sure that only the expired key in the type match will been removed by scan command - #assert_equal 1001 [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d] - + # make sure that expired key have been removed by scan command, + # pattern check before expired so key filtered by pattern will not be removed + # but expiration check is before type check so key:foo and key:hash will be removed + assert_equal 1001 [scan [regexp -inline {keys\=([\d]*)} [r info keyspace]] keys=%d] + r debug set-active-expire 1 } {OK} {needs:debug}