terrymanu commented on PR #39786: URL: https://github.com/apache/shardingsphere/pull/39786#issuecomment-5855172791
Conclusion: **The response does not overturn the review.** The author can propose narrowing the supported scope, but the “well under 0.01%” figure is an estimate without supporting data. [SQL Server documentation](https://learn.microsoft.com/en-us/sql/relational-databases/databases/database-identifiers?view=sql-server-ver17) explicitly specifies `]]` for a right bracket within an identifier. The current [configuration parser](https://github.com/apache/shardingsphere/blob/1f6acc58b41ceaf2a5f6f0bde0183555fa587aab/kernel/linked-server/api/src/main/java/org/apache/shardingsphere/linkedserver/config/rule/LinkedServerConfiguration.java#L61-L81) exits bracket mode at the first `]`, so it incorrectly splits the valid name `[HR]]v.2].dbo.Department`. The code already uses a stateful scanner; the claim that support would require complex scanners across every parsing path is not established by the current diff. The response also does not address the other part of the finding. SQL Server treats `HumanResources` and `[HumanResources]` as the same identifier, but the configuration preserves their original strings and [lookup uses exact string matching](https://github.com/apache/shardingsphere/blob/1f6acc58b41ceaf2a5f6f0bde0183555fa587aab/kernel/linked-server/core/src/main/java/org/apache/shardingsphere/linkedserver/rule/table/LinkedServerTable.java#L42-L55). This allows one remote table to map to two different logical tables, contrary to the [#39313 contract that each remote identity maps to at most one logical table](https://github.com/apache/shardingsphere/issues/39313). Even if maintainers decide to defer `]]` support, that mapping conflict still needs to be resolved. Jev: I did not use the typesafe-ai skill or call Jev MCP (0 calls). This assessment required checking the full code path and public contract rather than making a bounded classification. Jev did not affect the conclusion; any efficiency change cannot be measured accurately, and no Jev-related problems were found. -- 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]
