feat(identifier): denormalize base_type onto identifier table

[wezell]

Summary

• Adds base_type INT4 column + idx_identifier_base_type index to the identifier table, mirroring structure.structuretype
• Identifier bean, IdentifierFactoryImpl, and IdentifierTransformer updated to read/write base_type on every save
• Task260331AddBaseTypeColumnToIdentifier adds the column and index at startup (DDL only — fast), then schedules the backfill job
• PopulateIdentifierBaseTypeJob / PopulateIdentifierBaseTypeUtil backfill existing rows in batches of 500 (250 ms sleep between batches), fire immediately at startup, and self-unschedule when complete
• postgres.sql updated for fresh installs

Test plan

• Task260331AddBaseTypeColumnToIdentifierTest — column/index created, idempotent, correct metadata
• PopulateIdentifierBaseTypeUtilTest — sets correct base type per content type, ignores folders (null asset_subtype), returns 0 on second run
• PopulateIdentifierBaseTypeJobTest — schedules, skips when no work pending, removes itself
• IdentifierFactoryTest (5 new methods) — base_type persisted for CONTENT and FILEASSET, survives re-save, transformer maps it, folders get null

fixes #35162

[claude]

Claude finished @wezell's task in 3m 11s —— View job

---

PR Review: feat(identifier): denormalize base_type onto identifier table

• Read changed files
• Analyze code for bugs, risks, and design issues

---

3 issues worth a second look:

1. saveIdentifier mutates the caller's Identifier object as a side effect

IdentifierFactoryImpl.java:537

id.setBaseType(resolvedBaseType);

The stale-cache guard calls id.setBaseType() on the Identifier reference passed into saveIdentifier(). Callers don't expect saveIdentifier to modify their object's baseType. If the same instance is held elsewhere (e.g., in a local variable the caller reads after the save), it will silently have a different value. The mutation should either be documented on the method signature or avoided by returning the resolved value without touching the argument.

Fix this →

---

2. populate() blocks its Quartz thread until the entire migration completes

PopulateIdentifierBaseTypeUtil.java:80–103

populate() loops until zero rows remain, sleeping 250 ms between 500-row batches. For a customer with 5M content identifiers, that's ~10,000 batches × ~250 ms sleep = 41+ minutes holding one Quartz worker thread. If the thread pool is small and this job fires at startup alongside other jobs, it can starve them.

The HOURS_INTERVAL config (default 4 h) only matters for interrupted runs; a complete run never yields the thread mid-way. Either document this clearly as a known constraint, or split the loop so each Quartz execution processes one batch and reschedules itself, letting the scheduler manage the cadence.

---

3. processBatch() acquires a raw connection without explicit rollback on exception

PopulateIdentifierBaseTypeUtil.java:116–124

try (Connection conn = DbConnectionFactory.getDataSource().getConnection()) {
    conn.setAutoCommit(false);
    int result = new DotConnect().executeUpdate(conn, UPDATE_BATCH);
    conn.commit();
    return result;
} catch (final Exception e) {
    throw new DotRuntimeException(...);
}

On exception, the try-with-resources closes the connection without explicitly rolling back. Most JDBC drivers and pools will roll back uncommitted transactions on close, but it's not guaranteed by the spec and behavior varies across pool implementations. An explicit conn.rollback() in the catch before re-throwing makes the intent clear and is defensive against pool misbehavior.

Fix this →

---

Minor observations (not blocking):

• BATCH_SIZE is a static final initialized at class load from Config.getIntProperty(). If the config system isn't fully bootstrapped when the class is first loaded, you'll silently get the default 500. Consider lazy initialization (Lazy.of(...)) consistent with how HOURS_INTERVAL is handled in PopulateIdentifierBaseTypeJob.
• The test_populate_correctlyMapsMultipleBaseTypes test calls DbConnectionFactory.getConnection().setAutoCommit(true) directly. This can interfere with other tests' transaction management if they share the same connection. Check whether this is actually needed or can be removed.
• hasPendingRows() is called twice in a single run() execution (lines 55 and 67). Each is a DB round-trip. On busy clusters this is fine, but it's worth noting the result between the two calls is not guaranteed consistent (a concurrent save could fill the last rows between them). The existing comment on line 66 implicitly acknowledges this — just confirming the logic is still correct under that race.

Overall the design is sound — the DDL-only startup task, batched backfill job with self-unscheduling, and stale-cache guard are all solid patterns for this kind of denormalization.