mk-oracle: report non-CDB Oracle jobs in the legacy row format

andiumbreit · andiumbreit · commit 3d03699798b2 · 2026-08-12T22:14:55.000+02:00
A CDB jobs row carries eleven fields and a non-CDB row ten, and no single
statement produces both, so a non-CDB was reported in the CDB shape. The 12.1
query is therefore tagged CDB-only, and a non-CDB falls through to the ten-field
query, which matches the legacy output.

Tagging the query CDB-only also makes its fallback branch for a non-CDB
unreachable, so that branch is removed here. It matched a con_id of 0, which only
a non-CDB reports.

Two further divergences from legacy are fixed on the way.

- the CDB query mapped :IGNORE_DB_NAME = 1 to the database name, so by default
  it keyed the item on the instance name where legacy keys it on the database
  name
- a job of class SCHED$_LOG_ON_ERRORS_CLASS kept the status of its last run
  across a restart, in both arms, which turned a legacy WARN into a permanent
  CRIT

TESTS: Manual and automated. Unit tests cover the tenant split from the section
name down to the selected statement, and cover both arms for the item key and
for the log-on-errors class. Confirmed against real Oracle databases in Docker,
using 12.1 as a non-CDB and 19.3 and 23.26 as CDBs. The non-CDB now emits
ten-field rows with a bare database item, and the CDBs keep their eleven fields.

CMK-37363

Change-Id: I4602011274c7d19afefa6597364b17877d605687
diff --git a/packages/mk-oracle/BUILD b/packages/mk-oracle/BUILD
@@ -64,7 +64,7 @@ rust_library(
         "sqls/instance.12010002.all.sql",
         "sqls/io_stats.0.all.sql",
         "sqls/jobs.10020000.all.sql",
-        "sqls/jobs.12010000.all.sql",
+        "sqls/jobs.12010000.cdb.sql",
         "sqls/locks.10020000.all.sql",
         "sqls/locks.12010000.all.sql",
         "sqls/logswitches.0.all.sql",
diff --git a/packages/mk-oracle/sqls/jobs.10020000.all.sql b/packages/mk-oracle/sqls/jobs.10020000.all.sql
@@ -31,7 +31,13 @@ SELECT UPPER(
                    TO_DATE('1970-01-01', 'YYYY-MM-DD'))
        )                         AS next_run_date,    -- Next scheduled run (defaulted if null)
        NVL(j.schedule_name, '-') AS schedule_name,    -- Schedule name associated with the job (or '-' if none)
-       jd.status                                      -- Status of the most recent run (e.g., SUCCEEDED, FAILED, STOPPED)
+       -- Blank a log-on-errors job that restarted since its last log row:
+       -- the previous outcome is stale, not its current status.
+       CASE
+           WHEN j.job_class = 'SCHED$_LOG_ON_ERRORS_CLASS'
+               AND j.last_start_date > jd.actual_start_date THEN ''
+           ELSE jd.status
+           END                                        -- Status of the most recent run (e.g., SUCCEEDED, FAILED, STOPPED)
 FROM dba_scheduler_jobs j -- Data dictionary view of all Scheduler jobs
          JOIN v$database vd ON 1 = 1 -- Database metadata (name, role, mode)
          JOIN v$instance i ON 1 = 1 -- Instance metadata (instance name)
diff --git a/packages/mk-oracle/sqls/jobs.12010000.cdb.sql b/packages/mk-oracle/sqls/jobs.12010000.cdb.sql
@@ -29,12 +29,18 @@ SELECT vp.instance_name,                              -- Instance name (or DB na
                    TO_DATE('1970-01-01', 'YYYY-MM-DD'))
        )                         AS next_run_date,    -- Next scheduled run date/time (defaulted if null)
        NVL(j.schedule_name, '-') AS schedule_name,    -- Associated schedule name (or '-' if none)
-       jd.status                                      -- Status of the most recent run (SUCCEEDED, FAILED, STOPPED, etc.)
+       -- Blank a log-on-errors job that restarted since its last log row:
+       -- the previous outcome is stale, not its current status.
+       CASE
+           WHEN j.job_class = 'SCHED$_LOG_ON_ERRORS_CLASS'
+               AND j.last_start_date > jd.actual_start_date THEN ''
+           ELSE jd.status
+           END                                        -- Status of the most recent run (SUCCEEDED, FAILED, STOPPED, etc.)
 FROM cdb_scheduler_jobs j -- CDB view of scheduler jobs across all PDBs
 -- Join with Subquery: maps container ID to instance and container names
          JOIN (SELECT c.con_id,
                       UPPER(DECODE(NVL(:IGNORE_DB_NAME, 0),
-                                   1, d.NAME, -- If :IGNORE_DB_NAME = 1 → show DB name
+                                   0, d.NAME, -- If :IGNORE_DB_NAME = 0 → show DB name
                                    i.instance_name -- Else → show instance name
                             )) AS instance_name,
                       c.name   AS container_name -- PDB name
@@ -46,16 +52,6 @@ FROM cdb_scheduler_jobs j -- CDB view of scheduler jobs across all PDBs
                  AND c.con_id <> 2               -- Exclude seed PDB (con_id = 2)
                  AND d.database_role = 'PRIMARY' -- Only primary DB (exclude standby)
                  AND d.open_mode = 'READ WRITE'  -- Only open databases
-               UNION ALL
-               -- Handles non-CDB-like representation (fallback case)
-               SELECT 0, d.name, c.name
-               FROM v$database d
-                        JOIN v$instance i
-                             ON i.con_id = d.con_id
-                        LEFT JOIN v$containers c
-                                  ON c.dbid = d.dbid
-               WHERE d.database_role = 'PRIMARY'
-                 AND d.open_mode = 'READ WRITE'
               ) vp
               ON j.con_id = vp.con_id
 -- Join with Subquery: get latest run log ID per job
diff --git a/packages/mk-oracle/src/ora_sql/instance.rs b/packages/mk-oracle/src/ora_sql/instance.rs
@@ -1123,6 +1123,44 @@ oracle:
         );
     }
 
+    // CMK-37363: derivation (v$database.cdb -> Tenant) + plumbing + selection.
+    // Four numeric components, or the version parses to None and this is vacuous.
+    #[test]
+    fn test_jobs_non_cdb_selects_ten_field_query() {
+        let yaml = one_builtin_section_yaml("jobs");
+        let statements = selected_statements(&yaml, "TESTDB", "19.28.0.0.0", "NO");
+
+        assert_eq!(statements.len(), 1);
+        assert!(
+            statements[0].contains("dba_scheduler_jobs"),
+            "{}",
+            statements[0]
+        );
+        assert!(
+            !statements[0].contains("container_name"),
+            "{}",
+            statements[0]
+        );
+    }
+
+    #[test]
+    fn test_jobs_cdb_keeps_container_column() {
+        let yaml = one_builtin_section_yaml("jobs");
+        let statements = selected_statements(&yaml, "TESTCDB", "19.28.0.0.0", "YES");
+
+        assert_eq!(statements.len(), 1);
+        assert!(
+            statements[0].contains("cdb_scheduler_jobs"),
+            "{}",
+            statements[0]
+        );
+        assert!(
+            statements[0].contains("container_name"),
+            "{}",
+            statements[0]
+        );
+    }
+
     // CMK-37361: the item expression must branch on d.cdb, otherwise a non-CDB
     // (con_id 0) gets the item `TESTDB.TESTDB` instead of `TESTDB`.
     #[test]
diff --git a/packages/mk-oracle/src/ora_sql/section.rs b/packages/mk-oracle/src/ora_sql/section.rs
@@ -1048,6 +1048,66 @@ oracle:
         assert!(!pats[0].is_match("PDB4"));
     }
 
+    /// The factory statements of the builtin section `name`. Search dirs are
+    /// empty on purpose, so no stray on-disk SQL file can shadow them.
+    fn builtin_queries(name: &str, version: u32, tenant: Tenant) -> Vec<String> {
+        Section::new(
+            &section::SectionBuilder::new(name).build(),
+            0,
+            &Options::default(),
+        )
+        .find_queries_with_search_dirs(InstanceNumVersion::from(version), tenant, &[], &[])
+        .unwrap_or_else(|| panic!("builtin section '{name}' must resolve"))
+        .iter()
+        .map(|q| q.as_str().to_owned())
+        .collect()
+    }
+
+    // CMK-37363: covers the whole hop section name -> Id -> get_factory_query.
+    #[test]
+    fn test_jobs_section_query_selected_per_tenant() {
+        let no_cdb = builtin_queries(names::JOBS, 19_01_00_00, Tenant::NoCdb);
+        assert_eq!(no_cdb.len(), 1);
+        assert!(no_cdb[0].contains("dba_scheduler_jobs"), "{}", no_cdb[0]);
+        assert!(!no_cdb[0].contains("v$containers"), "{}", no_cdb[0]);
+
+        let cdb = builtin_queries(names::JOBS, 19_01_00_00, Tenant::Cdb);
+        assert_eq!(cdb.len(), 1);
+        assert!(cdb[0].contains("cdb_scheduler_jobs"), "{}", cdb[0]);
+        assert!(cdb[0].contains("v$containers"), "{}", cdb[0]);
+    }
+
+    // Legacy keys the item on the database name by default
+    // (agents/plugins/mk_oracle), and both arms must agree with it.
+    #[test]
+    fn test_jobs_both_tenants_key_the_item_on_the_db_name_by_default() {
+        for tenant in [Tenant::Cdb, Tenant::NoCdb] {
+            let sql = builtin_queries(names::JOBS, 19_01_00_00, tenant).join("\n");
+            // Flattened, so neither reindentation nor a CRLF checkout matters.
+            let flat = sql.split_whitespace().collect::<Vec<_>>().join(" ");
+            assert!(
+                flat.contains("DECODE(NVL(:IGNORE_DB_NAME, 0), 0,"),
+                "{tenant:?} arm does not map IGNORE_DB_NAME=0 to the DB name: {sql}"
+            );
+        }
+    }
+
+    // Without this the check goes permanently CRIT where legacy warned.
+    #[test]
+    fn test_jobs_both_tenants_blank_a_restarted_log_on_errors_job() {
+        for tenant in [Tenant::Cdb, Tenant::NoCdb] {
+            let sql = builtin_queries(names::JOBS, 19_01_00_00, tenant).join("\n");
+            assert!(
+                sql.contains("SCHED$_LOG_ON_ERRORS_CLASS"),
+                "{tenant:?} arm does not special-case the log-on-errors class: {sql}"
+            );
+            assert!(
+                sql.contains("j.last_start_date > jd.actual_start_date"),
+                "{tenant:?} arm does not compare the restart against the recorded run: {sql}"
+            );
+        }
+    }
+
     fn split_trimmed(sql: &str) -> Vec<&str> {
         split_sql_statements(sql)
             .into_iter()
diff --git a/packages/mk-oracle/src/ora_sql/sqls.rs b/packages/mk-oracle/src/ora_sql/sqls.rs
@@ -88,9 +88,9 @@ pub mod query {
     }];
     pub const JOBS_META: &[RawMetadata] = &[
         RawMetadata {
-            sql: include_str!("../../sqls/jobs.12010000.all.sql"),
+            sql: include_str!("../../sqls/jobs.12010000.cdb.sql"),
             min_version: 12010000,
-            tenant: Tenant::All,
+            tenant: Tenant::Cdb,
         },
         RawMetadata {
             sql: include_str!("../../sqls/jobs.10020000.all.sql"),
@@ -502,6 +502,97 @@ mod tests {
         assert!(query_nothing.is_err());
         assert_ne!(query_old, query_new);
         assert_eq!(query_last, query_new);
+        // A non-CDB must never get the 11-field CDB query, even at version `None`.
+        assert_eq!(find_helper(id, 0, Tenant::NoCdb).unwrap(), query_old);
+        // A pre-12.1 CDB must still resolve.
+        assert_eq!(find_helper(id, 11020000, Tenant::Cdb).unwrap(), query_old);
+    }
+
+    /// The only section whose non-CDB shape differs in arity.
+    #[test]
+    fn test_find_jobs_tenant_split() {
+        let cdb = find_helper(Id::Jobs, 19010000, Tenant::Cdb).unwrap();
+        let no_cdb = find_helper(Id::Jobs, 19010000, Tenant::NoCdb).unwrap();
+
+        assert_ne!(cdb, no_cdb);
+        assert!(cdb.contains("cdb_scheduler_jobs"), "{cdb}");
+        assert!(no_cdb.contains("dba_scheduler_jobs"), "{no_cdb}");
+        assert!(!no_cdb.contains("container_name"), "{no_cdb}");
+        assert!(!no_cdb.contains("v$containers"), "{no_cdb}");
+    }
+
+    /// A tenant tag must never make a whole section vanish.
+    #[test]
+    fn test_every_section_resolves_for_both_tenants() {
+        const ALL_IDS: &[Id] = &[
+            Id::IoStats,
+            Id::TsQuotas,
+            Id::Jobs,
+            Id::Resumable,
+            Id::UndoStat,
+            Id::RecoveryArea,
+            Id::AsmDiskGroup,
+            Id::Locks,
+            Id::LogSwitches,
+            Id::LongActiveSessions,
+            Id::Processes,
+            Id::RecoveryStatus,
+            Id::Rman,
+            Id::Sessions,
+            Id::SystemParameter,
+            Id::TableSpaces,
+            Id::DataGuardStats,
+            Id::Instance,
+            Id::AsmInstance,
+            Id::Performance,
+        ];
+        assert_eq!(ALL_IDS.len(), QUERY_MAP.len(), "new Id not covered here");
+        for id in ALL_IDS {
+            for version in [12_01_00_00u32, 19_01_00_00, 21_03_00_00, 23_08_00_25] {
+                for tenant in [Tenant::Cdb, Tenant::NoCdb] {
+                    assert!(
+                        find_helper(*id, version, tenant).is_ok(),
+                        "{id:?} does not resolve at {version} for {tenant:?}"
+                    );
+                }
+            }
+        }
+    }
+
+    /// Tripwire for the shadowing rule in `build_query_metadata`. No section
+    /// declares two entries at one min_version today, so the body does not run.
+    #[test]
+    fn test_no_all_entry_shares_min_version_with_a_tenant_entry() {
+        for (id, metas) in QUERY_MAP.iter() {
+            for (a, b) in metas
+                .iter()
+                .enumerate()
+                .flat_map(|(i, a)| metas[i + 1..].iter().map(move |b| (a, b)))
+            {
+                if a.min_version != b.min_version {
+                    continue;
+                }
+                assert!(
+                    a.tenant != Tenant::All && b.tenant != Tenant::All && a.tenant != b.tenant,
+                    "{id:?} has ambiguous entries at min_version {}: {:?} and {:?}",
+                    a.min_version,
+                    a.tenant,
+                    b.tenant
+                );
+            }
+        }
+    }
+
+    /// ASM instances resolve as `NoCdb`, so these must resolve for both tenants.
+    #[test]
+    fn test_asm_reachable_sections_are_tenant_agnostic() {
+        for id in [Id::AsmDiskGroup, Id::AsmInstance, Id::Processes] {
+            assert_eq!(
+                find_helper(id, 19010000, Tenant::NoCdb).unwrap(),
+                find_helper(id, 19010000, Tenant::Cdb).unwrap(),
+                "{id:?} must not depend on the tenant"
+            );
+        }
     }
     #[test]
     fn test_find_resumable() {
diff --git a/packages/mk-oracle/tests/test_ora_no_db.rs b/packages/mk-oracle/tests/test_ora_no_db.rs
@@ -958,11 +958,14 @@ mod custom_path_tests {
 
     /// First resolved query body for `section` at `version`, using `search_dirs`
     /// as the relative-`path:` search roots (irrelevant for absolute paths).
+    ///
+    /// `Cdb`, not `All`: an `All` argument matches only `All`-tagged entries and
+    /// would bypass the tenant-specific jobs query.
     fn first_query(section: &Section, version: u32, search_dirs: &[PathBuf]) -> Option<String> {
         section
             .find_queries_with_search_dirs(
                 InstanceNumVersion::from(version),
-                Tenant::All,
+                Tenant::Cdb,
                 &[],
                 search_dirs,
             )
diff --git a/packages/mk-oracle/tests/test_ora_with_db.rs b/packages/mk-oracle/tests/test_ora_with_db.rs
