From ff1def0a3323e5373e85a3c5fc595fb7c3957185 Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Sat, 20 Jun 2026 11:46:09 +0200 Subject: [PATCH 01/10] Add support for schema_view_fragment pragma to return binary blobs of selected schemas --- iModelCore/ECDb/ECDb/ECDbImpl.cpp | 1 + iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp | 109 ++++++++++++ iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h | 20 +++ iModelCore/ECDb/ECDb/SchemaViewWriter.cpp | 125 ++++++++++---- iModelCore/ECDb/ECDb/SchemaViewWriter.h | 31 +++- .../Tests/NonPublished/ECSqlPragmasTests.cpp | 158 ++++++++++++++++++ 6 files changed, 413 insertions(+), 31 deletions(-) diff --git a/iModelCore/ECDb/ECDb/ECDbImpl.cpp b/iModelCore/ECDb/ECDb/ECDbImpl.cpp index d8f1469dd8b..0e39c489721 100644 --- a/iModelCore/ECDb/ECDb/ECDbImpl.cpp +++ b/iModelCore/ECDb/ECDb/ECDbImpl.cpp @@ -268,6 +268,7 @@ void ECDb::Impl::RegisterECSqlPragmas() const GetPragmaManager().Register(PragmaECSqlVersion::Create()); GetPragmaManager().Register(PragmaSqliteSql::Create()); GetPragmaManager().Register(PragmaSchemaView::Create()); + GetPragmaManager().Register(PragmaSchemaViewFragment::Create()); } //-------------------------------------------------------------------------------------- diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp index af15c155c4e..dc425bbd628 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp @@ -1034,6 +1034,115 @@ DbResult PragmaSchemaView::Write(PragmaManager::RowSet& rowSet, ECDbCR ecdb, Pra return BE_SQLITE_READONLY; } +//======================================================================================= +// PragmaSchemaViewFragment +//======================================================================================= +namespace { +// True only for a non-empty run of ASCII decimal digits. +bool IsAllDigits(Utf8StringCR s) { + if (s.empty()) + return false; + for (char c : s) { + if (c < '0' || c > '9') + return false; + } + return true; +} +} + +//--------------------------------------------------------------------------------------- +// @bsimethod +//--------------------------------------------------------------------------------------- +DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ecdb, PragmaVal const& val, PragmaManager::OptionsMap const& options) { + auto reportError = [&](Utf8StringCR message) -> DbResult { + ecdb.GetImpl().Issues().ReportV(IssueSeverity::Error, IssueCategory::BusinessProperties, IssueType::ECSQL, ECDbIssueId::ECDb_0601, + "PRAGMA %s: %s", GetName().c_str(), message.c_str()); + rowSet = std::make_unique(ecdb); + rowSet->FreezeSchemaChanges(); + return BE_SQLITE_ERROR; + }; + + if (!val.IsString()) + return reportError("expects a single string argument: a comma-separated list of decimal ec_Schema ids, optionally prefixed with a 'v;' format-version token."); + + Utf8String arg(val.GetString().c_str()); + + // Optional leading 'v;' format-version token. The version is carried inside the one string + // argument because the pragma grammar allows only a single value; see the schema_view_fragment + // reference doc for the rationale. + uint8_t requestedVersion = CURRENT_FORMAT_VERSION; + Utf8String idList = arg; + size_t const semicolon = arg.find(';'); + if (semicolon != Utf8String::npos) { + Utf8String versionToken = arg.substr(0, semicolon); + idList = arg.substr(semicolon + 1); + if (versionToken.size() < 2 || (versionToken[0] != 'v' && versionToken[0] != 'V')) + return reportError("malformed version prefix; expected 'v;' (for example 'v1;')."); + Utf8String digits = versionToken.substr(1); + if (!IsAllDigits(digits)) + return reportError("malformed version prefix; expected 'v;' with N a positive integer."); + int64_t v = (int64_t)strtoll(digits.c_str(), nullptr, 10); + if (v < 1 || v > CURRENT_FORMAT_VERSION) + return reportError(Utf8PrintfString("unsupported format version %" PRId64 ". Supported versions: 1-%d.", v, (int)CURRENT_FORMAT_VERSION)); + requestedVersion = (uint8_t)v; + } + + // Parse the comma-separated decimal id list into a de-duplicated set; reject any malformed or + // repeated id rather than silently emit a partial fragment. + bvector tokens; + BeStringUtilities::Split(idList.c_str(), ",", tokens); + std::unordered_set schemaIds; + for (Utf8StringR token : tokens) { + token.Trim(); + if (!IsAllDigits(token)) + return reportError(Utf8PrintfString("invalid schema id '%s'; ids must be positive decimal integers.", token.c_str())); + int64_t id = (int64_t)strtoll(token.c_str(), nullptr, 10); + if (!schemaIds.insert(id).second) + return reportError(Utf8PrintfString("duplicate schema id %" PRId64 " in the request.", id)); + } + if (schemaIds.empty()) + return reportError("the schema id list is empty."); + + SchemaViewWriter writer; + DbResult writeResult = writer.WriteSchemas(ecdb, schemaIds); + if (writeResult == BE_SQLITE_NOTFOUND) + return reportError("one or more requested schema ids do not exist in this connection."); + if (writeResult != BE_SQLITE_OK) + return writeResult; + auto const& output = writer.GetOutput(); + + auto result = std::make_unique(ecdb); + result->AppendProperty("format", PRIMITIVETYPE_String); + result->AppendProperty("formatVersion", PRIMITIVETYPE_Integer); + result->AppendProperty("data", PRIMITIVETYPE_String); + result->AppendProperty("schemaToken", PRIMITIVETYPE_String); + result->FreezeSchemaChanges(); + + // schemaToken is the whole-connection schema hash, identical to schema_view's, so a fragment and + // the manifest it was planned from share one cache-invalidation key. + Utf8String schemaToken; + SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA, "main", SHA3Helper::HashSize::SHA3_256); + + auto row = result->AppendRow(); + row.appendValue() = "binary"; + row.appendValue() = (int64_t)requestedVersion; + row.appendValue().SetBinary(output.data(), output.size()); + row.appendValue() = schemaToken.c_str(); + + rowSet = std::move(result); + return BE_SQLITE_OK; +} + +//--------------------------------------------------------------------------------------- +// @bsimethod +//--------------------------------------------------------------------------------------- +DbResult PragmaSchemaViewFragment::Write(PragmaManager::RowSet& rowSet, ECDbCR ecdb, PragmaVal const&, PragmaManager::OptionsMap const& options) { + ecdb.GetImpl().Issues().ReportV(IssueSeverity::Error, IssueCategory::BusinessProperties, IssueType::ECSQL, ECDbIssueId::ECDb_0552, "PRAGMA %s is readonly.", GetName().c_str()); + rowSet = std::make_unique(ecdb); + rowSet->FreezeSchemaChanges(); + return BE_SQLITE_READONLY; +} + END_BENTLEY_SQLITE_EC_NAMESPACE diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h index bb8b3c35fbe..94fc904f0f5 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h @@ -172,6 +172,26 @@ struct PragmaSchemaView : PragmaManager::GlobalHandler { static std::unique_ptr Create () { return std::make_unique(); } }; +//======================================================================================= +// @bsiclass PragmaSchemaViewFragment +// Returns a chosen subset of schemas as a single binary blob, in the same format as +// schema_view, for incremental SchemaView loading. +// The single string argument is a comma-separated list of decimal ec_Schema ids, optionally +// prefixed with a 'v;' format-version token. The id list must be dependency-closed (the +// caller computes the reference closure; the pragma does not expand references). +// Usage: PRAGMA schema_view_fragment('131,145,150') -- latest format version +// PRAGMA schema_view_fragment('v1;131,145,150') -- explicit format version 1 +// Result: single row, same columns as schema_view: format, formatVersion, data, schemaToken. +//+===============+===============+===============+===============+===============+====== +struct PragmaSchemaViewFragment : PragmaManager::GlobalHandler { + static constexpr uint8_t CURRENT_FORMAT_VERSION = 1; + PragmaSchemaViewFragment():GlobalHandler("schema_view_fragment", "schema_view_fragment('[v;]id,id,...') - returns the given subset of schemas as a base64-encoded binary blob string for incremental loading. Optional leading 'v;' selects the format version (default: latest)."){} + ~PragmaSchemaViewFragment(){} + virtual DbResult Read(PragmaManager::RowSet&, ECDbCR, PragmaVal const&, PragmaManager::OptionsMap const&) override; + virtual DbResult Write(PragmaManager::RowSet&, ECDbCR, PragmaVal const&, PragmaManager::OptionsMap const&) override; + static std::unique_ptr Create () { return std::make_unique(); } +}; + //======================================================================================= // @bsiclass //+===============+===============+===============+===============+===============+====== diff --git a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp index 4e8671ee28f..23b7eb8f0b4 100644 --- a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp +++ b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp @@ -232,19 +232,8 @@ DbResult SchemaViewWriter::CollectPropertyDefsAndRefs(DbCR db) "AND hp_ca.ClassId=%" PRIi64 " ", (int)ECN::CustomAttributeContainerType::AnyProperty, m_hiddenPropertyCAClassId.value())); - if (!m_excludedSchemaIds.empty()) - { - sql.append("WHERE c.SchemaId NOT IN ("); - bool first = true; - for (auto id : m_excludedSchemaIds) - { - if (!first) sql.append(","); - sql.append(Utf8PrintfString("%" PRIi64, id)); - first = false; - } - sql.append(") "); - } - sql.append("ORDER BY c.Id, p.Ordinal"); + AppendSchemaFilter(sql, "c.SchemaId"); + sql.append(" ORDER BY c.Id, p.Ordinal"); Statement stmt; auto rc = PrepareStmt(stmt, db, sql.c_str()); @@ -443,23 +432,46 @@ DbResult SchemaViewWriter::CollectHiddenClassIds(DbCR db) } //--------------------------------------------------------------------------------------- -// Helper: append WHERE NOT IN (...) for excluded schemas. +// Helper: append a leading-space " WHERE ..." schema filter for `column`. Always excludes the +// standard/internal schemas; for a fragment, additionally restricts to the requested set. The two +// conditions combine with AND, so a requested id that is also an excluded schema is still dropped - +// matching the whole-schema blob, where a reference into an excluded schema resolves to "absent". //--------------------------------------------------------------------------------------- -static void AppendExcludedSchemaFilter(Utf8StringR sql, Utf8CP column, std::unordered_set const& ids) +void SchemaViewWriter::AppendSchemaFilter(Utf8StringR sql, Utf8CP column) const { - if (ids.empty()) + auto appendIdList = [&sql](std::unordered_set const& ids) + { + bool first = true; + for (auto id : ids) + { + if (!first) sql.append(","); + sql.append(Utf8PrintfString("%" PRIi64, id)); + first = false; + } + }; + + bool const hasExclusion = !m_excludedSchemaIds.empty(); + bool const hasRequest = m_isFragment; + if (!hasExclusion && !hasRequest) return; + sql.append(" WHERE "); - sql.append(column); - sql.append(" NOT IN ("); - bool first = true; - for (auto id : ids) + if (hasExclusion) { - if (!first) sql.append(","); - sql.append(Utf8PrintfString("%" PRIi64, id)); - first = false; + sql.append(column); + sql.append(" NOT IN ("); + appendIdList(m_excludedSchemaIds); + sql.append(")"); + } + if (hasRequest) + { + if (hasExclusion) + sql.append(" AND "); + sql.append(column); + sql.append(" IN ("); + appendIdList(m_requestedSchemaIds); + sql.append(")"); } - sql.append(")"); } //--------------------------------------------------------------------------------------- @@ -496,7 +508,7 @@ DbResult SchemaViewWriter::WritePropertyDefTable() DbResult SchemaViewWriter::WriteSchemaTable(DbCR db) { Utf8String sql("SELECT Id,Name,DisplayLabel,Description,Alias,VersionDigit1,VersionDigit2,VersionDigit3 FROM [main].[ec_Schema]"); - AppendExcludedSchemaFilter(sql, "Id", m_excludedSchemaIds); + AppendSchemaFilter(sql, "Id"); sql.append(" ORDER BY Id"); Statement stmt; @@ -533,7 +545,7 @@ DbResult SchemaViewWriter::WriteSchemaTable(DbCR db) DbResult SchemaViewWriter::WriteEnumTable(DbCR db) { Utf8String sql("SELECT e.Id,e.Name,e.DisplayLabel,e.Description,e.UnderlyingPrimitiveType,e.IsStrict,e.EnumValues,e.SchemaId FROM [main].[ec_Enumeration] e"); - AppendExcludedSchemaFilter(sql, "e.SchemaId", m_excludedSchemaIds); + AppendSchemaFilter(sql, "e.SchemaId"); sql.append(" ORDER BY e.SchemaId, e.Id"); Statement stmt; @@ -568,7 +580,7 @@ DbResult SchemaViewWriter::WriteEnumTable(DbCR db) DbResult SchemaViewWriter::WriteKoqTable(DbCR db) { Utf8String sql("SELECT k.Id,k.Name,k.DisplayLabel,k.Description,k.PersistenceUnit,k.RelativeError,k.PresentationUnits,k.SchemaId FROM [main].[ec_KindOfQuantity] k"); - AppendExcludedSchemaFilter(sql, "k.SchemaId", m_excludedSchemaIds); + AppendSchemaFilter(sql, "k.SchemaId"); sql.append(" ORDER BY k.SchemaId, k.Id"); Statement stmt; @@ -604,7 +616,7 @@ DbResult SchemaViewWriter::WriteKoqTable(DbCR db) DbResult SchemaViewWriter::WritePropCatTable(DbCR db) { Utf8String sql("SELECT c.Id,c.Name,c.DisplayLabel,c.Description,c.Priority,c.SchemaId FROM [main].[ec_PropertyCategory] c"); - AppendExcludedSchemaFilter(sql, "c.SchemaId", m_excludedSchemaIds); + AppendSchemaFilter(sql, "c.SchemaId"); sql.append(" ORDER BY c.SchemaId, c.Id"); Statement stmt; @@ -641,7 +653,7 @@ DbResult SchemaViewWriter::WriteClassTable(DbCR db) "SELECT c.Id,c.Name,c.DisplayLabel,c.Description,c.Type,c.Modifier," "c.RelationshipStrength,c.RelationshipStrengthDirection,c.SchemaId " "FROM [main].[ec_Class] c"); - AppendExcludedSchemaFilter(sql, "c.SchemaId", m_excludedSchemaIds); + AppendSchemaFilter(sql, "c.SchemaId"); sql.append(" ORDER BY c.SchemaId, c.Id"); Statement classStmt; @@ -826,9 +838,62 @@ void SchemaViewWriter::WriteStringTable(size_t stOffsetPos) } //--------------------------------------------------------------------------------------- -// Main orchestration - pre-collects metadata, then writes flat tables top-down. +// Verify every requested fragment schema id is a real ec_Schema row, so a typo or stale id +// fails the request cleanly instead of silently producing an empty/partial fragment. +//--------------------------------------------------------------------------------------- +DbResult SchemaViewWriter::ValidateRequestedSchemaIds(DbCR db) + { + std::unordered_set existing; + Statement stmt; + auto rc = PrepareStmt(stmt, db, "SELECT Id FROM [main].[ec_Schema]"); + if (rc != BE_SQLITE_OK) + return rc; + while (stmt.Step() == BE_SQLITE_ROW) + existing.insert(stmt.GetValueInt64(0)); + for (auto id : m_requestedSchemaIds) + { + if (existing.find(id) == existing.end()) + { + ECDbLogger::Get().errorv("SchemaViewWriter::ValidateRequestedSchemaIds: requested schema id %" PRIi64 " does not exist", id); + return BE_SQLITE_NOTFOUND; + } + } + return BE_SQLITE_OK; + } + +//--------------------------------------------------------------------------------------- +// Public entry: write every non-excluded schema (the whole-schema blob). //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::WriteAllSchemas(DbCR db) + { + m_isFragment = false; + m_requestedSchemaIds.clear(); + return WriteBlob(db); + } + +//--------------------------------------------------------------------------------------- +// Public entry: write a fragment - only the requested schemas' owned rows. +//--------------------------------------------------------------------------------------- +DbResult SchemaViewWriter::WriteSchemas(DbCR db, std::unordered_set const& requestedSchemaIds) + { + if (requestedSchemaIds.empty()) + { + ECDbLogger::Get().error("SchemaViewWriter::WriteSchemas: requested schema id set is empty"); + return BE_SQLITE_ERROR; + } + m_isFragment = true; + m_requestedSchemaIds = requestedSchemaIds; + if (auto rc = ValidateRequestedSchemaIds(db); rc != BE_SQLITE_OK) + return rc; + return WriteBlob(db); + } + +//--------------------------------------------------------------------------------------- +// Main orchestration - pre-collects metadata, then writes flat tables top-down. The +// AppendSchemaFilter calls inside the table writers honor m_isFragment / m_requestedSchemaIds, +// so this body is shared by the whole-schema and fragment entry points. +//--------------------------------------------------------------------------------------- +DbResult SchemaViewWriter::WriteBlob(DbCR db) { m_output.clear(); m_output.reserve(2 * 1024 * 1024); diff --git a/iModelCore/ECDb/ECDb/SchemaViewWriter.h b/iModelCore/ECDb/ECDb/SchemaViewWriter.h index 918459aa02d..3fb08f043a8 100644 --- a/iModelCore/ECDb/ECDb/SchemaViewWriter.h +++ b/iModelCore/ECDb/ECDb/SchemaViewWriter.h @@ -114,6 +114,12 @@ struct SchemaViewWriter std::unordered_set m_hiddenClassIds; std::unordered_set m_explicitlyShownClassIds; // HiddenClass with Show = true + // Fragment support. When m_isFragment is true, only rows owned by schemas in + // m_requestedSchemaIds are emitted (still intersected with the standard-schema exclusion). + // When false, every non-excluded schema is emitted - the whole-schema blob. + std::unordered_set m_requestedSchemaIds; + bool m_isFragment = false; + // Pre-pass: collect metadata needed before writing DbResult CollectExcludedSchemaIds(DbCR db); DbResult ResolveHiddenPropertyCAClassId(DbCR db); @@ -128,6 +134,17 @@ struct SchemaViewWriter DbResult CollectHiddenClassIds(DbCR db); uint32_t InternPropertyDef(PropertyDefRecord const& def); + // Append a "WHERE ..." filter (leading space, no trailing) restricting `column` - an + // ec_Schema.Id or ec_*.SchemaId column - to the schemas this writer emits: always excludes + // standard schemas, and for a fragment additionally restricts to m_requestedSchemaIds. + void AppendSchemaFilter(Utf8StringR sql, Utf8CP column) const; + + // Verify every id in m_requestedSchemaIds is a real ec_Schema row. BE_SQLITE_NOTFOUND if not. + DbResult ValidateRequestedSchemaIds(DbCR db); + + // Shared body of WriteAllSchemas / WriteSchemas - runs the pre-passes and writes the tables. + DbResult WriteBlob(DbCR db); + // Per-table writers DbResult WritePropertyDefTable(); DbResult WriteSchemaTable(DbCR db); @@ -167,7 +184,19 @@ struct SchemaViewWriter //! @return BE_SQLITE_OK on success, or an error code if the ec_ tables are missing/corrupt. DbResult WriteAllSchemas(DbCR db); - //! Get the output blob. Valid after WriteAllSchemas() completes. + //! Read a chosen subset of schemas and write the binary blob - a "fragment". + //! Only rows owned by schemas in requestedSchemaIds are emitted; standard/excluded schemas + //! are skipped even when requested. Cross-schema references to schemas outside the fragment + //! are written with the same global encoding as the whole-schema blob (row ids, name pairs); + //! the consumer resolves them against its accumulated view when merging fragments. + //! @param requestedSchemaIds ec_Schema.Id values to include. The caller is responsible for + //! passing a dependency-closed set (the reference closure); the writer does not expand + //! references. Must be non-empty. + //! @return BE_SQLITE_OK on success; BE_SQLITE_ERROR on an empty set; BE_SQLITE_NOTFOUND if any + //! id is not an ec_Schema row. + DbResult WriteSchemas(DbCR db, std::unordered_set const& requestedSchemaIds); + + //! Get the output blob. Valid after WriteAllSchemas() / WriteSchemas() completes. bvector const& GetOutput() const { return m_output; } //! Move the output blob out. Invalidates internal state. diff --git a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp index 47400853811..db9fc5931ac 100644 --- a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp +++ b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp @@ -729,6 +729,164 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_token_changes_after_schema_import) { } } +//--------------------------------------------------------------------------------------- +// Note on fragment testing scope: the schema_view / schema_view_fragment blob is an internal +// contract between this C++ writer and the TypeScript reader, and it flows in one direction only. +// Re-implementing the reader here to peek at the blob's internals would duplicate that contract and +// rot. So the native tests stay rough - they assert the pragma produces a well-formed row and that +// bad input is rejected. The content-level guarantees (no-leakage, exactly which schemas/classes +// land in a view, cross-schema reference resolution) are verified in TypeScript against a live +// iModel, where the blob is consumed by its real reader: +// itwinjs-core/core/backend/src/test/schema/SchemaViewFragmentLoading.test.ts +//+---------------+---------------+---------------+---------------+---------------+------ + +//--------------------------------------------------------------------------------------- +// Returns the ec_Schema.Id (== meta.ECSchemaDef.ECInstanceId) of a schema by name, or 0 if absent. +//+---------------+---------------+---------------+---------------+---------------+------ +static int64_t GetSchemaIdByName(ECDbR ecdb, Utf8CP name) { + ECSqlStatement stmt; + if (ECSqlStatus::Success != stmt.Prepare(ecdb, "SELECT ECInstanceId FROM meta.ECSchemaDef WHERE Name=?")) + return 0; + stmt.BindText(1, name, IECSqlBinder::MakeCopy::Yes); + if (BE_SQLITE_ROW != stmt.Step()) + return 0; + return stmt.GetValueInt64(0); +} + +//--------------------------------------------------------------------------------------- +// Rough end-to-end check that the fragment pragma emits a single well-formed binary row for a +// requested subset, and that asking for more schemas carries at least as much data. Exactly which +// schemas/classes land in the blob, and the no-leakage guarantee, are verified in TypeScript (see +// the scope note above) - we do not decode the blob here. +// @bsimethod +//+---------------+---------------+---------------+---------------+---------------+------ +TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_returns_blob) { + // SchemaA references SchemaB and derives AClass from b:BBase; SchemaB also owns an unreferenced + // class. The mix gives the two-vs-one size comparison something to measure. + ASSERT_EQ(BentleyStatus::SUCCESS, SetupECDb("schema_view_fragment.ecdb", SchemaItem( + "" + "" + " " + " " + " " + " " + ""))); + + ASSERT_EQ(BentleyStatus::SUCCESS, ImportSchema(SchemaItem( + "" + "" + " " + " " + " b:BBase" + " " + " " + ""))); + + int64_t const idA = GetSchemaIdByName(m_ecdb, "SchemaA"); + int64_t const idB = GetSchemaIdByName(m_ecdb, "SchemaB"); + ASSERT_GT(idA, 0); + ASSERT_GT(idB, 0); + + // Returns the length of the (base64 text) "data" column for a fragment request, asserting it is + // a single well-formed binary row along the way. + auto fragmentDataLen = [&](Utf8StringCR idList) -> size_t { + Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%s')", idList.c_str()); + ECSqlStatement stmt; + EXPECT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())) << sql.c_str(); + EXPECT_EQ(BE_SQLITE_ROW, stmt.Step()); + EXPECT_STREQ("binary", stmt.GetValueText(0)); + EXPECT_EQ(1, stmt.GetValueInt(1)); // formatVersion + Utf8CP const data = stmt.GetValueText(2); + EXPECT_TRUE(data != nullptr && *data != '\0') << "fragment blob must be non-empty"; + size_t const len = data == nullptr ? 0 : strlen(data); + EXPECT_EQ(BE_SQLITE_DONE, stmt.Step()) << "fragment pragma must return exactly one row"; + return len; + }; + + // Asking for both schemas must carry at least as much data as asking for one - a coarse proxy + // that the request set actually drives what the blob holds. Exact contents are a TS concern. + size_t const lenA = fragmentDataLen(Utf8PrintfString("%lld", (long long)idA)); + size_t const lenAB = fragmentDataLen(Utf8PrintfString("%lld,%lld", (long long)idA, (long long)idB)); + EXPECT_GE(lenAB, lenA); +} + +//--------------------------------------------------------------------------------------- +// The optional leading 'v;' token selects the blob format version; omitting it means latest. +// @bsimethod +//+---------------+---------------+---------------+---------------+---------------+------ +TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_version_prefix) { + ASSERT_EQ(BentleyStatus::SUCCESS, SetupECDb("schema_view_fragment_ver.ecdb", SchemaItem( + "" + "" + " " + " " + " " + ""))); + + int64_t const id = GetSchemaIdByName(m_ecdb, "TestSchema"); + ASSERT_GT(id, 0); + + { // explicit 'v1;' prefix succeeds and reports format version 1 + Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('v1;%lld')", (long long)id); + ECSqlStatement stmt; + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())); + ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); + ASSERT_STREQ("binary", stmt.GetValueText(0)); + ASSERT_EQ(1, stmt.GetValueInt(1)); // formatVersion + Utf8CP const data = stmt.GetValueText(2); + ASSERT_TRUE(data != nullptr && *data != '\0') << "fragment blob must be non-empty"; + } + + { // no prefix defaults to latest, also version 1 + Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%lld')", (long long)id); + ECSqlStatement stmt; + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())); + ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); + ASSERT_EQ(1, stmt.GetValueInt(1)); + } +} + +//--------------------------------------------------------------------------------------- +// Malformed argument strings fail the pragma rather than emitting a partial or wrong blob. +// @bsimethod +//+---------------+---------------+---------------+---------------+---------------+------ +TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_invalid_arguments) { + ASSERT_EQ(BentleyStatus::SUCCESS, SetupECDb("schema_view_fragment_invalid.ecdb", SchemaItem( + "" + "" + " " + " " + " " + ""))); + + int64_t const id = GetSchemaIdByName(m_ecdb, "TestSchema"); + ASSERT_GT(id, 0); + Utf8String const validId = Utf8PrintfString("%lld", (long long)id); + + auto expectPrepareFails = [&](Utf8CP sql) { + ECSqlStatement stmt; + EXPECT_EQ(ECSqlStatus::Status::SQLiteError, stmt.Prepare(m_ecdb, sql)) << sql; + }; + + expectPrepareFails("PRAGMA schema_view_fragment('')"); // empty list + expectPrepareFails("PRAGMA schema_view_fragment('abc')"); // non-integer id + expectPrepareFails("PRAGMA schema_view_fragment('v1;')"); // version present, empty list + expectPrepareFails("PRAGMA schema_view_fragment('v;1')"); // malformed version token (no digits) + expectPrepareFails("PRAGMA schema_view_fragment('vx;1')"); // malformed version token (non-digit) + expectPrepareFails("PRAGMA schema_view_fragment('v99;1')"); // unsupported version + expectPrepareFails("PRAGMA schema_view_fragment('999999999')"); // non-existent schema id + expectPrepareFails("PRAGMA schema_view_fragment(1)"); // integer argument, not a string + expectPrepareFails("PRAGMA schema_view_fragment=2"); // assignment form is read-only + expectPrepareFails(Utf8PrintfString("PRAGMA schema_view_fragment('%s,%s')", validId.c_str(), validId.c_str()).c_str()); // duplicate id + + { // sanity: the valid id alone prepares and returns a row + Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%s')", validId.c_str()); + ECSqlStatement stmt; + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())); + ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); + } +} + //--------------------------------------------------------------------------------------- // Regression test against the 4.0.0.1 benchmark fixture. Profile 4.0.0.1 predates the // EC3.2 Units/Formats migration (introduced in 4.0.0.2, ~2018). SchemaViewWriter does From b0d390409b2f9cfcffe93ec5fb51ca16cea5bed9 Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Tue, 23 Jun 2026 09:35:56 +0200 Subject: [PATCH 02/10] Add schema_token support to PRAGMA checksum for cheap schema-identity hashing --- iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp | 65 +++++++++++++-- iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h | 4 +- .../Tests/NonPublished/ECSqlPragmasTests.cpp | 81 +++++++++++++++++-- 3 files changed, 136 insertions(+), 14 deletions(-) diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp index dc425bbd628..851f6f31ba6 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp @@ -32,6 +32,7 @@ DbResult PragmaChecksum::Read(PragmaManager::RowSet& rowSet, ECDbCR ecdb, Pragma const Utf8String kECDbSchema = "ecdb_schema"; const Utf8String kECDbMap = "ecdb_map"; const Utf8String kSQLiteSchema = "sqlite_schema"; + const Utf8String kSchemaToken = "schema_token"; if (kECDbSchema.EqualsIAscii(val.GetString())) { Utf8String sha3; @@ -87,13 +88,35 @@ DbResult PragmaChecksum::Read(PragmaManager::RowSet& rowSet, ECDbCR ecdb, Pragma rowSet = std::move(result); return BE_SQLITE_OK; } + // Unlike the other keys, schema_token hashes schema *identity* only (every schema's name and + // version, one tiny row each), NOT schema contents. It is the cheap cache-invalidation key for + // SchemaView - see PRAGMA checksum docs / SchemaViewBinaryFormat.md. Limitation: a same-version + // content change (which ECDb only allows for dynamic schemas) does not change this value. + if (kSchemaToken.EqualsIAscii(val.GetString())) { + Utf8String sha3; + if (SHA3Helper::ComputeHash(sha3, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256) != BE_SQLITE_OK) { + ecdb.GetImpl().Issues().Report( + IssueSeverity::Error, + IssueCategory::BusinessProperties, + IssueType::ECSQL, + ECDbIssueId::ECDb_0593, + "Unable to compute schema token."); + + rowSet = std::move(result); + return BE_SQLITE_ERROR; + } + auto row = result->AppendRow(); + row.appendValue() = sha3; + rowSet = std::move(result); + return BE_SQLITE_OK; + } ecdb.GetImpl().Issues().ReportV( IssueSeverity::Error, IssueCategory::BusinessProperties, IssueType::ECSQL, ECDbIssueId::ECDb_0596, - "Unable checksum val '%s'. Valid values are ecdb_schema|ecdb_map|sqlite_schema", val.GetString().c_str()); + "Unable checksum val '%s'. Valid values are ecdb_schema|ecdb_map|sqlite_schema|schema_token", val.GetString().c_str()); rowSet = std::move(result); return BE_SQLITE_ERROR; @@ -403,6 +426,8 @@ DbResult SHA3Helper::ComputeHash(Utf8StringR hash, DbCR db, SourceType type, Utf }, dbAlias, hashSize, skipTableThatDoesNotExists); } else if (type == SourceType::SQLITE_SCHEMA) { rc = ComputeSQLiteSchemaHash(hash, db, dbAlias, hashSize); + } else if (type == SourceType::ECDB_SCHEMA_TOKEN) { + rc = ComputeSchemaTokenHash(hash, db, dbAlias, hashSize); } if (rc != BE_SQLITE_OK) { return rc; @@ -412,6 +437,32 @@ DbResult SHA3Helper::ComputeHash(Utf8StringR hash, DbCR db, SourceType type, Utf return rc; } +//--------------------------------------------------------------------------------------- +// Cheap schema-identity hash: hashes only the name and version of every schema (one row per +// schema in ec_Schema), NOT their contents. Backs PRAGMA checksum(schema_token) and the schemaToken +// column of schema_view / schema_view_fragment. Ordered by Name for a session-stable digest. +// Limitation: a same-version content change (which ECDb only allows for dynamic schemas) does +// not change this hash. See the PRAGMA checksum(schema_token) docs. +// @bsimethod +//+---------------+---------------+---------------+---------------+---------------+------ +DbResult SHA3Helper::ComputeSchemaTokenHash(Utf8String& hash, DbCR db, Utf8CP dbAlias, HashSize hashSize) { + Statement stmt; + auto rc = stmt.Prepare(db, SqlPrintfString( + "select hex(sha3_query(\"select Name,VersionDigit1,VersionDigit2,VersionDigit3 from [%s].ec_Schema order by Name\", %d))", dbAlias, (int)hashSize)); + + if (rc != BE_SQLITE_OK) { + return rc; + } + + rc = stmt.Step(); + if (rc != BE_SQLITE_ROW) { + return rc; + } + + hash = stmt.GetValueText(0); + return BE_SQLITE_OK; +} + //--------------------------------------------------------------------------------------- // @bsimethod //+---------------+---------------+---------------+---------------+---------------+------ @@ -1010,9 +1061,10 @@ DbResult PragmaSchemaView::Read(PragmaManager::RowSet& rowSet, ECDbCR ecdb, Prag return writeResult; auto const& output = writer.GetOutput(); - // Compute schema token for cache invalidation + // Compute schema token for cache invalidation. This is the cheap schema-identity hash + // (ec_Schema names + versions only), NOT the full ecdb_schema checksum - see PRAGMA checksum(schema_token). Utf8String schemaToken; - SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA, "main", SHA3Helper::HashSize::SHA3_256); + SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256); auto row = result->AppendRow(); row.appendValue() = "binary"; @@ -1118,10 +1170,11 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec result->AppendProperty("schemaToken", PRIMITIVETYPE_String); result->FreezeSchemaChanges(); - // schemaToken is the whole-connection schema hash, identical to schema_view's, so a fragment and - // the manifest it was planned from share one cache-invalidation key. + // schemaToken is the cheap schema-identity hash (ec_Schema names + versions), identical to + // schema_view's and PRAGMA checksum(schema_token), so a fragment and the manifest it was planned from share + // one cache-invalidation key. Utf8String schemaToken; - SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA, "main", SHA3Helper::HashSize::SHA3_256); + SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256); auto row = result->AppendRow(); row.appendValue() = "binary"; diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h index 94fc904f0f5..a022908e54a 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h @@ -83,7 +83,7 @@ struct PragmaSqliteSql : PragmaManager::GlobalHandler { // @bsiclass PragmaChecksum //+===============+===============+===============+===============+===============+====== struct PragmaChecksum : PragmaManager::GlobalHandler { - PragmaChecksum():GlobalHandler("checksum", "checksum([ec_schema|ec_map|db_schema]) return sha1 checksum for data."){} + PragmaChecksum():GlobalHandler("checksum", "checksum([ecdb_schema|ecdb_map|sqlite_schema|schema_token]) return sha3 checksum for data. 'schema_token' is a cheap schema-identity hash (names+versions only) for SchemaView cache invalidation."){} ~PragmaChecksum(){} virtual DbResult Read(PragmaManager::RowSet&, ECDbCR, PragmaVal const&, PragmaManager::OptionsMap const&) override; virtual DbResult Write(PragmaManager::RowSet&, ECDbCR, PragmaVal const&, PragmaManager::OptionsMap const&) override; @@ -206,6 +206,7 @@ struct SHA3Helper final { ECDB_SCHEMA, ECDB_MAP, SQLITE_SCHEMA, + ECDB_SCHEMA_TOKEN, // schema identity only (ec_Schema name + version), cheap; backs PRAGMA checksum(schema_token) }; @@ -213,6 +214,7 @@ struct SHA3Helper final { static bool TableExists(DbCR, Utf8CP, Utf8CP); static DbResult ComputeHash(Utf8String&, DbCR, std::vector const &, Utf8CP, HashSize, bool); static DbResult ComputeSQLiteSchemaHash(Utf8String&, DbCR, Utf8CP, HashSize); + static DbResult ComputeSchemaTokenHash(Utf8String&, DbCR, Utf8CP, HashSize); public: static DbResult ComputeHash(Utf8StringR, DbCR, SourceType, Utf8CP dbAlias = "main", HashSize hashSize = HashSize::SHA3_256); }; diff --git a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp index db9fc5931ac..4d7e1c3c9ac 100644 --- a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp +++ b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp @@ -669,13 +669,14 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_token_determinism_and_checksum_match } ASSERT_STREQ(token1.c_str(), token2.c_str()) << "schema_view token must be deterministic across calls"; - // Get checksum from PRAGMA checksum(ecdb_schema) - must match schema_view's schemaToken + // schema_view's schemaToken column is the cheap schema-identity hash, so it must match + // PRAGMA checksum(schema_token) (and NOT PRAGMA checksum(ecdb_schema), which hashes full schema contents). { ECSqlStatement stmt; - ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA checksum(ecdb_schema)")); + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA checksum(schema_token)")); ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); - Utf8CP checksumVal = stmt.GetValueText(0); - ASSERT_STREQ(token1.c_str(), checksumVal) << "schema_view schemaToken must match checksum(ecdb_schema)"; + Utf8CP schemaTokenVal = stmt.GetValueText(0); + ASSERT_STREQ(token1.c_str(), schemaTokenVal) << "schema_view schemaToken must match PRAGMA checksum(schema_token)"; } } @@ -720,12 +721,78 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_token_changes_after_schema_import) { } ASSERT_STRNE(tokenBefore.c_str(), tokenAfter.c_str()) << "schema_view token must change after schema import"; - // checksum(ecdb_schema) must still match the new schema_view token + // PRAGMA checksum(schema_token) must still match the new schema_view token (both are the cheap + // schema-identity hash; the import bumped TestSchema 1.0.0 -> 1.0.1, so the hash changed). { ECSqlStatement stmt; - ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA checksum(ecdb_schema)")); + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA checksum(schema_token)")); ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); - ASSERT_STREQ(tokenAfter.c_str(), stmt.GetValueText(0)) << "checksum(ecdb_schema) must match schema_view token after import"; + ASSERT_STREQ(tokenAfter.c_str(), stmt.GetValueText(0)) << "PRAGMA checksum(schema_token) must match schema_view token after import"; + } +} + +//--------------------------------------------------------------------------------------- +// PRAGMA checksum(schema_token) is the cheap cache-invalidation key: a hash of every schema's name +// and version only. It must be deterministic across repeated calls, change whenever a schema is added +// or its version bumped, and be read-only. This walks a small series of imports / minor-version +// bumps, asking for the token repeatedly along the way. +// @bsimethod +//+---------------+---------------+---------------+---------------+---------------+------ +TEST_F(ECSqlPragmasTestFixture, schema_token_reflects_schema_changes) { + ASSERT_EQ(BentleyStatus::SUCCESS, SetupECDb("schema_token.ecdb", SchemaItem( + "" + "" + " " + " " + " " + ""))); + + // Reads PRAGMA checksum(schema_token), asserting a single well-formed non-empty string row. + auto readToken = [&]() -> Utf8String { + ECSqlStatement stmt; + EXPECT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA checksum(schema_token)")); + EXPECT_EQ(BE_SQLITE_ROW, stmt.Step()); + Utf8CP const t = stmt.GetValueText(0); + EXPECT_TRUE(t != nullptr && *t != '\0') << "schema_token must be a non-empty string"; + Utf8String const token(t == nullptr ? "" : t); + EXPECT_EQ(BE_SQLITE_DONE, stmt.Step()) << "schema_token must return exactly one row"; + return token; + }; + + // Deterministic: two back-to-back reads with no schema change are identical. + Utf8String const t0 = readToken(); + ASSERT_STREQ(t0.c_str(), readToken().c_str()) << "schema_token must be deterministic across calls"; + + // Adding a new schema changes the token. + ASSERT_EQ(BentleyStatus::SUCCESS, ImportSchema(SchemaItem( + "" + "" + " " + " " + " " + ""))); + Utf8String const t1 = readToken(); + ASSERT_STRNE(t0.c_str(), t1.c_str()) << "schema_token must change when a schema is added"; + ASSERT_STREQ(t1.c_str(), readToken().c_str()) << "schema_token must be stable when nothing changed"; + + // Bumping a schema's minor version changes the token. + ASSERT_EQ(BentleyStatus::SUCCESS, ImportSchema(SchemaItem( + "" + "" + " " + " " + " " + " " + ""))); + Utf8String const t2 = readToken(); + ASSERT_STRNE(t1.c_str(), t2.c_str()) << "schema_token must change when a schema's version is bumped"; + + // Read-only: schema_token is a checksum key (func-style argument), so there is no valid + // assignment syntax for it - the grammar's '(val)' and '=val' forms are mutually exclusive, + // making 'checksum(schema_token)=1' a parse error rather than a reachable write. + { + ECSqlStatement stmt; + ASSERT_EQ(ECSqlStatus::Status::InvalidECSql, stmt.Prepare(m_ecdb, "PRAGMA checksum(schema_token)=1")); } } From 7628fecc27efe87917900bc8f7a4d578b3cc7e83 Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Tue, 23 Jun 2026 12:32:19 +0200 Subject: [PATCH 03/10] Change primitiveType from uint8_t to uint16_t in WriteEnumTable for full ECN::PrimitiveType support --- iModelCore/ECDb/ECDb/SchemaViewWriter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp index 23b7eb8f0b4..ff7baba423c 100644 --- a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp +++ b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp @@ -562,7 +562,7 @@ DbResult SchemaViewWriter::WriteEnumTable(DbCR db) { PutU32(SafeU32Id(stmt.GetValueInt64(7))); // schemaEcId PutSRef(Safe(stmt.GetValueText(1))); // name - PutU8((uint8_t)stmt.GetValueInt(4)); // primitiveType + PutU16((uint16_t)stmt.GetValueInt(4)); // primitiveType (full ECN::PrimitiveType, e.g. 0x501 Integer / 0x901 String - does not fit in a byte) PutU8(stmt.GetValueInt(5) != 0 ? 1 : 0); // isStrict PutSRef(Safe(stmt.GetValueText(2))); // label PutSRef(Safe(stmt.GetValueText(3))); // description From bfe40190ffe96753aec5a80ffdc5320fb5999cbb Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Tue, 23 Jun 2026 14:11:19 +0200 Subject: [PATCH 04/10] Refactor SchemaViewWriter to use SafeU8 and SafeU16 for type safety and logging on value saturation --- .../ECDb/ECDb/ConcurrentQueryManagerImpl.cpp | 7 ++- iModelCore/ECDb/ECDb/SchemaViewWriter.cpp | 53 +++++++++++++------ 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/iModelCore/ECDb/ECDb/ConcurrentQueryManagerImpl.cpp b/iModelCore/ECDb/ECDb/ConcurrentQueryManagerImpl.cpp index 6427a8fe6fe..f8c6dfa7eb7 100644 --- a/iModelCore/ECDb/ECDb/ConcurrentQueryManagerImpl.cpp +++ b/iModelCore/ECDb/ECDb/ConcurrentQueryManagerImpl.cpp @@ -1064,8 +1064,11 @@ std::string QueryHelper::FormatQuery(const char* query) { // @bsimethod //--------------------------------------------------------------------------------------- void QueryHelper::BindLimits(ECSqlStatement& stmt, QueryLimit const& limit) { - const auto idxCount = stmt.GetParameterIndex(LIMIT_VAR_COUNT); - const auto idxOffset = stmt.GetParameterIndex(LIMIT_VAR_OFFSET); + // A PRAGMA (or any statement FormatQuery left unwrapped) has no LIMIT/OFFSET parameters. Use the + // non-logging TryGetParameterIndex so probing for their absence does not emit "No + // parameter index found" errors - GetParameterIndex would log one per missing parameter. + const auto idxCount = stmt.TryGetParameterIndex(LIMIT_VAR_COUNT); + const auto idxOffset = stmt.TryGetParameterIndex(LIMIT_VAR_OFFSET); if (idxCount < 0 || idxOffset < 0) return; // PRAGMA or other statement without LIMIT/OFFSET parameters stmt.BindInt64(idxCount, limit.GetCount()); diff --git a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp index ff7baba423c..718e7d5d3fa 100644 --- a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp +++ b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp @@ -22,6 +22,25 @@ static uint32_t SafeU32Id(int64_t val) return UINT32_MAX; } +// Narrow a small int field (an ordinal, or a version digit) read from an ec_ table to a +// byte / 16-bit value for the binary format. These values are tiny by construction, so a value that +// does not fit signals data corruption. Like SafeU32Id, log and saturate. +static uint8_t SafeU8(int val, Utf8CP field) + { + if (val >= 0 && val <= (int)UINT8_MAX) + return (uint8_t)val; + ECDbLogger::Get().warningv("SchemaViewWriter: %s value %d does not fit in a uint8_t; saturating", field, val); + return val < 0 ? (uint8_t)0 : (uint8_t)UINT8_MAX; + } + +static uint16_t SafeU16(int val, Utf8CP field) + { + if (val >= 0 && val <= (int)UINT16_MAX) + return (uint16_t)val; + ECDbLogger::Get().warningv("SchemaViewWriter: %s value %d does not fit in a uint16_t; saturating", field, val); + return val < 0 ? (uint16_t)0 : (uint16_t)UINT16_MAX; + } + // Helper: prepare a statement and return error on failure. static DbResult PrepareStmt(Statement& stmt, DbCR db, Utf8CP sql) { @@ -248,8 +267,8 @@ DbResult SchemaViewWriter::CollectPropertyDefsAndRefs(DbCR db) PropertyDefRecord def; def.nameSid = Intern(Safe(stmt.GetValueText(2))); def.descriptionSid= Intern(Safe(stmt.GetValueText(4))); - def.kind = (uint8_t)stmt.GetValueInt(5); - def.primitiveType = (uint16_t)stmt.GetValueInt(6); + def.kind = SafeU8(stmt.GetValueInt(5), "property kind"); + def.primitiveType = SafeU16(stmt.GetValueInt(6), "property primitiveType"); def.extTypeSid = Intern(Safe(stmt.GetValueText(7))); // FK columns are NULL when absent; GetValueInt64 returns 0 for NULL. // EC metadata IDs are 1-based, so 0 is a safe "no reference" sentinel. @@ -262,7 +281,7 @@ DbResult SchemaViewWriter::CollectPropertyDefsAndRefs(DbCR db) def.arrayMinOccurs= stmt.IsColumnNull(12) ? UINT32_MAX : (uint32_t)stmt.GetValueInt(12); def.arrayMaxOccurs= stmt.IsColumnNull(13) ? UINT32_MAX : (uint32_t)stmt.GetValueInt(13); def.navRelClassRowId = SafeU32Id(stmt.GetValueInt64(14)); - def.navDirection = (uint8_t)stmt.GetValueInt(15); + def.navDirection = SafeU8(stmt.GetValueInt(15), "navigation direction"); def.isReadonly = stmt.GetValueInt(16) != 0 ? (uint8_t)1 : (uint8_t)0; // HiddenProperty CA: if the JOIN is present, column 17 is hp_ca.Instance if (hasHiddenJoin) @@ -524,9 +543,9 @@ DbResult SchemaViewWriter::WriteSchemaTable(DbCR db) while (stmt.Step() == BE_SQLITE_ROW) { PutSRef(Safe(stmt.GetValueText(1))); // name - PutU16((uint16_t)stmt.GetValueInt(5)); - PutU16((uint16_t)stmt.GetValueInt(6)); - PutU16((uint16_t)stmt.GetValueInt(7)); + PutU16(SafeU16(stmt.GetValueInt(5), "schema read version")); + PutU16(SafeU16(stmt.GetValueInt(6), "schema write version")); + PutU16(SafeU16(stmt.GetValueInt(7), "schema minor version")); PutSRef(Safe(stmt.GetValueText(4))); // alias PutSRef(Safe(stmt.GetValueText(2))); // label PutSRef(Safe(stmt.GetValueText(3))); // description @@ -560,9 +579,9 @@ DbResult SchemaViewWriter::WriteEnumTable(DbCR db) while (stmt.Step() == BE_SQLITE_ROW) { - PutU32(SafeU32Id(stmt.GetValueInt64(7))); // schemaEcId + PutU32(SafeU32Id(stmt.GetValueInt64(7))); // schemaECId PutSRef(Safe(stmt.GetValueText(1))); // name - PutU16((uint16_t)stmt.GetValueInt(4)); // primitiveType (full ECN::PrimitiveType, e.g. 0x501 Integer / 0x901 String - does not fit in a byte) + PutU16(SafeU16(stmt.GetValueInt(4), "enumeration primitiveType")); // full ECN::PrimitiveType, e.g. 0x501 Integer / 0x901 String - does not fit in a byte PutU8(stmt.GetValueInt(5) != 0 ? 1 : 0); // isStrict PutSRef(Safe(stmt.GetValueText(2))); // label PutSRef(Safe(stmt.GetValueText(3))); // description @@ -595,7 +614,7 @@ DbResult SchemaViewWriter::WriteKoqTable(DbCR db) while (stmt.Step() == BE_SQLITE_ROW) { - PutU32(SafeU32Id(stmt.GetValueInt64(7))); // schemaEcId + PutU32(SafeU32Id(stmt.GetValueInt64(7))); // schemaECId PutSRef(Safe(stmt.GetValueText(1))); // name PutSRef(Safe(stmt.GetValueText(2))); // label PutSRef(Safe(stmt.GetValueText(3))); // description @@ -631,7 +650,7 @@ DbResult SchemaViewWriter::WritePropCatTable(DbCR db) while (stmt.Step() == BE_SQLITE_ROW) { - PutU32(SafeU32Id(stmt.GetValueInt64(5))); // schemaEcId + PutU32(SafeU32Id(stmt.GetValueInt64(5))); // schemaECId PutSRef(Safe(stmt.GetValueText(1))); // name PutSRef(Safe(stmt.GetValueText(2))); // label PutSRef(Safe(stmt.GetValueText(3))); // description @@ -704,16 +723,16 @@ DbResult SchemaViewWriter::WriteClassTable(DbCR db) if (m_queryViewClassIds.count(classId)) classType = 5; // View - PutU32(SafeU32Id(schemaId)); // schemaEcId + PutU32(SafeU32Id(schemaId)); // schemaECId PutSRef(Safe(classStmt.GetValueText(1))); // name - PutU8((uint8_t)classType); - PutU8((uint8_t)classStmt.GetValueInt(5)); // modifier + PutU8(SafeU8(classType, "class type")); + PutU8(SafeU8(classStmt.GetValueInt(5), "class modifier")); // modifier PutSRef(Safe(classStmt.GetValueText(2))); // label PutSRef(Safe(classStmt.GetValueText(3))); // description if (classType == 1) // Relationship { - PutU8((uint8_t)classStmt.GetValueInt(6)); // strength - PutU8((uint8_t)classStmt.GetValueInt(7)); // strengthDirection + PutU8(SafeU8(classStmt.GetValueInt(6), "relationship strength")); // strength + PutU8(SafeU8(classStmt.GetValueInt(7), "relationship strength direction")); // strengthDirection } PutU32(SafeU32Id(classId)); // ecInstanceId // Class hidden flag (tri-state): @@ -740,7 +759,7 @@ DbResult SchemaViewWriter::WriteClassTable(DbCR db) { PutSRef(Safe(baseStmt.GetValueText(0))); // schema name PutSRef(Safe(baseStmt.GetValueText(1))); // class name - PutU8((uint8_t)baseStmt.GetValueInt(2)); // ordinal + PutU8(SafeU8(baseStmt.GetValueInt(2), "base class ordinal")); // ordinal if (++baseCount == UINT16_MAX) { ECDbLogger::Get().errorv("SchemaViewWriter: class %" PRIi64 " has too many base classes", classId); @@ -778,7 +797,7 @@ DbResult SchemaViewWriter::WriteClassTable(DbCR db) while (constrStmt.Step() == BE_SQLITE_ROW) { int64_t constraintId = constrStmt.GetValueInt64(0); - PutU8((uint8_t)constrStmt.GetValueInt(1)); // relEnd + PutU8(SafeU8(constrStmt.GetValueInt(1), "relationship constraint end")); // relEnd (source=0 / target=1, not a class id) PutU32((uint32_t)constrStmt.GetValueInt(2)); // multLower PutU32((uint32_t)constrStmt.GetValueInt(3)); // multUpper PutU8(constrStmt.GetValueInt(4) != 0 ? 1 : 0); // isPolymorphic From 5bc7cd3f4dc91d3f8b5972c2a36657163e2f9730 Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Fri, 26 Jun 2026 12:25:07 +0200 Subject: [PATCH 05/10] Refactor schema view handling and improve documentation in ECSqlPragmas and SchemaViewWriter --- iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp | 114 ++++++++---------- iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h | 4 +- iModelCore/ECDb/ECDb/SchemaViewWriter.cpp | 55 +++++---- iModelCore/ECDb/ECDb/SchemaViewWriter.h | 10 +- .../Tests/NonPublished/ECSqlPragmasTests.cpp | 9 +- 5 files changed, 94 insertions(+), 98 deletions(-) diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp index 851f6f31ba6..1ab709bd8da 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp @@ -88,10 +88,6 @@ DbResult PragmaChecksum::Read(PragmaManager::RowSet& rowSet, ECDbCR ecdb, Pragma rowSet = std::move(result); return BE_SQLITE_OK; } - // Unlike the other keys, schema_token hashes schema *identity* only (every schema's name and - // version, one tiny row each), NOT schema contents. It is the cheap cache-invalidation key for - // SchemaView - see PRAGMA checksum docs / SchemaViewBinaryFormat.md. Limitation: a same-version - // content change (which ECDb only allows for dynamic schemas) does not change this value. if (kSchemaToken.EqualsIAscii(val.GetString())) { Utf8String sha3; if (SHA3Helper::ComputeHash(sha3, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256) != BE_SQLITE_OK) { @@ -443,6 +439,10 @@ DbResult SHA3Helper::ComputeHash(Utf8StringR hash, DbCR db, SourceType type, Utf // column of schema_view / schema_view_fragment. Ordered by Name for a session-stable digest. // Limitation: a same-version content change (which ECDb only allows for dynamic schemas) does // not change this hash. See the PRAGMA checksum(schema_token) docs. +// In the future, if we ever need this to deterministically change based on schema contents, a +// profile update which adds a fingerprint column to ec_Schema would be a correct way to do it. +// Our existing schema checksums are unreliable, we may need a better standardized hashing method if +// it ever gets to this. // @bsimethod //+---------------+---------------+---------------+---------------+---------------+------ DbResult SHA3Helper::ComputeSchemaTokenHash(Utf8String& hash, DbCR db, Utf8CP dbAlias, HashSize hashSize) { @@ -1009,6 +1009,46 @@ DbResult PragmaCheckECSqlWriteValues::Write(PragmaManager::RowSet& rowSet, ECDbC //======================================================================================= // PragmaSchemaView //======================================================================================= +namespace { +// Emits the standard schema_view result rowSet (format/formatVersion/data/schemaToken) from a +// completed SchemaViewWriter blob. Reused by both PRAGMA schema_view and schema_view_fragment +DbResult BuildSchemaViewResult(PragmaManager::RowSet& rowSet, ECDbCR ecdb, uint8_t requestedVersion, bvector const& output) { + // Profile 4.0.0.1 predates the EC3.2 Units/Formats migration (introduced in 4.0.0.2, ~2018). + // The writer queries no tables/columns added in 4.0.0.2+, so the pragma succeeds, but + // KindOfQuantity persistence/presentation strings are still in legacy FUS format rather + // than the alias-qualified EC3.2 form. + if (ecdb.GetECDbProfileVersion() < ProfileVersion(4, 0, 0, 2)) { + ECDbLogger::Get().warningv( + "PRAGMA schema_view: ECDb profile %s predates the EC3.2 Units/Formats migration. " + "KindOfQuantity persistence and presentation strings use the legacy FUS format. " + "Upgrade the ECDb profile to 4.0.0.2 or later for EC3.2-formatted strings.", + ecdb.GetECDbProfileVersion().ToString().c_str()); + } + + auto result = std::make_unique(ecdb); + result->AppendProperty("format", PRIMITIVETYPE_String); + result->AppendProperty("formatVersion", PRIMITIVETYPE_Integer); + result->AppendProperty("data", PRIMITIVETYPE_String); + result->AppendProperty("schemaToken", PRIMITIVETYPE_String); + result->FreezeSchemaChanges(); + + // schemaToken is the cheap schema-identity hash (ec_Schema names + versions), identical to + // PRAGMA checksum(schema_token), so a fragment and the manifest it was planned from share one + // cache-invalidation key. + Utf8String schemaToken; + SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256); + + auto row = result->AppendRow(); + row.appendValue() = "binary"; + row.appendValue() = (int64_t)requestedVersion; + row.appendValue().SetBinary(output.data(), output.size()); + row.appendValue() = schemaToken.c_str(); + + rowSet = std::move(result); + return BE_SQLITE_OK; +} +} + //--------------------------------------------------------------------------------------- // @bsimethod //--------------------------------------------------------------------------------------- @@ -1033,25 +1073,6 @@ DbResult PragmaSchemaView::Read(PragmaManager::RowSet& rowSet, ECDbCR ecdb, Prag return BE_SQLITE_ERROR; } - auto result = std::make_unique(ecdb); - result->AppendProperty("format", PRIMITIVETYPE_String); - result->AppendProperty("formatVersion", PRIMITIVETYPE_Integer); - result->AppendProperty("data", PRIMITIVETYPE_String); - result->AppendProperty("schemaToken", PRIMITIVETYPE_String); - result->FreezeSchemaChanges(); - - // Profile 4.0.0.1 predates the EC3.2 Units/Formats migration (introduced in 4.0.0.2, ~2018). - // The writer queries no tables/columns added in 4.0.0.2+, so the pragma succeeds, but - // KindOfQuantity persistence/presentation strings are still in legacy FUS format rather - // than the alias-qualified EC3.2 form. See SchemaViewBinaryFormat.md "ECDb Profile Compatibility". - if (ecdb.GetECDbProfileVersion() < ProfileVersion(4, 0, 0, 2)) { - ECDbLogger::Get().warningv( - "PRAGMA schema_view: ECDb profile %s predates the EC3.2 Units/Formats migration. " - "KindOfQuantity persistence and presentation strings will be returned in legacy FUS format. " - "Upgrade the ECDb profile to 4.0.0.2 or later for EC3.2-formatted strings.", - ecdb.GetECDbProfileVersion().ToString().c_str()); - } - // Build the binary blob (v1: property definition dedup). // Currently only one format version exists. When adding v2+, this must route to // the appropriate writer based on requestedVersion instead of always using v1. @@ -1059,21 +1080,8 @@ DbResult PragmaSchemaView::Read(PragmaManager::RowSet& rowSet, ECDbCR ecdb, Prag auto writeResult = writer.WriteAllSchemas(ecdb); if (writeResult != BE_SQLITE_OK) return writeResult; - auto const& output = writer.GetOutput(); - // Compute schema token for cache invalidation. This is the cheap schema-identity hash - // (ec_Schema names + versions only), NOT the full ecdb_schema checksum - see PRAGMA checksum(schema_token). - Utf8String schemaToken; - SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256); - - auto row = result->AppendRow(); - row.appendValue() = "binary"; - row.appendValue() = (int64_t)requestedVersion; - row.appendValue().SetBinary(output.data(), output.size()); - row.appendValue() = schemaToken.c_str(); - - rowSet = std::move(result); - return BE_SQLITE_OK; + return BuildSchemaViewResult(rowSet, ecdb, requestedVersion, writer.GetOutput()); } //--------------------------------------------------------------------------------------- @@ -1120,8 +1128,7 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec Utf8String arg(val.GetString().c_str()); // Optional leading 'v;' format-version token. The version is carried inside the one string - // argument because the pragma grammar allows only a single value; see the schema_view_fragment - // reference doc for the rationale. + // argument (the pragma grammar allows only a single argument) uint8_t requestedVersion = CURRENT_FORMAT_VERSION; Utf8String idList = arg; size_t const semicolon = arg.find(';'); @@ -1139,8 +1146,7 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec requestedVersion = (uint8_t)v; } - // Parse the comma-separated decimal id list into a de-duplicated set; reject any malformed or - // repeated id rather than silently emit a partial fragment. + // Parse the comma-separated decimal id list into a de-duplicated set; reject any malformed id bvector tokens; BeStringUtilities::Split(idList.c_str(), ",", tokens); std::unordered_set schemaIds; @@ -1149,8 +1155,7 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec if (!IsAllDigits(token)) return reportError(Utf8PrintfString("invalid schema id '%s'; ids must be positive decimal integers.", token.c_str())); int64_t id = (int64_t)strtoll(token.c_str(), nullptr, 10); - if (!schemaIds.insert(id).second) - return reportError(Utf8PrintfString("duplicate schema id %" PRId64 " in the request.", id)); + schemaIds.insert(id).second; // de-duplicate } if (schemaIds.empty()) return reportError("the schema id list is empty."); @@ -1161,29 +1166,8 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec return reportError("one or more requested schema ids do not exist in this connection."); if (writeResult != BE_SQLITE_OK) return writeResult; - auto const& output = writer.GetOutput(); - - auto result = std::make_unique(ecdb); - result->AppendProperty("format", PRIMITIVETYPE_String); - result->AppendProperty("formatVersion", PRIMITIVETYPE_Integer); - result->AppendProperty("data", PRIMITIVETYPE_String); - result->AppendProperty("schemaToken", PRIMITIVETYPE_String); - result->FreezeSchemaChanges(); - - // schemaToken is the cheap schema-identity hash (ec_Schema names + versions), identical to - // schema_view's and PRAGMA checksum(schema_token), so a fragment and the manifest it was planned from share - // one cache-invalidation key. - Utf8String schemaToken; - SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256); - - auto row = result->AppendRow(); - row.appendValue() = "binary"; - row.appendValue() = (int64_t)requestedVersion; - row.appendValue().SetBinary(output.data(), output.size()); - row.appendValue() = schemaToken.c_str(); - rowSet = std::move(result); - return BE_SQLITE_OK; + return BuildSchemaViewResult(rowSet, ecdb, requestedVersion, writer.GetOutput()); } //--------------------------------------------------------------------------------------- diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h index a022908e54a..7426e7e1d17 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h @@ -178,7 +178,7 @@ struct PragmaSchemaView : PragmaManager::GlobalHandler { // schema_view, for incremental SchemaView loading. // The single string argument is a comma-separated list of decimal ec_Schema ids, optionally // prefixed with a 'v;' format-version token. The id list must be dependency-closed (the -// caller computes the reference closure; the pragma does not expand references). +// caller computes and includes the references; the pragma does not expand references). // Usage: PRAGMA schema_view_fragment('131,145,150') -- latest format version // PRAGMA schema_view_fragment('v1;131,145,150') -- explicit format version 1 // Result: single row, same columns as schema_view: format, formatVersion, data, schemaToken. @@ -206,7 +206,7 @@ struct SHA3Helper final { ECDB_SCHEMA, ECDB_MAP, SQLITE_SCHEMA, - ECDB_SCHEMA_TOKEN, // schema identity only (ec_Schema name + version), cheap; backs PRAGMA checksum(schema_token) + ECDB_SCHEMA_TOKEN, // schema identity only (ec_Schema name + version), cheap }; diff --git a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp index 718e7d5d3fa..0a8a0936d03 100644 --- a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp +++ b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp @@ -23,8 +23,9 @@ static uint32_t SafeU32Id(int64_t val) } // Narrow a small int field (an ordinal, or a version digit) read from an ec_ table to a -// byte / 16-bit value for the binary format. These values are tiny by construction, so a value that -// does not fit signals data corruption. Like SafeU32Id, log and saturate. +// byte / 16-bit value for the binary format. These values are tiny by construction, if we ever encounter overflow, +// it warrants a warning but we still produce a valid blob by saturating to the min/max of the target type. +// If we hit this and it's valid, the binary format should be updated to use a larger type. static uint8_t SafeU8(int val, Utf8CP field) { if (val >= 0 && val <= (int)UINT8_MAX) @@ -92,7 +93,6 @@ static bool IsExcludedSchema(Utf8CP name) //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::CollectExcludedSchemaIds(DbCR db) { - m_excludedSchemaIds.clear(); Statement stmt; auto rc = PrepareStmt(stmt, db, "SELECT Id, Name FROM [main].[ec_Schema]"); if (rc != BE_SQLITE_OK) @@ -140,8 +140,6 @@ uint32_t SchemaViewWriter::Intern(Utf8CP str) //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::ResolveHiddenPropertyCAClassId(DbCR db) { - m_hiddenPropertyCAClassId.reset(); - Statement stmt; auto rc = PrepareStmt(stmt, db, "SELECT c.Id FROM [main].[ec_Class] c " @@ -188,8 +186,6 @@ bool SchemaViewWriter::IsHiddenFromInstanceXml(Utf8CP instanceXml, Utf8CP showPr //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::ResolveQueryViewCAClassId(DbCR db) { - m_queryViewCAClassId.reset(); - Statement stmt; auto rc = PrepareStmt(stmt, db, "SELECT c.Id FROM [main].[ec_Class] c " @@ -220,10 +216,6 @@ uint32_t SchemaViewWriter::InternPropertyDef(PropertyDefRecord const& def) DbResult SchemaViewWriter::CollectPropertyDefsAndRefs(DbCR db) { - m_propDefs.clear(); - m_propDefIndex.clear(); - m_classPropRefs.clear(); - // Excluded schemas are filtered at the SQL level via NOT IN for efficiency. // Ordered by ClassId,Ordinal so m_classPropRefs preserves per-class order. // When the HiddenProperty CA class exists, an extra column (hp_ca.Instance) is added @@ -310,7 +302,6 @@ DbResult SchemaViewWriter::CollectPropertyDefsAndRefs(DbCR db) //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::CollectMixinClassIds(DbCR db) { - m_mixinClassIds.clear(); Statement stmt; auto rc = PrepareStmt(stmt, db, Utf8PrintfString( "SELECT ca.ContainerId FROM [main].[ec_CustomAttribute] ca " @@ -331,7 +322,6 @@ DbResult SchemaViewWriter::CollectMixinClassIds(DbCR db) //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::CollectQueryViewClassIds(DbCR db) { - m_queryViewClassIds.clear(); if (!m_queryViewCAClassId.has_value()) return BE_SQLITE_OK; Statement stmt; @@ -352,8 +342,6 @@ DbResult SchemaViewWriter::CollectQueryViewClassIds(DbCR db) //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::ResolveHiddenSchemaCAClassId(DbCR db) { - m_hiddenSchemaCAClassId.reset(); - Statement stmt; auto rc = PrepareStmt(stmt, db, "SELECT c.Id FROM [main].[ec_Class] c " @@ -372,8 +360,6 @@ DbResult SchemaViewWriter::ResolveHiddenSchemaCAClassId(DbCR db) //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::ResolveHiddenClassCAClassId(DbCR db) { - m_hiddenClassCAClassId.reset(); - Statement stmt; auto rc = PrepareStmt(stmt, db, "SELECT c.Id FROM [main].[ec_Class] c " @@ -396,8 +382,6 @@ DbResult SchemaViewWriter::ResolveHiddenClassCAClassId(DbCR db) //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::CollectHiddenSchemaIds(DbCR db) { - m_hiddenSchemaIds.clear(); - m_schemasWithHiddenClasses.clear(); if (!m_hiddenSchemaCAClassId.has_value()) return BE_SQLITE_OK; Statement stmt; @@ -427,8 +411,6 @@ DbResult SchemaViewWriter::CollectHiddenSchemaIds(DbCR db) //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::CollectHiddenClassIds(DbCR db) { - m_hiddenClassIds.clear(); - m_explicitlyShownClassIds.clear(); if (!m_hiddenClassCAClassId.has_value()) return BE_SQLITE_OK; Statement stmt; @@ -907,6 +889,33 @@ DbResult SchemaViewWriter::WriteSchemas(DbCR db, std::unordered_set con return WriteBlob(db); } +//--------------------------------------------------------------------------------------- +// Reset all per-run accumulation state so a single instance can be reused for multiple +// WriteAllSchemas / WriteSchemas calls. Centralizing the reset here keeps reuse correct even if +// a new field is added and its collector forgets to clear. +// m_isFragment / m_requestedSchemaIds are owned by the public entry points and not reset here. +//--------------------------------------------------------------------------------------- +void SchemaViewWriter::ResetState() + { + m_output.clear(); + m_stringTable.clear(); + m_stringIndex.clear(); + m_propDefs.clear(); + m_propDefIndex.clear(); + m_classPropRefs.clear(); + m_hiddenPropertyCAClassId.reset(); + m_hiddenSchemaCAClassId.reset(); + m_hiddenClassCAClassId.reset(); + m_queryViewCAClassId.reset(); + m_excludedSchemaIds.clear(); + m_mixinClassIds.clear(); + m_queryViewClassIds.clear(); + m_hiddenSchemaIds.clear(); + m_schemasWithHiddenClasses.clear(); + m_hiddenClassIds.clear(); + m_explicitlyShownClassIds.clear(); + } + //--------------------------------------------------------------------------------------- // Main orchestration - pre-collects metadata, then writes flat tables top-down. The // AppendSchemaFilter calls inside the table writers honor m_isFragment / m_requestedSchemaIds, @@ -914,10 +923,8 @@ DbResult SchemaViewWriter::WriteSchemas(DbCR db, std::unordered_set con //--------------------------------------------------------------------------------------- DbResult SchemaViewWriter::WriteBlob(DbCR db) { - m_output.clear(); + ResetState(); m_output.reserve(2 * 1024 * 1024); - m_stringTable.clear(); - m_stringIndex.clear(); Intern(""); // index 0 = empty string // Pre-pass: collect metadata needed before writing diff --git a/iModelCore/ECDb/ECDb/SchemaViewWriter.h b/iModelCore/ECDb/ECDb/SchemaViewWriter.h index 3fb08f043a8..abf6b315e49 100644 --- a/iModelCore/ECDb/ECDb/SchemaViewWriter.h +++ b/iModelCore/ECDb/ECDb/SchemaViewWriter.h @@ -31,6 +31,7 @@ BEGIN_BENTLEY_SQLITE_EC_NAMESPACE //! Format spec: docs/learning/metadata/SchemaViewBinaryFormat.md in itwinjs-core. //! //! Thread safety: safe for concurrent reads when ECDb is in WAL mode. +//! HOWEVER, the writer itself is **not designed for concurrent use.** Each thread must create its own SchemaViewWriter instance. //! Called from the PragmaSchemaView handler on the ConcurrentQuery thread pool. // @bsistruct //======================================================================================= @@ -142,7 +143,11 @@ struct SchemaViewWriter // Verify every id in m_requestedSchemaIds is a real ec_Schema row. BE_SQLITE_NOTFOUND if not. DbResult ValidateRequestedSchemaIds(DbCR db); - // Shared body of WriteAllSchemas / WriteSchemas - runs the pre-passes and writes the tables. + // Reset all per-run accumulation state so a single instance can be reused for multiple + // WriteAllSchemas / WriteSchemas calls. + void ResetState(); + + // Shared body of WriteAllSchemas / WriteSchemas - resets state, runs the pre-passes and writes the tables. DbResult WriteBlob(DbCR db); // Per-table writers @@ -196,7 +201,8 @@ struct SchemaViewWriter //! id is not an ec_Schema row. DbResult WriteSchemas(DbCR db, std::unordered_set const& requestedSchemaIds); - //! Get the output blob. Valid after WriteAllSchemas() / WriteSchemas() completes. + //! Get the output blob. Only valid after WriteAllSchemas() / WriteSchemas() returns BE_SQLITE_OK; + //! on an error return the blob is partial and must not be used. bvector const& GetOutput() const { return m_output; } //! Move the output blob out. Invalidates internal state. diff --git a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp index 4d7e1c3c9ac..80b69866a39 100644 --- a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp +++ b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp @@ -803,8 +803,7 @@ TEST_F(ECSqlPragmasTestFixture, schema_token_reflects_schema_changes) { // rot. So the native tests stay rough - they assert the pragma produces a well-formed row and that // bad input is rejected. The content-level guarantees (no-leakage, exactly which schemas/classes // land in a view, cross-schema reference resolution) are verified in TypeScript against a live -// iModel, where the blob is consumed by its real reader: -// itwinjs-core/core/backend/src/test/schema/SchemaViewFragmentLoading.test.ts +// iModel, where the blob is consumed by its real reader. //+---------------+---------------+---------------+---------------+---------------+------ //--------------------------------------------------------------------------------------- @@ -822,7 +821,7 @@ static int64_t GetSchemaIdByName(ECDbR ecdb, Utf8CP name) { //--------------------------------------------------------------------------------------- // Rough end-to-end check that the fragment pragma emits a single well-formed binary row for a -// requested subset, and that asking for more schemas carries at least as much data. Exactly which +// requested subset, and that asking for more schemas carries more data. Exactly which // schemas/classes land in the blob, and the no-leakage guarantee, are verified in TypeScript (see // the scope note above) - we do not decode the blob here. // @bsimethod @@ -870,11 +869,11 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_returns_blob) { return len; }; - // Asking for both schemas must carry at least as much data as asking for one - a coarse proxy + // Asking for both schemas must carry more data as asking for one - a coarse proxy // that the request set actually drives what the blob holds. Exact contents are a TS concern. size_t const lenA = fragmentDataLen(Utf8PrintfString("%lld", (long long)idA)); size_t const lenAB = fragmentDataLen(Utf8PrintfString("%lld,%lld", (long long)idA, (long long)idB)); - EXPECT_GE(lenAB, lenA); + EXPECT_GT(lenAB, lenA); } //--------------------------------------------------------------------------------------- From 2d7edf56ff8e9105a4421001c09df2876ac7519b Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Fri, 26 Jun 2026 12:37:08 +0200 Subject: [PATCH 06/10] Update schema view fragment handling to accept duplicate IDs and reject zero as a valid schema ID --- iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp | 8 ++++++-- iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp | 9 ++++++++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp index 1ab709bd8da..8667ecc760f 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp @@ -1146,7 +1146,9 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec requestedVersion = (uint8_t)v; } - // Parse the comma-separated decimal id list into a de-duplicated set; reject any malformed id + // Parse the comma-separated decimal id list into a set; reject any malformed id. Duplicate ids + // are intentionally de-duplicated (not an error): a caller passing the same schema id twice still + // gets one copy of that schema in the fragment. bvector tokens; BeStringUtilities::Split(idList.c_str(), ",", tokens); std::unordered_set schemaIds; @@ -1155,7 +1157,9 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec if (!IsAllDigits(token)) return reportError(Utf8PrintfString("invalid schema id '%s'; ids must be positive decimal integers.", token.c_str())); int64_t id = (int64_t)strtoll(token.c_str(), nullptr, 10); - schemaIds.insert(id).second; // de-duplicate + if (id == 0) + return reportError(Utf8PrintfString("invalid schema id '%s'; ids must be positive decimal integers.", token.c_str())); + schemaIds.insert(id); // de-duplicate (duplicate ids are accepted, not rejected) } if (schemaIds.empty()) return reportError("the schema id list is empty."); diff --git a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp index 80b69866a39..9eca3c77375 100644 --- a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp +++ b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp @@ -943,7 +943,7 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_invalid_arguments) { expectPrepareFails("PRAGMA schema_view_fragment('999999999')"); // non-existent schema id expectPrepareFails("PRAGMA schema_view_fragment(1)"); // integer argument, not a string expectPrepareFails("PRAGMA schema_view_fragment=2"); // assignment form is read-only - expectPrepareFails(Utf8PrintfString("PRAGMA schema_view_fragment('%s,%s')", validId.c_str(), validId.c_str()).c_str()); // duplicate id + expectPrepareFails("PRAGMA schema_view_fragment('0')"); // zero is not a positive id { // sanity: the valid id alone prepares and returns a row Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%s')", validId.c_str()); @@ -951,6 +951,13 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_invalid_arguments) { ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())); ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); } + + { // duplicate ids are intentionally de-duplicated, not rejected: same id twice still prepares + Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%s,%s')", validId.c_str(), validId.c_str()); + ECSqlStatement stmt; + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())) << sql.c_str(); + ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); + } } //--------------------------------------------------------------------------------------- From 4204b06bd50635b10f9a6d3da04d959c100a7080 Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Fri, 26 Jun 2026 12:41:48 +0200 Subject: [PATCH 07/10] Return proper error message if schema token pragma call fails Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp index 8667ecc760f..fd898ba8ce0 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp @@ -1036,8 +1036,17 @@ DbResult BuildSchemaViewResult(PragmaManager::RowSet& rowSet, ECDbCR ecdb, uint8 // PRAGMA checksum(schema_token), so a fragment and the manifest it was planned from share one // cache-invalidation key. Utf8String schemaToken; - SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256); - + if (SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256) != BE_SQLITE_OK) { + ecdb.GetImpl().Issues().Report( + IssueSeverity::Error, + IssueCategory::BusinessProperties, + IssueType::ECSQL, + ECDbIssueId::ECDb_0593, + "Unable to compute schema token."); + rowSet = std::make_unique(ecdb); + rowSet->FreezeSchemaChanges(); + return BE_SQLITE_ERROR; + } auto row = result->AppendRow(); row.appendValue() = "binary"; row.appendValue() = (int64_t)requestedVersion; From fdc68c2f006bf5e66726a044a97d7af402a130d0 Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Tue, 30 Jun 2026 09:36:54 +0200 Subject: [PATCH 08/10] Enhance comments in SafeU8 function to clarify validation approach and resilience in SchemaView handling --- iModelCore/ECDb/ECDb/SchemaViewWriter.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp index 0a8a0936d03..f335f034d12 100644 --- a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp +++ b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp @@ -26,6 +26,9 @@ static uint32_t SafeU32Id(int64_t val) // byte / 16-bit value for the binary format. These values are tiny by construction, if we ever encounter overflow, // it warrants a warning but we still produce a valid blob by saturating to the min/max of the target type. // If we hit this and it's valid, the binary format should be updated to use a larger type. +// Explicitly didn't choose an error here because SchemaView needs to be resilient since it's a "all or nothing"-mechanism. +// Adding too strict validation would cause consumers to not see any schema at all, which is very disruptive. +// The warning is enough to catch the issue in testing. static uint8_t SafeU8(int val, Utf8CP field) { if (val >= 0 && val <= (int)UINT8_MAX) From 85fa6700e70345c2e93d079d7839deaee673b5a7 Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Fri, 3 Jul 2026 14:20:03 +0200 Subject: [PATCH 09/10] Update PragmaSchemaViewFragment to accept schema names instead of IDs and adjust related tests --- iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp | 41 +++++++------ iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h | 13 ++-- .../Tests/NonPublished/ECSqlPragmasTests.cpp | 59 ++++++++++--------- 3 files changed, 63 insertions(+), 50 deletions(-) diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp index fd898ba8ce0..1ff1bdb0e82 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp @@ -1132,18 +1132,19 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec }; if (!val.IsString()) - return reportError("expects a single string argument: a comma-separated list of decimal ec_Schema ids, optionally prefixed with a 'v;' format-version token."); + return reportError("expects a single string argument: a comma-separated list of schema names, optionally prefixed with a 'v;' format-version token."); Utf8String arg(val.GetString().c_str()); // Optional leading 'v;' format-version token. The version is carried inside the one string - // argument (the pragma grammar allows only a single argument) + // argument (the pragma grammar allows only a single argument). Schema names are ECNames, so + // ';' can never occur in one - a semicolon always means a version prefix. uint8_t requestedVersion = CURRENT_FORMAT_VERSION; - Utf8String idList = arg; + Utf8String nameList = arg; size_t const semicolon = arg.find(';'); if (semicolon != Utf8String::npos) { Utf8String versionToken = arg.substr(0, semicolon); - idList = arg.substr(semicolon + 1); + nameList = arg.substr(semicolon + 1); if (versionToken.size() < 2 || (versionToken[0] != 'v' && versionToken[0] != 'V')) return reportError("malformed version prefix; expected 'v;' (for example 'v1;')."); Utf8String digits = versionToken.substr(1); @@ -1155,28 +1156,34 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec requestedVersion = (uint8_t)v; } - // Parse the comma-separated decimal id list into a set; reject any malformed id. Duplicate ids - // are intentionally de-duplicated (not an error): a caller passing the same schema id twice still - // gets one copy of that schema in the fragment. + // Parse the comma-separated schema name list and resolve each name to its ec_Schema id. + // Names are ECNames ([A-Za-z_][A-Za-z0-9_]*), so ',' can never occur in one. A malformed or + // unknown name fails the pragma (no partial fragment). Duplicate names are intentionally + // de-duplicated (not an error): a caller passing the same schema twice still gets one copy. bvector tokens; - BeStringUtilities::Split(idList.c_str(), ",", tokens); + BeStringUtilities::Split(nameList.c_str(), ",", tokens); std::unordered_set schemaIds; + Statement nameStmt; + if (BE_SQLITE_OK != nameStmt.Prepare(ecdb, "SELECT Id FROM [main].[ec_Schema] WHERE Name=? COLLATE NOCASE")) + return reportError("failed to prepare the schema name lookup."); for (Utf8StringR token : tokens) { token.Trim(); - if (!IsAllDigits(token)) - return reportError(Utf8PrintfString("invalid schema id '%s'; ids must be positive decimal integers.", token.c_str())); - int64_t id = (int64_t)strtoll(token.c_str(), nullptr, 10); - if (id == 0) - return reportError(Utf8PrintfString("invalid schema id '%s'; ids must be positive decimal integers.", token.c_str())); - schemaIds.insert(id); // de-duplicate (duplicate ids are accepted, not rejected) + if (!ECN::ECNameValidation::IsValidName(token.c_str())) + return reportError(Utf8PrintfString("invalid schema name '%s'; names must be valid ECNames.", token.c_str())); + nameStmt.Reset(); + nameStmt.ClearBindings(); + nameStmt.BindText(1, token.c_str(), Statement::MakeCopy::Yes); + if (BE_SQLITE_ROW != nameStmt.Step()) + return reportError(Utf8PrintfString("schema '%s' does not exist in this connection.", token.c_str())); + schemaIds.insert(nameStmt.GetValueInt64(0)); } if (schemaIds.empty()) - return reportError("the schema id list is empty."); + return reportError("the schema name list is empty."); SchemaViewWriter writer; DbResult writeResult = writer.WriteSchemas(ecdb, schemaIds); - if (writeResult == BE_SQLITE_NOTFOUND) - return reportError("one or more requested schema ids do not exist in this connection."); + if (writeResult == BE_SQLITE_NOTFOUND) // cannot happen (ids were just resolved), kept as defense in depth + return reportError("one or more requested schemas do not exist in this connection."); if (writeResult != BE_SQLITE_OK) return writeResult; diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h index 7426e7e1d17..daf7e74c958 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.h @@ -176,16 +176,17 @@ struct PragmaSchemaView : PragmaManager::GlobalHandler { // @bsiclass PragmaSchemaViewFragment // Returns a chosen subset of schemas as a single binary blob, in the same format as // schema_view, for incremental SchemaView loading. -// The single string argument is a comma-separated list of decimal ec_Schema ids, optionally -// prefixed with a 'v;' format-version token. The id list must be dependency-closed (the -// caller computes and includes the references; the pragma does not expand references). -// Usage: PRAGMA schema_view_fragment('131,145,150') -- latest format version -// PRAGMA schema_view_fragment('v1;131,145,150') -- explicit format version 1 +// The single string argument is a comma-separated list of schema names, optionally +// prefixed with a 'v;' format-version token. Names are ECNames, so ',' and ';' can never +// occur in one. The name list must be dependency-closed (the caller computes and includes +// the references; the pragma does not expand references). +// Usage: PRAGMA schema_view_fragment('BisCore,Generic') -- latest format version +// PRAGMA schema_view_fragment('v1;BisCore,Generic') -- explicit format version 1 // Result: single row, same columns as schema_view: format, formatVersion, data, schemaToken. //+===============+===============+===============+===============+===============+====== struct PragmaSchemaViewFragment : PragmaManager::GlobalHandler { static constexpr uint8_t CURRENT_FORMAT_VERSION = 1; - PragmaSchemaViewFragment():GlobalHandler("schema_view_fragment", "schema_view_fragment('[v;]id,id,...') - returns the given subset of schemas as a base64-encoded binary blob string for incremental loading. Optional leading 'v;' selects the format version (default: latest)."){} + PragmaSchemaViewFragment():GlobalHandler("schema_view_fragment", "schema_view_fragment('[v;]name,name,...') - returns the given subset of schemas as a base64-encoded binary blob string for incremental loading. Optional leading 'v;' selects the format version (default: latest)."){} ~PragmaSchemaViewFragment(){} virtual DbResult Read(PragmaManager::RowSet&, ECDbCR, PragmaVal const&, PragmaManager::OptionsMap const&) override; virtual DbResult Write(PragmaManager::RowSet&, ECDbCR, PragmaVal const&, PragmaManager::OptionsMap const&) override; diff --git a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp index 9eca3c77375..687e8d737db 100644 --- a/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp +++ b/iModelCore/ECDb/Tests/NonPublished/ECSqlPragmasTests.cpp @@ -855,8 +855,8 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_returns_blob) { // Returns the length of the (base64 text) "data" column for a fragment request, asserting it is // a single well-formed binary row along the way. - auto fragmentDataLen = [&](Utf8StringCR idList) -> size_t { - Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%s')", idList.c_str()); + auto fragmentDataLen = [&](Utf8StringCR nameList) -> size_t { + Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%s')", nameList.c_str()); ECSqlStatement stmt; EXPECT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())) << sql.c_str(); EXPECT_EQ(BE_SQLITE_ROW, stmt.Step()); @@ -871,9 +871,13 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_returns_blob) { // Asking for both schemas must carry more data as asking for one - a coarse proxy // that the request set actually drives what the blob holds. Exact contents are a TS concern. - size_t const lenA = fragmentDataLen(Utf8PrintfString("%lld", (long long)idA)); - size_t const lenAB = fragmentDataLen(Utf8PrintfString("%lld,%lld", (long long)idA, (long long)idB)); + size_t const lenA = fragmentDataLen("SchemaA"); + size_t const lenAB = fragmentDataLen("SchemaA,SchemaB"); EXPECT_GT(lenAB, lenA); + + // Names are matched case-insensitively and duplicates are de-duplicated, not rejected. + EXPECT_EQ(fragmentDataLen("schemaa"), lenA); + EXPECT_EQ(fragmentDataLen("SchemaA,schemaA"), lenA); } //--------------------------------------------------------------------------------------- @@ -893,9 +897,8 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_version_prefix) { ASSERT_GT(id, 0); { // explicit 'v1;' prefix succeeds and reports format version 1 - Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('v1;%lld')", (long long)id); ECSqlStatement stmt; - ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())); + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA schema_view_fragment('v1;TestSchema')")); ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); ASSERT_STREQ("binary", stmt.GetValueText(0)); ASSERT_EQ(1, stmt.GetValueInt(1)); // formatVersion @@ -904,9 +907,8 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_version_prefix) { } { // no prefix defaults to latest, also version 1 - Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%lld')", (long long)id); ECSqlStatement stmt; - ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())); + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA schema_view_fragment('TestSchema')")); ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); ASSERT_EQ(1, stmt.GetValueInt(1)); } @@ -927,35 +929,38 @@ TEST_F(ECSqlPragmasTestFixture, schema_view_fragment_invalid_arguments) { int64_t const id = GetSchemaIdByName(m_ecdb, "TestSchema"); ASSERT_GT(id, 0); - Utf8String const validId = Utf8PrintfString("%lld", (long long)id); auto expectPrepareFails = [&](Utf8CP sql) { ECSqlStatement stmt; EXPECT_EQ(ECSqlStatus::Status::SQLiteError, stmt.Prepare(m_ecdb, sql)) << sql; }; - expectPrepareFails("PRAGMA schema_view_fragment('')"); // empty list - expectPrepareFails("PRAGMA schema_view_fragment('abc')"); // non-integer id - expectPrepareFails("PRAGMA schema_view_fragment('v1;')"); // version present, empty list - expectPrepareFails("PRAGMA schema_view_fragment('v;1')"); // malformed version token (no digits) - expectPrepareFails("PRAGMA schema_view_fragment('vx;1')"); // malformed version token (non-digit) - expectPrepareFails("PRAGMA schema_view_fragment('v99;1')"); // unsupported version - expectPrepareFails("PRAGMA schema_view_fragment('999999999')"); // non-existent schema id - expectPrepareFails("PRAGMA schema_view_fragment(1)"); // integer argument, not a string - expectPrepareFails("PRAGMA schema_view_fragment=2"); // assignment form is read-only - expectPrepareFails("PRAGMA schema_view_fragment('0')"); // zero is not a positive id - - { // sanity: the valid id alone prepares and returns a row - Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%s')", validId.c_str()); - ECSqlStatement stmt; - ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())); + expectPrepareFails("PRAGMA schema_view_fragment('')"); // empty list + expectPrepareFails("PRAGMA schema_view_fragment('NoSuchSchema')"); // non-existent schema name + expectPrepareFails("PRAGMA schema_view_fragment('v1;')"); // version present, empty list + expectPrepareFails("PRAGMA schema_view_fragment('v;TestSchema')"); // malformed version token (no digits) + expectPrepareFails("PRAGMA schema_view_fragment('vx;TestSchema')"); // malformed version token (non-digit) + expectPrepareFails("PRAGMA schema_view_fragment('v99;TestSchema')"); // unsupported version + expectPrepareFails("PRAGMA schema_view_fragment('123')"); // decimal id, no longer a valid ECName argument + expectPrepareFails("PRAGMA schema_view_fragment('Bad Name')"); // space is not valid in an ECName + expectPrepareFails("PRAGMA schema_view_fragment(1)"); // integer argument, not a string + expectPrepareFails("PRAGMA schema_view_fragment=2"); // assignment form is read-only + + { // sanity: the valid name alone prepares and returns a row + ECSqlStatement stmt; + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA schema_view_fragment('TestSchema')")); + ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); + } + + { // duplicate names are intentionally de-duplicated, not rejected: same name twice still prepares + ECSqlStatement stmt; + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA schema_view_fragment('TestSchema,TestSchema')")); ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); } - { // duplicate ids are intentionally de-duplicated, not rejected: same id twice still prepares - Utf8String const sql = Utf8PrintfString("PRAGMA schema_view_fragment('%s,%s')", validId.c_str(), validId.c_str()); + { // a trailing comma is tolerated: Split skips empty tokens, so only the real name remains ECSqlStatement stmt; - ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, sql.c_str())) << sql.c_str(); + ASSERT_EQ(ECSqlStatus::Success, stmt.Prepare(m_ecdb, "PRAGMA schema_view_fragment('TestSchema,')")); ASSERT_EQ(BE_SQLITE_ROW, stmt.Step()); } } From 0cd13dd19035644cfd1f76cda1b474522ee6ecc1 Mon Sep 17 00:00:00 2001 From: Robert Schili Date: Sat, 25 Jul 2026 09:42:12 +0200 Subject: [PATCH 10/10] Refactor comments for clarity and conciseness in QueryHelper, SHA3Helper, and SchemaViewWriter --- .../ECDb/ECDb/ConcurrentQueryManagerImpl.cpp | 5 ++-- iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp | 24 ++++++++----------- iModelCore/ECDb/ECDb/SchemaViewWriter.cpp | 23 ++++++++---------- 3 files changed, 22 insertions(+), 30 deletions(-) diff --git a/iModelCore/ECDb/ECDb/ConcurrentQueryManagerImpl.cpp b/iModelCore/ECDb/ECDb/ConcurrentQueryManagerImpl.cpp index 3bba549b27d..0f94f23413e 100644 --- a/iModelCore/ECDb/ECDb/ConcurrentQueryManagerImpl.cpp +++ b/iModelCore/ECDb/ECDb/ConcurrentQueryManagerImpl.cpp @@ -1163,9 +1163,8 @@ std::string QueryHelper::FormatQuery(const char* query) { // @bsimethod //--------------------------------------------------------------------------------------- void QueryHelper::BindLimits(ECSqlStatement& stmt, QueryLimit const& limit) { - // A PRAGMA (or any statement FormatQuery left unwrapped) has no LIMIT/OFFSET parameters. Use the - // non-logging TryGetParameterIndex so probing for their absence does not emit "No - // parameter index found" errors - GetParameterIndex would log one per missing parameter. + // A PRAGMA (or any statement FormatQuery left unwrapped) has no LIMIT/OFFSET parameters. + // TryGetParameterIndex probes for them without logging a "No parameter index found" error each time. const auto idxCount = stmt.TryGetParameterIndex(LIMIT_VAR_COUNT); const auto idxOffset = stmt.TryGetParameterIndex(LIMIT_VAR_OFFSET); if (idxCount < 0 || idxOffset < 0) diff --git a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp index 1ff1bdb0e82..2b17a8b5c6c 100644 --- a/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp +++ b/iModelCore/ECDb/ECDb/ECSql/ECSqlPragmas.cpp @@ -438,11 +438,9 @@ DbResult SHA3Helper::ComputeHash(Utf8StringR hash, DbCR db, SourceType type, Utf // schema in ec_Schema), NOT their contents. Backs PRAGMA checksum(schema_token) and the schemaToken // column of schema_view / schema_view_fragment. Ordered by Name for a session-stable digest. // Limitation: a same-version content change (which ECDb only allows for dynamic schemas) does -// not change this hash. See the PRAGMA checksum(schema_token) docs. -// In the future, if we ever need this to deterministically change based on schema contents, a -// profile update which adds a fingerprint column to ec_Schema would be a correct way to do it. -// Our existing schema checksums are unreliable, we may need a better standardized hashing method if -// it ever gets to this. +// not change this hash. +// Note: If we ever need to make this track content, we can add a fingerprint column to ec_Schema in a +// profile update. // @bsimethod //+---------------+---------------+---------------+---------------+---------------+------ DbResult SHA3Helper::ComputeSchemaTokenHash(Utf8String& hash, DbCR db, Utf8CP dbAlias, HashSize hashSize) { @@ -1033,8 +1031,7 @@ DbResult BuildSchemaViewResult(PragmaManager::RowSet& rowSet, ECDbCR ecdb, uint8 result->FreezeSchemaChanges(); // schemaToken is the cheap schema-identity hash (ec_Schema names + versions), identical to - // PRAGMA checksum(schema_token), so a fragment and the manifest it was planned from share one - // cache-invalidation key. + // PRAGMA checksum(schema_token), so a fragment and the manifest it was planned from share one key. Utf8String schemaToken; if (SHA3Helper::ComputeHash(schemaToken, ecdb, SHA3Helper::SourceType::ECDB_SCHEMA_TOKEN, "main", SHA3Helper::HashSize::SHA3_256) != BE_SQLITE_OK) { ecdb.GetImpl().Issues().Report( @@ -1136,9 +1133,9 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec Utf8String arg(val.GetString().c_str()); - // Optional leading 'v;' format-version token. The version is carried inside the one string - // argument (the pragma grammar allows only a single argument). Schema names are ECNames, so - // ';' can never occur in one - a semicolon always means a version prefix. + // Optional leading 'v;' format-version token. It rides inside the single string argument + // because the pragma grammar allows only one. Schema names are ECNames, so a ';' always means a + // version prefix. uint8_t requestedVersion = CURRENT_FORMAT_VERSION; Utf8String nameList = arg; size_t const semicolon = arg.find(';'); @@ -1156,10 +1153,9 @@ DbResult PragmaSchemaViewFragment::Read(PragmaManager::RowSet& rowSet, ECDbCR ec requestedVersion = (uint8_t)v; } - // Parse the comma-separated schema name list and resolve each name to its ec_Schema id. - // Names are ECNames ([A-Za-z_][A-Za-z0-9_]*), so ',' can never occur in one. A malformed or - // unknown name fails the pragma (no partial fragment). Duplicate names are intentionally - // de-duplicated (not an error): a caller passing the same schema twice still gets one copy. + // Resolve each name in the comma-separated list to its ec_Schema id. Names are ECNames, so ',' + // can never occur in one. A malformed or unknown name fails the pragma - no partial fragment. + // Duplicate names are de-duplicated rather than rejected. bvector tokens; BeStringUtilities::Split(nameList.c_str(), ",", tokens); std::unordered_set schemaIds; diff --git a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp index f335f034d12..24efc2fb5e0 100644 --- a/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp +++ b/iModelCore/ECDb/ECDb/SchemaViewWriter.cpp @@ -22,13 +22,11 @@ static uint32_t SafeU32Id(int64_t val) return UINT32_MAX; } -// Narrow a small int field (an ordinal, or a version digit) read from an ec_ table to a -// byte / 16-bit value for the binary format. These values are tiny by construction, if we ever encounter overflow, -// it warrants a warning but we still produce a valid blob by saturating to the min/max of the target type. -// If we hit this and it's valid, the binary format should be updated to use a larger type. -// Explicitly didn't choose an error here because SchemaView needs to be resilient since it's a "all or nothing"-mechanism. -// Adding too strict validation would cause consumers to not see any schema at all, which is very disruptive. -// The warning is enough to catch the issue in testing. +// Narrow a small int field (an ordinal, or a version digit) read from an ec_ table to a byte / +// 16-bit value for the binary format. These values are tiny by construction. On overflow we warn and +// saturate instead of failing: SchemaView is all-or-nothing, so an error here would leave consumers +// with no schema at all. A warning is enough to catch it in testing, and the fix is to widen the +// field in the binary format. static uint8_t SafeU8(int val, Utf8CP field) { if (val >= 0 && val <= (int)UINT8_MAX) @@ -438,8 +436,8 @@ DbResult SchemaViewWriter::CollectHiddenClassIds(DbCR db) //--------------------------------------------------------------------------------------- // Helper: append a leading-space " WHERE ..." schema filter for `column`. Always excludes the // standard/internal schemas; for a fragment, additionally restricts to the requested set. The two -// conditions combine with AND, so a requested id that is also an excluded schema is still dropped - -// matching the whole-schema blob, where a reference into an excluded schema resolves to "absent". +// conditions combine with AND, so a requested id that is also excluded is still dropped - matching +// the whole-schema blob, where a reference into an excluded schema resolves to "absent". //--------------------------------------------------------------------------------------- void SchemaViewWriter::AppendSchemaFilter(Utf8StringR sql, Utf8CP column) const { @@ -893,10 +891,9 @@ DbResult SchemaViewWriter::WriteSchemas(DbCR db, std::unordered_set con } //--------------------------------------------------------------------------------------- -// Reset all per-run accumulation state so a single instance can be reused for multiple -// WriteAllSchemas / WriteSchemas calls. Centralizing the reset here keeps reuse correct even if -// a new field is added and its collector forgets to clear. -// m_isFragment / m_requestedSchemaIds are owned by the public entry points and not reset here. +// Reset all per-run accumulation state so one instance can serve multiple WriteAllSchemas / +// WriteSchemas calls. m_isFragment / m_requestedSchemaIds are owned by the public entry points and +// are not reset here. //--------------------------------------------------------------------------------------- void SchemaViewWriter::ResetState() {