Skip to content

[IGNITE-18225] Sql. Pushdown MODIFY to data node.#1798

Merged
AMashenkov merged 47 commits intoapache:mainfrom
gridgain:IGNITE-18225
Mar 28, 2023
Merged

[IGNITE-18225] Sql. Pushdown MODIFY to data node.#1798
AMashenkov merged 47 commits intoapache:mainfrom
gridgain:IGNITE-18225

Conversation

@lowka
Copy link
Contributor

@lowka lowka commented Mar 15, 2023

No description provided.

@lowka lowka changed the title Ignite 18225 [IGNITE-18225] Sql. Pushdown MODIFY to data node. Mar 15, 2023
@lowka lowka force-pushed the IGNITE-18225 branch 2 times, most recently from dcb9ffe to b030575 Compare March 18, 2023 10:31
@lowka lowka marked this pull request as ready for review March 20, 2023 13:02
int aggCallCnt = rowType.getFieldCount();
List<AggregateCall> aggCalls = new ArrayList<>();

for (int i = 0; i < aggCallCnt; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look reliable. For now it works just because ModifyNode returns exactly one column, but one of the possible changes is support for RETURNING clause (see this for details). With this in mind, I definitely would not take SUM over an arbitrary column

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@korlov42 Could you please point me how the RETURNING clause can be specified, since at the moment there is no support for RETURNING clause in neither Parser nor RelNodes (TableModify). I am 100% sure about the parser part, because it fails when you add a RETURNING clause.
If is there a ticket for RETURNING clause, I would add a link to it here.


RelDataType rowType = tableModify.getRowType();
RelDataTypeFactory typeFactory = cluster.getTypeFactory();
RelDataType sumType = typeFactory.createTypeWithNullability(typeFactory.createSqlType(SqlTypeName.DECIMAL), true);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better to use org.apache.calcite.rel.type.RelDataTypeSystem#deriveSumType to derive type of a SUM aggregate

Copy link
Contributor Author

@lowka lowka Mar 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@korlov42 This is not possible to use org.apache.calcite.rel.type.RelDataTypeSystem#deriveSumType since this method is going to derive an incorrect type and that is going to cause a type difference assertion to fire.
I am going to add a comment that clarifies this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

*/
@Test
public void testInsertIntoSingleDistributedTable() throws Exception {
IgniteTable test1 = createTable("TEST1", IgniteDistributions.single(), "C1", Integer.class, "C2", Integer.class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use TestBuilder framework to create test tables

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed.

# Conflicts:
#	modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java
@AMashenkov AMashenkov merged commit a041b6c into apache:main Mar 28, 2023
@AMashenkov AMashenkov deleted the IGNITE-18225 branch March 28, 2023 11:35
DefaultValueStrategy.DEFAULT_COMPUTED,
() -> UUID.randomUUID().toString()
() -> {
throw new AssertionError("Implicit primary key is generated by a function");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
throw new AssertionError("Implicit primary key is generated by a function");
throw new AssertionError("Implicit primary key is generated by the function");

lowka added a commit to gridgain/apache-ignite-3 that referenced this pull request Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants