IGNITE-12692 SQL Calcite: Distributed table modify #12593
Conversation
alex-plekhanov
force-pushed
the
ignite-12692
branch
from
8ad2bb1 to
a523525
Compare
alex-plekhanov
force-pushed
the
ignite-12692
branch
from
a523525 to
e5116e8
Compare
...alcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/type/OtherType.java
Outdated
Show resolved
Hide resolved
|
|
|
||
| /** | ||
| * Creates a ConverterRule. | ||
| */ | ||
| public TableModifyConverterRule() { |
There was a problem hiding this comment.
But TableModifySingleNodeConverterRule is instance of ConverterRule, nothing wrong here.
...pache/ignite/internal/processors/query/calcite/rule/TableModifyDistributedConverterRule.java
Show resolved
Hide resolved
...pache/ignite/internal/processors/query/calcite/rule/TableModifyDistributedConverterRule.java
Outdated
Show resolved
Hide resolved
...st/java/org/apache/ignite/internal/processors/query/calcite/planner/AbstractPlannerTest.java
Outdated
Show resolved
Hide resolved
...pache/ignite/internal/processors/query/calcite/rule/TableModifyDistributedConverterRule.java
Show resolved
Hide resolved
| // Check UPDATE statements. | ||
|
|
||
| // partitioned. | ||
| assertPlan("UPDATE test SET val = val + 1", schema, |
There was a problem hiding this comment.
It looks really too short check. Maybe we need to search for Excnanheg/Aggregates.
There was a problem hiding this comment.
We check full plan bellow TableModify, Above distributed table modify aggregates/exchange/project never change. Correctness of the full plan is verified by integration tests (TableDmlIntegrationTest at least)
|
|
||
| // partitioned. | ||
| assertPlan("UPDATE test SET val = val + 1", schema, | ||
| hasChildThat(isInstanceOf(IgniteTableModify.class).and(hasDistribution(IgniteDistributions.random())) |
There was a problem hiding this comment.
Why random(), not hash() ?
There was a problem hiding this comment.
It's an output distribution conaining only one column (count of affected rows), it's not hashed on any key.
| * Converts LogicalTableModify to distributed IgniteTableModify (Perform table modify on remote nodes, | ||
| * aggregate affected rows count and send result to the initiator node). | ||
| */ | ||
| public class TableModifyDistributedConverterRule extends AbstractIgniteConverterRule<LogicalTableModify> { |
There was a problem hiding this comment.
What if underlying cache has a node filer? Do we need to consider it?
There was a problem hiding this comment.
There are two cases:
- when TableModify affects data source, we only allow the same hash distribution on input as on modified table, so node filter on modified table and on input data will be the same.
- when TableModify doesn't affect data source, in this case we allow any distributed input and shouldn't be care about node filter.
| .and(input(isTableScan("TEST_REPL")))); | ||
|
|
||
| // broadcast (force distributed). | ||
| assertPlan("UPDATE test_repl SET val = val + 1", schema, |
There was a problem hiding this comment.
Can it duplicate or corrupt data? Or TrimExchange prevents?
There was a problem hiding this comment.
Yes, trim exchange prevents.
|
|
||
| /** Tests that table modify can be executed on remote nodes. */ | ||
| @Test | ||
| public void testDistributedTableModify() throws Exception { |
There was a problem hiding this comment.
Do we need tests with filters (WHERE )
There was a problem hiding this comment.
I've added one test for UPDATE with change indexed column, since it adds spool to the plan, in other cases this filter will not change anything.
|
|
||
| /** | ||
| * Creates a ConverterRule. | ||
| */ | ||
| public TableModifyConverterRule() { |
There was a problem hiding this comment.
But TableModifySingleNodeConverterRule is instance of ConverterRule, nothing wrong here.
2 participants
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.