Use integer IDs for components and supplemental attributes instead of UUIDs#587
Draft
daniel-thom wants to merge 3 commits into
Draft
Use integer IDs for components and supplemental attributes instead of UUIDs#587daniel-thom wants to merge 3 commits into
daniel-thom wants to merge 3 commits into
Conversation
Components and supplemental attributes are now identified by small integer
IDs assigned by SystemData when attached, instead of random UUIDs. SystemData
tracks two independent ID streams via next_component_id and
next_supplemental_attribute_id, each starting at 1; a component and an
attribute may therefore share a numeric ID. IDs are preserved across
serialization/deserialization.
- InfrastructureSystemsInternal gains an id::Int field (UNASSIGNED_ID until
attached); identity goes through get_id/set_id!. Time series keep their UUIDs.
- SystemData.component_uuids -> component_ids::Dict{Int}; subsystems -> Set{Int};
ComponentUUIDs -> ComponentIDs.
- Supplemental attribute associations table uses component_id/attribute_id
INTEGER columns.
- Time series metadata store uses owner_id INTEGER. Because the two ID streams
can collide, owner-specific queries (has_metadata, owner where-clause,
replace_component_id!) also filter by owner_category.
- Tests updated; new tests cover ID round-trip preservation, both counters
surviving serialization, and independent ID streams.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Component and supplemental attribute ids are independent streams and may collide numerically, so the unique time-series index must key on (owner_id, owner_category), not owner_id alone. Without this, a component and an attribute that share an id and have a time series with the same name/type/params violate the unique constraint. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
daniel-thom
force-pushed
the
IS4-integer-ids
branch
from
1ab5d74 to
2707dc4
Compare
There was a problem hiding this comment.
Pull request overview
This pull request transitions component and supplemental attribute identity within InfrastructureSystems.jl from UUID-based references to integer IDs, updating core containers, association stores, and serialization/tests accordingly (while retaining UUIDs for time series objects as noted in the PR description).
Changes:
- Replace component/supplemental-attribute UUID storage and lookup paths with integer ID storage/lookup (including subsystem membership).
- Update SQLite-backed time series metadata associations and supplemental attribute associations to store
owner_id/component_idinstead of UUID strings. - Update serialization/deserialization and tests to validate ID preservation and correct behavior after ID reassignment.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_time_series.jl | Updates time series tests to use component IDs and owner_id schema fields. |
| test/test_system_data.jl | Updates SystemData tests for component/attribute ID lookup and ID reassignment. |
| test/test_supplemental_attributes.jl | Adjusts manager-direct tests to manually set integer IDs to mimic attachment. |
| test/test_serialization.jl | Adds coverage for ID and next-ID counter preservation across serialization. |
| test/test_internal.jl | Updates internal tests for integer ID semantics and serialization round-trip. |
| test/test_component_container.jl | Updates component container tests to use integer IDs in lookups. |
| src/time_series_metadata_store.jl | Migrates metadata associations from owner_uuid to (owner_id, owner_category) and updates queries/indexes. |
| src/system_data.jl | Introduces component/attribute ID streams, ID assignment, and component lookup by ID. |
| src/supplemental_attribute_manager.jl | Stores attributes keyed by integer ID rather than UUID. |
| src/supplemental_attribute_associations.jl | Updates association table schema/queries to use integer IDs. |
| src/subsystems.jl | Switches subsystem membership tracking from UUID sets to integer ID sets. |
| src/iterators.jl | Updates filtered iteration to filter by get_id and Set{Int}. |
| src/internal.jl | Adds integer id to internal storage and introduces UNASSIGNED_ID, get_id, set_id!. |
| src/InfrastructureSystems.jl | Switches include from component_uuids.jl to component_ids.jl and updates docs. |
| src/components.jl | Renames component filtering parameter to component_ids and routes to ID-based iterators. |
| src/component.jl | Replaces component UUID reassignment logic with ID reassignment and reference updates. |
| src/component_uuids.jl | Removes old ComponentUUIDs helper. |
| src/component_ids.jl | Adds new ComponentIDs helper for supplemental attribute component ID tracking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1249
to
1253
| function assign_new_id!(data::SystemData, component::InfrastructureSystemsComponent) | ||
| orig_id = get_id(component) | ||
| if isnothing(pop!(data.component_ids, orig_id, nothing)) | ||
| throw(ArgumentError("component with ID = $orig_id is not stored.")) | ||
| end |
Comment on lines
1232
to
1234
| """ | ||
| Return a Vector of NamedTuple of owner UUID and time series metadata matching the inputs. | ||
| """ |
Comment on lines
29
to
+30
| "User-defined subystems. Components can be regular or masked." | ||
| subsystems::Dict{String, Set{Base.UUID}} | ||
| subsystems::Dict{String, Set{Int}} |
Comment on lines
+9
to
11
| data.subsystems[subsystem_name] = Set{Int}() | ||
| @debug "Added subystem $subsystem_name" _group = LOG_GROUP_SYSTEM | ||
| return |
Comment on lines
92
to
97
| T = typeof(attribute) | ||
| if !haskey(mgr.data, T) | ||
| mgr.data[T] = Dict{Base.UUID, T}() | ||
| mgr.data[T] = Dict{Int, T}() | ||
| end | ||
| mgr.data[T][get_uuid(attribute)] = attribute | ||
| mgr.data[T][get_id(attribute)] = attribute | ||
| end |
Comment on lines
129
to
136
| ) | ||
| TimerOutputs.@timeit_debug SYSTEM_TIMERS "add supplemental attribute association" begin | ||
| row = ( | ||
| string(get_uuid(attribute)), | ||
| get_id(attribute), | ||
| string(nameof(typeof(attribute))), | ||
| string(get_uuid(component)), | ||
| get_id(component), | ||
| string(nameof(typeof(component))), | ||
| ) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: InfrastructureSystemsInternal will continue to have a UUID field for time series until we have migrated to the new TimeSeriesStore.