mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-08-18 20:18:30 +00:00
dennis zhuang
09c0b23a23
* fix(meta): actually acquire logical table locks in alter-logical-tables procedure The procedure listed its logical table locks from table_info_values, which is only filled during Prepare, while procedure lock keys are fixed at submission — so the logical locks were never acquired. Today every writer of a logical table's info is serialized by the physical table lock, which hides the problem; a metadata-only alter procedure targeting a single logical table would race it. Resolve the logical table ids at submission, persist them in the procedure state (serde(default): state dumped by older versions keeps the previous behavior), lock physical + logical tables, and re-check the resolved ids against the locked set at Prepare so a table dropped and recreated after submission cannot be mutated without a lock. Signed-off-by: Dennis Zhuang <killme2008@gmail.com> * feat: manage semantic table options via ALTER TABLE SET/UNSET CREATE TABLE accepts greptime.semantic.* options, but ALTER TABLE SET routed every option through SetRegionOption, whose closed match rejects them — tables auto-created by ingestion could never receive semantic declarations after the fact. Semantic options are pure metadata markers no region consumes, so they now take a metadata-only alter, following the repartition-hint precedent: - New AlterKind::SetAnnotations/UnsetAnnotations carrying an AnnotationFamily (currently only Semantic), so future marker-style option families reuse the same machinery. The converter classifies a SET/UNSET batch by key prefix and rejects batches that mix annotation keys with regular options. - The procedure reuses the MetadataOnly flow: no region dispatch, table-info update plus cache invalidation only. - Validation lives in the table-meta mutation layer, so it runs at frontend verification and again in the procedure's prepare step under the table lock: SET is strict (known key, value domain, entity columns exist and render as strings); UNSET is lenient inside the namespace so stale keys can be cleaned up. ModifyColumnTypes re-checks columns referenced by entity declarations at the same layer, closing a verify-then-execute race. - Logical metric tables are supported: an annotation alter submits a regular alter-table task locking only the logical table, and the DDL manager's physical-route guard admits it. - create_table_info re-checks semantic value domains for gRPC-built expressions that bypass the SQL parser. Signed-off-by: Dennis Zhuang <killme2008@gmail.com> * refactor(table): centralize annotation option classification and validation Address review feedback on the AnnotationFamily abstraction: with only one variant that every consumer immediately destructured, the generality was fake. Make it real and exhaustive instead: - AnnotationFamily gains RepartitionHint: repartition.column.hint is the same kind of marker option (pure metadata, no region consumes it) and previously had a hand-rolled special case in the converter, the metadata-only classifier, and a dedicated AlterKind pair — all deleted, one classification API remains. Per-family logical-table eligibility (allows_logical_tables) replaces the hard-coded Semantic check in the DDL manager guard. - One validation core in the table crate (check_annotation) serves both DDL entry points. CREATE and ALTER previously duplicated the rules; each keeps its existing error variants, status codes and messages via thin adapters over a typed error (ALTER missing column stays 4002 TableColumnNotFound, CREATE stays InvalidArguments). - The batch classifier returns Result instead of swallowing the mixed-batch error: a mixed SET on a logical table now reports the actual problem instead of UnexpectedLogicalRouteTable, and the flow classifiers propagate instead of guessing. The converter also moves its owned payloads instead of cloning them. Signed-off-by: Dennis Zhuang <killme2008@gmail.com> * test(meta): cover logical-table annotation alter routing The route-guard branch admitting metadata-only annotation alters on logical tables was only exercised end to end by sqlness. Pin it at the DDL manager level: a semantic SET on a logical table succeeds, updates only the logical table's metadata and dispatches nothing to datanodes; a mixed batch reports its own error instead of the route guard's; the repartition hint stays rejected on logical routes. Signed-off-by: Dennis Zhuang <killme2008@gmail.com> * fix(table): keep entity guard on ADD COLUMN and report missing columns first Review follow-ups: the old verify_alter loop scanned the post-alter schema, so it also caught DROP COLUMN followed by re-adding the declared column with a non-string type — the mutation-layer move only kept the MODIFY path. Guard add_columns the same way (this also covers ingestion auto-alter). And run the MODIFY drift check after the existence lookup, so altering a dropped-but-still-declared column reports ColumnNotExists (4002) like every other MODIFY on a missing column. Signed-off-by: Dennis Zhuang <killme2008@gmail.com> * style(grpc-expr): drop a test comment restating the classifier doc Signed-off-by: Dennis Zhuang <killme2008@gmail.com> * refactor(table): rename annotation validation helpers per review check_annotation* validated and normalized; align the names with the validate_and_normalize_* convention nearby, and spell out AnnotationContext (Cx is not used in this repo). Signed-off-by: Dennis Zhuang <killme2008@gmail.com> --------- Signed-off-by: Dennis Zhuang <killme2008@gmail.com>