terrymanu commented on issue #24781:
URL:
https://github.com/apache/shardingsphere/issues/24781#issuecomment-3620692447
Issue Understanding
- #24781 is a feature request: current H2 tests use mocks, which may hide
real behavior (see #24780 NPE); propose using an embedded H2 server for tests.
- Linked #24780 (closed bug): metadata loading missed columns due to H2
identifier uppercasing, causing ORDER BY merge NPE.
Root Cause
- #24780 root cause: H2 uppercases unquoted identifiers; table-name
filtering in metadata queries didn’t align cases, so column metadata was empty
(table null).
- Tests missed this because mocked ResultSets returned lowercase
table/column names, not matching H2 INFORMATION_SCHEMA uppercase behavior.
Analysis
- Current master H2MetaDataLoader builds queries with UPPER(TABLE_NAME)
and uppercases inputs—fixing #24780; tests already assert uppercase SQL. No
known remaining defect of this type.
- ShardingSphere practice: dialect metadata loader unit tests stay mocked
for speed and stability; end-to-end behavior is handled by e2e/IT, not by
replacing all unit tests with embedded DBs.
- Swapping “all H2 tests” to embedded H2 would slow CI and add env
coupling (version/settings like DATABASE_TO_UPPER/CASE_INSENSITIVE). The cited
gap (case) is already fixed and can be covered by adjusting mocks (return
uppercase names) or a small targeted integration
check—no need for a broad change.
- Info is insufficient to scope the request: missing target modules/cases,
configs (DATABASE_TO_UPPER/CASE_INSENSITIVE?), acceptable runtime, and any
reproducible issue on current master.
Conclusion
- No current functional defect seen; #24780 is addressed. A blanket move
to embedded H2 tests isn’t necessary and diverges from the project’s
lightweight test convention.
--
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]
