Add sql federation enumerable modify rule #28762
Conversation
|
|
||
| @Override | ||
| public Collection<Object[]> getModifiableCollection() { | ||
| return new LinkedList<>(); |
There was a problem hiding this comment.
Why return empty list here?
There was a problem hiding this comment.
If the implementation of the getModifiableCollection method returns null, it will trigger a NullPointerException for the EnumerableTableModify class. However, for our implemented class EnumerableModify, it is allowed to return null and it should not call the getModifiableCollection method in its future implementations.
There was a problem hiding this comment.
I have changed the return type of this method.
# Conflicts: # kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/operator/physical/EnumerableModify.java # kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/planner/rule/converter/EnumerableModifyConverterRule.java # kernel/sql-federation/optimizer/src/main/java/org/apache/shardingsphere/sqlfederation/optimizer/planner/util/SQLFederationPlannerUtils.java # kernel/sql-federation/optimizer/src/test/resources/cases/federation-delete-sql-cases.xml
| public RelNode convert(final RelNode rel) { | ||
| TableModify modify = (TableModify) rel; | ||
| ModifiableTable modifiableTable = modify.getTable().unwrap(ModifiableTable.class); | ||
| if (modifiableTable == null) { |
There was a problem hiding this comment.
Please put null condition on the left side.
|
|
||
| @Override | ||
| public Collection<Object[]> getModifiableCollection() { | ||
| return null; |
There was a problem hiding this comment.
If this method does not support now, can we throw UnsupportedOperationExcetion?
| @Override | ||
| public Result implement(final EnumerableRelImplementor implementor, final Prefer prefer) { | ||
| // TODO generate delete statements based on the data set and related table information. | ||
| return null; |
There was a problem hiding this comment.
If this method does not support now, can we throw UnsupportedOperationExcetion?
Fixes #ISSUSE_ID.
Changes proposed in this pull request:
Before committing this PR, I'm sure that I have checked the following options:
./mvnw clean install -B -T1C -Dmaven.javadoc.skip -Dmaven.jacoco.skip -e.