Skip to content

[Misc] Add generic extension points to reduce cross-branch conflicts - #18683

Merged
JackieTien97 merged 20 commits into
masterfrom
sync-generic-changes
Oct 8, 2026
Merged

JackieTien97 merged 20 commits into
masterfrom
sync-generic-changes

Conversation

@shuwenwei

Copy link
Copy Markdown
Member

This PR bundles a set of small, self-contained and behavior-preserving changes that are introduced ahead of time so that long-lived feature branches do not conflict with master. It only adds extension points / placeholders and does not change existing behavior.

  • Information schema: add an SPI extension point and provider registry so extra tables/columns can be registered without touching the built-in schema.
  • ConfigNode: allow additional persistence/snapshot processors to be injected through ConfigManager.ConfigManagerContext (new protected hooks).
  • RPC: reserve a block of status codes for future use.
  • Auth: version the user/role profile file format with an extensible extra-segment region; drop the unused Role/User serialize/deserialize.
  • Query: log a set of expected status codes at info level, reserve snapshot/procedure/plan type ids, and add the SetColumnProperties execution flow (AST -> task -> executor -> ConfigNode) with its SQL formatter/visitor hooks.

No behavior change for existing statements; the new extension points and reserved identifiers are inert until a feature uses them.

@JackieTien97 JackieTien97 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for two compatibility regressions in existing Pipe paths: V3 auth snapshots sent to older receivers, and mixed-table InsertRows forwarded as single requests.

Validation: 72 focused unit tests passed, with additional new/old profile-reader, procedure-serialization and information-schema SPI probes. The findings were reproduced using real serialization and request/reader code; a complete cross-version two-cluster run was not performed.

// Now it's version 1
protected static final int VERSION = 2;
// Version 3 appends an extra segment region after the RBAC privileges.
protected static final int VERSION = 3;

@JackieTien97 JackieTien97 Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve the legacy profile format for older Pipe receivers

The default writer now emits V3 even when no extension segment is present, while auth snapshots are still transferred verbatim using the existing ConfigSnapshot protocol. If the sender is upgraded before its Pipe receiver, the receiver's old CNPhysicalPlanGenerator consumes the two session fields only for tag == 2, so it misreads V3's default -1/-1 session limits as privilege data. An independently compiled pre-PR reader produced 13 extra GrantUser plans, all with grant option, for the same unprivileged user whose V2 profile produced none. Please retain the complete V2 format when extensions are unused, or negotiate/transcode the profile format before sending it to an older receiver.

Comment on lines +117 to +120
if (!fromPipeBatch) {
return rowStatements.isEmpty()
? Collections.emptySet()
: Collections.singleton(resolveTargetTable(rowStatements.get(0), context));

@JackieTien97 JackieTien97 Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Validate every table in single-request Pipe row plans

An unmarked InsertRows is not necessarily single-table. Pipe batching before #18577 grouped rows only by database and could create a legitimate table1/table2/table1 plan; with sink.batch.enable=false, a cascading Pipe forwards that original InsertNode as one V2 request. The single-request decoder does not set fromPipeBatch, so this branch checks only table1, although subsequent schema/device validation still processes every row. A probe using the real serialization and request constructor checked both tables on the merge base but only table1 here; a normal rejection for table2 was skipped and schema validation was entered. Please retain all-row target deduplication, or maintain the invariant through every producer and rebuilder, including single/binary transfers.

@sonarqubecloud

sonarqubecloud Bot commented Oct 8, 2026

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@codecov

codecov Bot commented Oct 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 27.13816% with 443 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.52%. Comparing base (40e7ce6) to head (a8c7bc9).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...l/schema/table/AbstractSetPropertiesProcedure.java 21.83% 68 Missing ⚠️
...e/plan/relational/sql/ast/SetColumnProperties.java 0.00% 42 Missing ⚠️
.../plan/execution/config/TableConfigTaskVisitor.java 0.00% 37 Missing ⚠️
...onfignode/manager/schema/ClusterSchemaManager.java 0.00% 30 Missing ⚠️
...chema/table/SetTableColumnPropertiesProcedure.java 0.00% 25 Missing ⚠️
.../org/apache/iotdb/commons/lbac/RequiredLabels.java 0.00% 23 Missing ⚠️
...ion/config/executor/ClusterConfigTaskExecutor.java 0.00% 22 Missing ⚠️
...uest/write/table/SetTableColumnPropertiesPlan.java 0.00% 20 Missing ⚠️
...iotdb/commons/auth/role/LocalFileRoleAccessor.java 32.00% 17 Missing ⚠️
...ipe/receiver/protocol/IoTDBConfigNodeReceiver.java 0.00% 14 Missing ⚠️
... and 34 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #18683      +/-   ##
============================================
- Coverage     45.57%   45.52%   -0.05%     
  Complexity      712      712              
============================================
  Files          5483     5496      +13     
  Lines        396291   396642     +351     
  Branches      51567    51608      +41     
============================================
- Hits         180590   180582       -8     
- Misses       215701   216060     +359     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JackieTien97
JackieTien97 merged commit d20082b into master Oct 8, 2026
38 of 42 checks passed
@JackieTien97
JackieTien97 deleted the sync-generic-changes branch October 8, 2026 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants