terrymanu commented on issue #15689:
URL:
https://github.com/apache/shardingsphere/issues/15689#issuecomment-3515100429
Background and Context
After in-depth analysis, we have discovered the complete background and
root cause of this issue:
Discovery Process
1. User Feedback: Users encountered NPE when executing Oracle INSERT ALL
syntax with ShardingSphere-JDBC 5.1.0
2. Environment Comparison: The same configuration works normally on MySQL,
indicating this is an Oracle-specific issue
3. Source Code Tracing: Through analysis of ShardingSphere's SQL parsing
and binding process, we located the root cause
Core Problem
The issue stems from compatibility deficiencies in ShardingSphere when
handling Oracle-specific syntax:
1. Uniqueness of Oracle INSERT ALL Syntax
INSERT ALL INTO demo_sharding (...) VALUES (?,?,?,?,?,?,?,?,?) SELECT 1
FROM DUAL
- This is Oracle's proprietary multi-table insert syntax
- Completely different from standard INSERT INTO ... VALUES (...), (...)
syntax
- Values are not provided through standard VALUES clauses, but through
SELECT subqueries
2. Gaps in ShardingSphere Processing Flow
- Parsing Layer: Already correctly identifies and parses INSERT ALL
syntax (dedicated test cases exist)
- Binding Layer: Unable to correctly extract value expressions in
InsertStatementBaseContext.getAllValueExpressions()
- Parameter Layer: InsertStatementBindingContext cannot create parameter
contexts due to missing value expressions
3. Specific NPE Trigger Path
// InsertStatementBaseContext.java:169 - Cannot extract value expressions
here
getAllValueExpressionsFromValues(insertStatement.getValues()) // values is
empty collection
// InsertStatementBindingContext.java:134-140 - insertValueContexts is
empty
public List<List<Object>> getGroupedParameters() {
for (InsertValueContext each : insertValueContexts) { // Empty
collection loop
result.add(each.getParameters());
}
}
// Subsequent parameter access may trigger various null pointer issues
Existing Partial Improvements
We noticed a related improvement in the latest master branch:
- Commit d40551404fa (September 2024): Modified InsertStatement#getTable()
method to return Optional to adapt to Oracle multi-table insert statements
- Limitation: This improvement mainly solves table structure handling
issues but doesn't resolve the core parameter binding problem
Fix Solution Design
Fix Approach
We need to add complete support for Oracle INSERT ALL syntax in
ShardingSphere's binding layer, ensuring ability to:
1. Correctly identify multi-table insert statements
2. Extract value expressions from MultiTableInsertIntoSegment
3. Create correct parameter binding contexts
Specific Fix Solution
1. Modify InsertStatementBaseContext.getAllValueExpressions() Method
Add multi-table insert support in
infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/context/statement/type/dml/InsertStatementBaseContext.java:
private List<List<ExpressionSegment>> getAllValueExpressions(final
InsertStatement insertStatement) {
Optional<SetAssignmentSegment> setAssignment =
insertStatement.getSetAssignment();
if (setAssignment.isPresent()) {
return
Collections.singletonList(getAllValueExpressionsFromSetAssignment(setAssignment.get()));
}
// Check if it's a multi-table insert statement
if (insertStatement.getMultiTableInsertInto().isPresent()) {
return
getAllValueExpressionsFromMultiTableInsert(insertStatement.getMultiTableInsertInto().get());
}
return getAllValueExpressionsFromValues(insertStatement.getValues());
}
/**
* Extract value expressions from multi-table insert
*/
private List<List<ExpressionSegment>>
getAllValueExpressionsFromMultiTableInsert(final MultiTableInsertIntoSegment
multiTableInsertInto) {
List<List<ExpressionSegment>> result = new ArrayList<>();
for (InsertStatement each :
multiTableInsertInto.getInsertStatements()) {
if (!each.getValues().isEmpty()) {
for (InsertValuesSegment valueSegment : each.getValues()) {
result.add(valueSegment.getValues());
}
}
}
return result;
}
2. Enhance InsertStatementBindingContext Parameter Handling
Improve constructor in
infra/binder/core/src/main/java/org/apache/shardingsphere/infra/binder/context/statement/type/dml/InsertStatementBindingContext.java:
public InsertStatementBindingContext(final InsertStatementBaseContext
baseContext, final List<Object> params,
final ShardingSphereMetaData metaData,
final String currentDatabaseName) {
this.baseContext = baseContext;
AtomicInteger parametersOffset = new AtomicInteger(0);
// Handle multi-table insert special case
if
(baseContext.getSqlStatement().getMultiTableInsertInto().isPresent() &&
baseContext.getValueExpressions().isEmpty()) {
// For multi-table insert, extract parameters from
MultiTableInsertIntoSegment
insertValueContexts =
getInsertValueContextsForMultiTableInsert(baseContext, params,
parametersOffset);
} else {
insertValueContexts = getInsertValueContexts(params,
parametersOffset, baseContext.getValueExpressions());
}
insertSelectContext = getInsertSelectContext(params, parametersOffset,
metaData, currentDatabaseName).orElse(null);
onDuplicateKeyUpdateValueContext =
getOnDuplicateKeyUpdateValueContext(params, parametersOffset).orElse(null);
generatedKeyContext = new
GeneratedKeyContextEngine(baseContext.getSqlStatement(),
baseContext.getSchema())
.createGenerateKeyContext(baseContext.getInsertColumnNamesAndIndexes(),
insertValueContexts, params).orElse(null);
}
/**
* Create value contexts for multi-table insert
*/
private List<InsertValueContext>
getInsertValueContextsForMultiTableInsert(final InsertStatementBaseContext
baseContext,
final List<Object> params,
final AtomicInteger paramsOffset) {
List<InsertValueContext> result = new LinkedList<>();
MultiTableInsertIntoSegment multiTableInsertInto =
baseContext.getSqlStatement().getMultiTableInsertInto().get();
for (InsertStatement insertStatement :
multiTableInsertInto.getInsertStatements()) {
for (InsertValuesSegment valuesSegment :
insertStatement.getValues()) {
InsertValueContext insertValueContext = new
InsertValueContext(valuesSegment.getValues(), params, paramsOffset.get());
result.add(insertValueContext);
paramsOffset.addAndGet(insertValueContext.getParameterCount());
}
}
return result;
}
3. Add Test Cases
Create complete test coverage, including unit tests and integration tests:
@Test
void assertInsertAllStatementBindingContext() {
// Mock INSERT ALL statement
InsertStatement insertStatement = createMockInsertAllStatement();
ShardingSphereMetaData metaData = createMockMetaData();
InsertStatementBaseContext baseContext = new
InsertStatementBaseContext(insertStatement, metaData, "test_db");
List<Object> params = Arrays.asList(1, "test", "content", "remark",
100, System.currentTimeMillis(), 101, System.currentTimeMillis(), 1);
InsertStatementBindingContext bindingContext = new
InsertStatementBindingContext(baseContext, params, metaData, "test_db");
// Verify parameter grouping is correct
assertThat(bindingContext.getGroupedParameters().size(), is(1));
assertThat(bindingContext.getGroupedParameters().get(0).size(), is(9));
}
@Test
void assertOracleInsertAllWithParameters() {
// Integration test: Simulate complete Oracle INSERT ALL execution flow
String sql = "INSERT ALL INTO demo_sharding (id,name,content) VALUES
(?,?,?) SELECT 1 FROM DUAL";
List<Object> params = Arrays.asList(1, "test", "content");
// Test complete SQL parsing and binding process
InsertStatementContext context = createInsertStatementContext(sql,
params);
assertThat(context.getGroupedParameters().size(), is(1));
assertThat(context.getGroupedParameters().get(0), is(params));
}
🤝 Warm Invitation to Submit PR
We sincerely hope you can participate in fixing this issue! As an open
source project, ShardingSphere's growth depends on the contributions of every
community member. This is a great opportunity to deeply understand
ShardingSphere's internal mechanisms while bringing real value to the
entire community.
Development Guidance Steps
1. Preparation
# Fork repository to your GitHub account
git clone https://github.com/YOUR_USERNAME/shardingsphere.git
cd shardingsphere
git checkout dev
git checkout -b fix/oracle-insert-all-npe
2. Implement Changes
- Modify InsertStatementBaseContext.java according to the above solution
- Modify constructor of InsertStatementBindingContext.java
- Add necessary helper methods
- Create complete test cases
3. Local Verification
# Run related module tests to ensure existing functionality is not broken
./mvnw test -pl infra/binder/core -am
./mvnw test -pl parser/sql/engine/dialect/oracle -am
# Run code formatting
./mvnw spotless:apply
# Ensure all tests pass
./mvnw install -T1C -DskipTests
./mvnw test
4. Commit and Create PR
git add .
git commit -m "Fix NPE when executing Oracle INSERT ALL statements
(#issue_number)"
git push origin fix/oracle-insert-all-npe
Value of Contributing
By completing this fix, you will gain:
- Technical Growth: Deep understanding of ShardingSphere's SQL parsing,
binding, and execution mechanisms
- Community Recognition: Your name in ShardingSphere's contributor list
- Real Impact: Solving real problems for all Oracle users
- Collaboration Experience: Valuable experience working with global
developers
- Open Source Spirit: Contributing your strength to the open source
ecosystem
Support and Help
If you encounter any difficulties during development:
- Technical Questions: Welcome to continue asking in this issue
- Development Guidance: Can refer to ShardingSphere's official
contribution guidelines
- Code Reference: Can look at other similar PR implementations
- Community Support: The ShardingSphere community is very willing to help
new contributors
🚀 Let's Make ShardingSphere Stronger Together
This fix not only solves the specific problem you encountered but, more
importantly, helps all ShardingSphere users using Oracle databases. Every
community contribution pushes this project forward, and we sincerely look
forward to seeing your PR!
Let's build a more powerful and complete ShardingSphere together!
--
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]
