From 5a9b3ab510a62fbcc4ef6f0edea5bb7b39c1fa6a Mon Sep 17 00:00:00 2001 From: Yewon Kwak Date: Wed, 26 Aug 2026 21:01:43 +0000 Subject: [PATCH] Add 8.4 and 9.x reserved keywords to the upgrade check The reserved keywords check only knew about words reserved up to 8.0.31, and wasn't registered for any later version, so it missed newer keywords and didn't run at all for upgrades starting past 8.0.31 (e.g. 8.0 to 8.4). Add the words reserved since then -- QUALIFY and TABLESAMPLE (8.4.0), LIBRARY (9.2.0) and EXTERNAL (9.4.0) -- and register the check for those crossings. MANUAL and PARALLEL were reserved in 8.4.0 but nonreserved again in 8.4.11, so add_keywords gains an optional upper-bound version to report them only below 8.4.11. Adds unit tests for the generated keyword list (including the 8.4.11 boundary) and the new registry crossings. This contribution is under the OCA signed by Amazon and covering submissions to the MySQL project. --- .../upgrade_checker/upgrade_check_creators.cc | 10 ++- .../upgrade_checker/upgrade_check_registry.cc | 3 +- .../upgrade_check_creators_t.cc | 61 +++++++++++++++++++ .../upgrade_check_registry_t.cc | 10 ++- 4 files changed, 79 insertions(+), 5 deletions(-) diff --git a/modules/util/upgrade_checker/upgrade_check_creators.cc b/modules/util/upgrade_checker/upgrade_check_creators.cc index a15928a00..0326f9456 100644 --- a/modules/util/upgrade_checker/upgrade_check_creators.cc +++ b/modules/util/upgrade_checker/upgrade_check_creators.cc @@ -65,9 +65,11 @@ std::unique_ptr get_old_temporal_check() { std::unique_ptr get_reserved_keywords_check( const Upgrade_info &info) { std::string keywords; - const auto add_keywords = [&keywords, &info](const char *v, const char *kws) { + const auto add_keywords = [&keywords, &info](const char *v, const char *kws, + const char *max_v = nullptr) { Version kv(v); - if (info.server_version < kv && info.target_version >= kv) { + if (info.server_version < kv && info.target_version >= kv && + (max_v == nullptr || info.target_version < Version(max_v))) { if (!keywords.empty()) keywords += ", "; keywords += kws; } @@ -83,6 +85,10 @@ std::unique_ptr get_reserved_keywords_check( add_keywords("8.0.17", "'ARRAY' ,'MEMBER'"); add_keywords("8.0.31", "'FULL', 'INTERSECT'"); + add_keywords("8.4.0", "'QUALIFY', 'TABLESAMPLE'"); + add_keywords("8.4.0", "'MANUAL', 'PARALLEL'", "8.4.11"); + add_keywords("9.2.0", "'LIBRARY'"); + add_keywords("9.4.0", "'EXTERNAL'"); keywords = "(" + keywords + ");"; return std::make_unique( diff --git a/modules/util/upgrade_checker/upgrade_check_registry.cc b/modules/util/upgrade_checker/upgrade_check_registry.cc index 5c9f78e38..796792be9 100644 --- a/modules/util/upgrade_checker/upgrade_check_registry.cc +++ b/modules/util/upgrade_checker/upgrade_check_registry.cc @@ -80,7 +80,8 @@ namespace { [[maybe_unused]] bool register_reserved = Upgrade_check_registry::register_check(&get_reserved_keywords_check, Target::OBJECT_DEFINITIONS, "8.0.11", - "8.0.14", "8.0.17", "8.0.31"); + "8.0.14", "8.0.17", "8.0.31", "8.4.0", + "9.2.0", "9.4.0"); [[maybe_unused]] bool register_utf8mb3 = Upgrade_check_registry::register_check( std::bind(&get_utf8mb3_check), Target::OBJECT_DEFINITIONS, "8.0.11"); diff --git a/unittest/modules/util/upgrade_checker/upgrade_check_creators_t.cc b/unittest/modules/util/upgrade_checker/upgrade_check_creators_t.cc index 13ca300ed..7da616cf6 100644 --- a/unittest/modules/util/upgrade_checker/upgrade_check_creators_t.cc +++ b/unittest/modules/util/upgrade_checker/upgrade_check_creators_t.cc @@ -215,5 +215,66 @@ TEST(Upgrade_check_creators, } } +TEST(Upgrade_check_creators, get_reserved_keywords_check_test) { + // Every per-object-type query produced by the reserved keywords check embeds + // the same keyword IN-list, so inspecting the first query is enough to verify + // which keywords are included for a given source -> target version pair. + const auto keyword_list = [](const Version &server, const Version &target) { + const auto info = upgrade_info(server, target); + const auto check = get_reserved_keywords_check(info); + return check->get_queries().front().first; + }; + const auto has = [](const std::string &query, const char *keyword) { + return query.find(keyword) != std::string::npos; + }; + + // 8.0 -> 8.4.8: QUALIFY/TABLESAMPLE plus MANUAL/PARALLEL, which are still + // reserved below 8.4.11. LIBRARY/EXTERNAL do not apply yet (target < 9.x). + { + const auto query = keyword_list(Version(8, 0, 0), Version(8, 4, 8)); + EXPECT_TRUE(has(query, "'QUALIFY'")); + EXPECT_TRUE(has(query, "'TABLESAMPLE'")); + EXPECT_TRUE(has(query, "'MANUAL'")); + EXPECT_TRUE(has(query, "'PARALLEL'")); + EXPECT_FALSE(has(query, "'LIBRARY'")); + EXPECT_FALSE(has(query, "'EXTERNAL'")); + } + + // 8.0 -> 8.4.11: MANUAL/PARALLEL became nonreserved in 8.4.11 and must drop + // out (upper bound of the ranged add_keywords call); QUALIFY/TABLESAMPLE stay. + { + const auto query = keyword_list(Version(8, 0, 0), Version(8, 4, 11)); + EXPECT_TRUE(has(query, "'QUALIFY'")); + EXPECT_TRUE(has(query, "'TABLESAMPLE'")); + EXPECT_FALSE(has(query, "'MANUAL'")); + EXPECT_FALSE(has(query, "'PARALLEL'")); + } + + // 8.4.0 -> 9.2.0: LIBRARY (reserved since 9.2.0) applies; EXTERNAL (9.4.0) + // does not. The 8.4.0 words are not re-reported since the source is 8.4.0. + { + const auto query = keyword_list(Version(8, 4, 0), Version(9, 2, 0)); + EXPECT_TRUE(has(query, "'LIBRARY'")); + EXPECT_FALSE(has(query, "'EXTERNAL'")); + EXPECT_FALSE(has(query, "'QUALIFY'")); + EXPECT_FALSE(has(query, "'MANUAL'")); + } + + // 8.4.0 -> 9.4.0: both LIBRARY (9.2.0) and EXTERNAL (9.4.0) apply. + { + const auto query = keyword_list(Version(8, 4, 0), Version(9, 4, 0)); + EXPECT_TRUE(has(query, "'LIBRARY'")); + EXPECT_TRUE(has(query, "'EXTERNAL'")); + } + + // 9.2.0 -> 9.4.0: LIBRARY is already reserved on the source, so only EXTERNAL + // is newly reserved on the target. + { + const auto query = keyword_list(Version(9, 2, 0), Version(9, 4, 0)); + EXPECT_TRUE(has(query, "'EXTERNAL'")); + EXPECT_FALSE(has(query, "'LIBRARY'")); + } +} + } // namespace upgrade_checker } // namespace mysqlsh diff --git a/unittest/modules/util/upgrade_checker/upgrade_check_registry_t.cc b/unittest/modules/util/upgrade_checker/upgrade_check_registry_t.cc index 2a880a331..8b3d4b70a 100644 --- a/unittest/modules/util/upgrade_checker/upgrade_check_registry_t.cc +++ b/unittest/modules/util/upgrade_checker/upgrade_check_registry_t.cc @@ -380,14 +380,20 @@ TEST(Upgrade_check_registry, create_checklist) { {{v5_7_0, Version(8, 0, 11)}, {Version(8, 0, 11), Version(8, 0, 14)}, {Version(8, 0, 14), Version(8, 0, 17)}, - {Version(8, 0, 17), Version(8, 0, 31)}}); + {Version(8, 0, 17), Version(8, 0, 31)}, + {Version(8, 0, 31), Version(8, 4, 0)}, + {Version(8, 4, 0), Version(9, 2, 0)}, + {Version(9, 2, 0), Version(9, 4, 0)}, + {Version(8, 0, 31), vShell}}); test_check_availability(ids::k_reserved_keywords_check, false, {{v5_7_0, Version(8, 0, 10)}, {Version(8, 0, 11), Version(8, 0, 13)}, {Version(8, 0, 14), Version(8, 0, 16)}, {Version(8, 0, 17), Version(8, 0, 30)}, - {Version(8, 0, 31), vShell}}); + {Version(8, 4, 0), Version(8, 4, 11)}, + {Version(8, 4, 1), Version(9, 1, 0)}, + {Version(9, 4, 0), vShell}}); // syntax_check: // available: always between series versions