[PR] Refactor database-aware DataNode parsing [shardingsphere]

["terrymanu (via GitHub)" <gi...@apache.org>]

terrymanu opened a new pull request, #39615:
URL: https://github.com/apache/shardingsphere/pull/39615

   (no comment)


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Refactor database-aware DataNode parsing [shardingsphere]

["terrymanu (via GitHub)" <gi...@apache.org>]

terrymanu commented on PR #39615:
URL: https://github.com/apache/shardingsphere/pull/39615#issuecomment-5435718174

   ### Result
   
   **Review Result: Mergeable**
   
   The production changes are correct, behavior-preserving where intended, behavior-fixing where claimed, and protected by focused regression tests that all pass. The bulk of the added lines are updates to the repository's existing `.codex/` agent-policy harness and `AGENTS.md`, which are already version-controlled on master, so this is an established (if title-under-described) dev-branch merge rather than accidental tooling leakage.
   
   ### Evidence
   
   - `DataNode` refactor is behavior-preserving. `infra/common/src/main/java/org/apache/shardingsphere/infra/datanode/DataNode.java:75-97` replaces the double-split `isValidDataNode` precheck with a single candidate split. I traced 2-segment, 3-segment, 4+-segment, and invalid-delimiter inputs for both schema-available (PostgreSQL) and schema-unavailable (MySQL) dialects against the pre-change implementation; outcomes (dataSource/schema/asterisk-default/table-suffix) are identical in every case. The new `postgresql_dotted_table_suffix` case in `DataNodeTest.java:151` locks in the 4-segment behavior.
   - CDC selector parsing fixes are real. `CDCSchemaTableUtils.java:72` changes `putAll` to `mergeSchemaTables`, so consecutive wildcard-schema expressions (e.g. `*.t_order` then `*.t_order_item`) no longer drop earlier results — covered by `assertParseTableExpressionsMergesSchemaWildcardResults`. `CDCSchemaTableUtils.java:78` replaces a would-be NPE with `SchemaNotFoundException` for an explicit missing schema; the remaining `putAll` in the `schema.*` branch is safe because the replacement is always a superset for the same schema key.
   - Job startup validation is sound. `CDCJobAPI.java:261-285` re-validates the persisted configuration in `start()`, and on `PipelineInvalidParameterException` stops the job, disables it unless already disabled, and preserves cleanup failures as suppressed exceptions. Both validation helpers (`checkDataSources`, `checkSchemaTableNames`) throw only `PipelineInvalidParameterException`, so the cleanup catch is complete. Moving `PipelineJobRegistry.stop` from `CDCBackendHandler.startStreaming` into `start()` is safe: `stop()` is a documented no-op for unregistered jobs (`PipelineJobRegistry.java:74-78`), and the happy-path ordering (stop → add → enable → execute) is equivalent to the old handler-side stop.
   - Frontend exception wrapping is a strict improvement. `CDCChannelInboundHandler.java:205-218` catches `ShardingSphereSQLException` (superset of the old `PipelineSQLException`) for STREAM_DATA and adds the same wrapping for START_STREAMING, converting validation and `SchemaNotFoundException` failures into request-correlated FAILED responses. `CDCExceptionWrapper` extends `RuntimeException`, so privilege failures (`checkPrivileges`) keep their existing propagation path. `JDBCBackendStatementTest` simplification is safe: the removed parameterized variants never used the `databaseTypeName` parameter.
   - Verification: `./mvnw -pl infra/common,kernel/data-pipeline/scenario/cdc/core,proxy/frontend/core,proxy/backend/core -Dtest=DataNodeTest,CDCJobAPITest,CDCBackendHandlerTest,CDCSchemaTableUtilsTest,CDCChannelInboundHandlerTest,JDBCBackendStatementTest test` → exit 0, 126 tests, 0 failures. Harness `python3 -m unittest test_run` → 46 tests OK; `run.py`/`test_run.py` parse and both TOML files load; no local absolute paths in added lines; headerless new `.md` files match both existing `.codex` practice and the RAT `**/*.md` exclusion.
   
   ### Coverage
   
   - Reviewed head `bfa2c53465d` (equals local HEAD); merge-base `f016aa5cd90` (GitHub base ref `master` at `8e4afc5db4`). The authoritative GitHub 30-file list matches the local triple-dot list exactly.
   - Behavior clusters reviewed: DataNode database-aware parsing; CDC schema-table selector parsing; CDC job lifecycle validation and cleanup; CDC frontend exception-to-response wrapping; JDBC test simplification; `.codex`/`AGENTS.md` policy-harness updates (syntax, self-tests, reference resolution, RAT compliance).
   - All three discovery lenses (root cause/behavior, blast radius/contracts, tests/runtime/operations) completed; convergence pass after the last candidate produced no new findings. No public API, SPI, protocol, or configuration-key changes; `DataNode` constructor semantics preserved for all dialect paths traced.
   - Non-blocking observation: the PR title describes only the final commit; roughly 2,300 of 2,535 added lines are `.codex`/`AGENTS.md` policy-harness commits included from the `dev` branch. Consider a more representative title or description for release-note traceability.
   - Unresolved gaps: none that could change the blocker set. This is a Code Correctness Review: the result is code-scope only and CI was not reviewed.


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Refactor database-aware DataNode parsing [shardingsphere]

["terrymanu (via GitHub)" <gi...@apache.org>]

terrymanu merged PR #39615:
URL: https://github.com/apache/shardingsphere/pull/39615


-- 
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: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org