github-actions[bot] commented on code in PR #68785:
URL: https://github.com/apache/doris/pull/68785#discussion_r4226489721
##########
gensrc/AGENTS.md:
##########
@@ -0,0 +1,29 @@
+# AGENTS.md — gensrc (thrift / protobuf)
+
+Everything under `gensrc/thrift` and `gensrc/proto` is a wire format between
FE, BE and the cloud services; `olap_file.proto` and `segment_v2.proto` are
also on-disk formats that every later release reads back. A field id or an enum
value that has shipped on any branch is taken for good: a field that lands on
another field of the same wire type is read as that field, and an enum member
is just an integer on the wire, so neither mismatch produces an error.
+
+## Rules
+
+1. Append only. A new field takes the next unused id of its struct; a new enum
member goes after the last one. Never insert into the middle of an enum, and
never fill a gap: an id that any live release branch uses for a different field
is taken on master too.
+2. Never reuse an id. To drop a field, keep its id taken: in thrift leave the
field in place marked `// Deprecated` (`TQueryOptions` 1) or comment the line
out with its id (`TQueryOptions` 11); in proto list the number under
`reserved`. Older proto messages renamed the field to `DEPRECATED_<name>`
instead (`ColumnMetaPB` 14), which also keeps the number. A dropped enum member
keeps its value the same way.
+3. A field picked to a release branch keeps the id it has on master. If that
id is already taken on the branch, renumber on master first so the same option
never has two ids on two lines. `TQueryOptions` 183-231 on branch-4.1/4.2
against master is what happens otherwise; #68785 is the cleanup.
+4. Changing what a field means is a new field. Keep the type and id for a pure
rename only; a new meaning gets a new id even when the old field is dropped in
the same PR. `TabletSchemaPB` 24 went from `cluster_key_idxes` (column index)
to `cluster_key_uids` (column unique id) in place, so a 4.0 BE reads a 3.1
tablet's indexes as uids.
+5. What a receiver does with an unset field is part of the contract: the
declared default, or the value an `__isset` / `has_` branch picks instead
(`QueryContext` uses 1.0 for an unset `max_scan_mem_ratio`, whose declared
default is 0.3). It only matters for a sender that does not set the field, such
as an older FE or a path that builds the struct without it, but changing it
changes what that sender means.
+6. Renaming is allowed: the wire and the on-disk binaries carry ids, not
names. Rename only when the meaning is unchanged, rename on every live branch
that has the field or not at all, and check the name-keyed paths (json2pb dumps
that `meta_tool load_meta` reads back, the cloud meta-service HTTP API) that
break for data serialized under the old name.
+
+## Before you submit
+
+Commit the change, then compare it with every release branch that still
receives picks (today: branch-4.2, branch-4.1, branch-4.0, branch-3.1; add a
new branch when it is cut). `<remote>` is the remote that points to
apache/doris (`upstream` in a fork clone, `origin` in a direct clone); fetch it
first. Put your own remote, file, struct, id and name in and paste the output
into the PR:
+
+ for ref in HEAD <remote>/branch-4.2 <remote>/branch-4.1
<remote>/branch-4.0 <remote>/branch-3.1; do
+ echo "== $ref"
+ git show "$ref:gensrc/thrift/<File>.thrift" | awk '/^struct <Struct>
/,/^}/' | grep -E '^\s*<id>:|\b<name>\b'
Review Comment:
[P2] Make the branch check fail when it cannot see an occupied ID. Even with
valid refs, the AWK range requires `^struct <Struct> `; it misses valid `struct
TListPrivilegesResult{` in `FrontendService.thrift` and indented ` struct
TPartitionSortNode {` in `PlanNodes.thrift`, so active ID 1 appears absent. The
grep also skips commented-out reservations such as `// 11:` in `TQueryOptions`.
Because line 23 treats empty output as an unused ID, this check can approve
reuse of active or reserved IDs. Parse the struct robustly, verify that the
block was found, and include tombstones before accepting absence.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
