Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Author
|
@codex review |
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.
Part 1 of #1040: storage, logging and lookups. Rollback through a plugin handler will follow in a second PR.
Summary
Plugins can log changes to inventories that have no block or entity, such as
/enderchest, team storage or backpacks, and find them with/co lookup container:<type>[/<id>].virtual_containerhas theentity_containercolumns withoutwid/x/y/z, andcontainer_idinstead ofentity_spawn_rowid.virtual_container_map(id, container)works likeentity_map. Both come from the normal startup DDL, so there is no patch script and no schema version bump.namespace:type/id. The id is[a-z0-9._-]{1,64}(lowercase because of the MySQL collation), and the whole key is at most 191 characters (utf8mb4 index limit on older MySQL).beforeandafterare index-aligned snapshots, each row records the slot where the change happened, so a stack taken from two slots gives two rows. The net diff is unchanged: moving a stack inside the container still logs nothing. The slot is for the rollback handler in part 2.container:returns these rows. Radius, world,#globaland inspector lookups never include them. It acceptsu: t: i: e: a:container a:+container a:-container #count #summary.r:,#container,/co rollbackand/co restorereject it.logVirtualContainerTransactionandCoreProtectPreLogEvent.Action.VIRTUAL_CONTAINER_TRANSACTION(locationnull, message is the key). Documented indocs/api/version/v13.mdanddocs/commands.md.ClickHouse.
EVENT_PARTITION_KEYis built fromPurgePolicy, so a new purgeable family changes the key andvalidateTablethen refuses existing databases. This PR adds a fixedtimePartitionedflag toClickHouseFamily(the 11 families that are monthly-partitioned today) and builds both the key andeventPartitionIdfrom it. The key stays byte-identical to master;virtual_containerlives in partition 0 and is purged by mutation. This is the part I'd most like your view on (also asked in #1040). It's isolated in 994212d, so it's easy to change or drop.Testing
container:;u:andr:lookups don't show virtual rows;/co rollback container:...is rejected;src/testis ignored here. They cover:container:parsing and parameter conflicts;EVENT_PARTITION_KEYandeventPartitionIdmatch master for every family.Merge risk
Additive. The PR adds two tables and changes no existing columns, indexes or patch scripts. Existing lookups, rollbacks and purges never read the new tables, and after a downgrade they simply sit unused. The ClickHouse partition key is unchanged.