ClaireLytt commented on PR #39786:
URL: https://github.com/apache/shardingsphere/pull/39786#issuecomment-5811523087
### Result
**Review Result: Mergeable**
From a pure code-correctness standpoint, no additional confirmed defects
were found in the current head. The remote table identity, enhanced-table
semantics, case handling, and test naming issues raised in the previous review
have all been fixed in the current code; the missing JDBC/Proxy runtime
dependencies have also been added. This conclusion covers code only and does
not judge CI or other merge gates.
### Evidence
- jdbc/pom.xml (lines 68–70) and proxy/backend/core/pom.xml (lines 92–94)
now include the shardingsphere-linked-server-core dependency; a dependency-path
check confirms this module is reachable from the JDBC and Proxy runtimes.
- LinkedServerConfiguration.java (lines 44–57) now validates that the remote
table identity has exactly three non-empty sections, and preserves table name
casing. LinkedServerTable.java (lines 40–55) now looks up tables by full
identity. Lookup failures for tables differing only in case are also covered by
tests.
- LinkedServerTableMapperRuleAttribute.java (lines 48–50) no longer marks
tables that only have identity mappings as enhanced tables.
- The test value naming issue flagged in the previous round has been fixed —
for example, OpenQueryUtilsTest now uses "actualReEncoded".
### Coverage
- Candidate: public PR #39786, head
7ee4bbfd5ec6720b55464189a4954557ecd20529, base
d613652c999968544ed7278e8e21897353e88d5c. The GitHub file list matches a local
three-way comparison, totaling 23 files; the latest commit only touches
jdbc/pom.xml and proxy/backend/core/pom.xml.
- Coverage included dependency/release wiring, configuration and remote
identity, runtime rules and mapping, YAML/SPI, OPENQUERY helper logic, tests,
and release notes; root-cause/behavior, impact/contract, test/runtime, and
final-convergence checks found no further independent code issues.
- Local style checks on the current head passed with exit code 0, running:
mvnw with modules kernel, kernel/linked-server, kernel/linked-server/api,
kernel/linked-server/core, jdbc, proxy/backend/core, profile "check", target
spotless:check checkstyle:check. Spotless covered 6 POM files and 14 Java files:
- POM: jdbc/pom.xml, kernel/pom.xml, kernel/linked-server/pom.xml,
kernel/linked-server/api/pom.xml, kernel/linked-server/core/pom.xml,
proxy/backend/core/pom.xml.
- API Java: LinkedServerRuleConfiguration.java,
LinkedServerConfiguration.java.
- Core Java: LinkedServerOrder.java, OpenQueryUtils.java,
LinkedServerRule.java, LinkedServerTableMapperRuleAttribute.java,
LinkedServerRuleBuilder.java, LinkedServerTable.java,
YamlLinkedServerRuleConfiguration.java, YamlLinkedServerConfiguration.java,
YamlLinkedServerRuleConfigurationSwapper.java.
- Test Java: OpenQueryUtilsTest.java, LinkedServerRuleTest.java,
YamlLinkedServerRuleConfigurationSwapperTest.java.
- Checkstyle covered the same 14 Java files above; it does not apply to
POM files. RELEASE-NOTES.md and the two META-INF/services descriptor files are
not governed by Spotless/Checkstyle.
--
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]
