Catalog factory array: misleading swapped in-class initializer, [6]/[5] declaration mismatch, unchecked GetCatalogFactory indexing

[liunyl]

Summary

Three related code-health hazards around the per-engine CatalogFactory* array. None is an active bug today (the wrong values are dead code / unused paths), but each is a trap for the next change.

Details

1. Swapped slots in a dead in-class initializer. tx_service/include/cc/local_cc_shards.h:2089:

   CatalogFactory *catalog_factory_[5]{nullptr, nullptr, nullptr,
                                       &hash_catalog_factory_,    // slot 3 = TableEngine::InternalRange!
                                       &range_catalog_factory_};  // slot 4 = TableEngine::InternalHash!

   The constructor's init list (tx_service/src/cc/local_cc_shards.cpp:112-116) initializes the same member correctly (..., &range_catalog_factory_, &hash_catalog_factory_) and always overrides the in-class default — so the swapped values never take effect, but they actively mislead readers and become live the moment someone adds a constructor that forgets the member.

2. Declaration/definition array-size mismatch. CcShard's constructor parameter is declared CatalogFactory *catalog_factory[6] (tx_service/include/cc/cc_shard.h:297) while the member and all real arrays are [5] (cc_shard.h:1404, local_cc_shards.h:2089). Array parameters decay so it compiles, but the 6 is wrong documentation in the signature.

3. Unchecked indexing. CcShard::GetCatalogFactory returns catalog_factory_[static_cast<int>(table_engine) - 1] (cc_shard.h:548) with no bounds check — TableEngine::None (0) indexes [-1], UB if ever passed.

Suggested fix

Fix the in-class initializer order (or drop it in favor of the ctor list alone), change the parameter to [5] (or std::array<CatalogFactory*, 5>& to get type checking), and add an assert/guard for TableEngine::None in GetCatalogFactory.

---

Found during the module-docs review (#493); see docs/05-data-model-and-catalog.md Gotchas.

🤖 Found with Claude Code