IGNITE-27431 SQL Calcite: Optimize scans with filter by alex-plekhanov · Pull Request #12600 · apache/ignite

alex-plekhanov · 2025-12-22T17:28:26Z

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

There is a single JIRA ticket related to the pull request.
The web-link to the pull request is attached to the JIRA ticket.
The JIRA ticket has the Patch Available state.
The pull request body describes changes that have been made.
The description explains WHAT and WHY was made instead of HOW.
The pull request title is treated as the final commit message.
The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
A reviewer has been mentioned through the JIRA comments
(see the Maintainers list)
The pull request has been checked by the Teamcity Bot and
the green visa attached 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.

.../java/org/apache/ignite/internal/processors/query/calcite/exec/AbstractCacheColumnsScan.java

...src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableRowIterable.java

Vladsz83 · 2026-01-02T15:30:57Z

.../java/org/apache/ignite/internal/processors/query/calcite/exec/AbstractCacheColumnsScan.java

-    /** Participating columns. */
-    protected final ImmutableBitSet requiredColumns;
+    /** Row field to column mapping. */
+    protected final int[] fieldColMapping;


Suggestion: it looks bit confusing. Let's rename pameters and fields like this everywhere to something like:
rawToRelColMapping and/or relToRawColMapping

Here we have mapping from node row fields to table row columns. We have identical naming in IndexScan: idxFieldMapping - from index keys to row fields, fieldIdxMapping - from row fields to index keys.
Maybe just extend the javadoc?

Vladsz83 · 2026-01-02T15:35:29Z

...src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableRowIterable.java

+     * @param nodeRow Relational node row.
+     * @param fieldColMapping Mapping from node row fields to table row columns. If column is not requried
+     *        corresponding field has -1 mapped value.
+     * @return Enriched relational node row.


If we return a relational row, why the mapping above is from node row to table row. The suggested renaming of fieldColMapping should also help with this.

Output row fields (node row) is a subset of input row fields (table row), it's faster to iterate over output row mapping, so we have mapping from output to input.

Vladsz83 · 2026-01-02T16:41:39Z

...src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableRowIterable.java

+ * @param <TableRow>> Raw table row type.
+ * @param <Row>> Relational node row type.
+ */
+public interface TableRowIterable<TableRow, Row> extends Iterable<Row> {


Do we need this new extra interface? Looks like we already have a bunch of scan interfaces and abstract classes. Can it be inside AbstractCacheColumnsScan?

Why not? We don't have any scan interfaces except this new one. Interfaces are more flexible, maybe in future we can implement another scan, not inherited from AbstractCacheColumnsScan, with row enrichment (currently system view scans, table function scans and spool scans don't give any benefits with row enriching iterator)

...alcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexScan.java

...src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableRowIterable.java

Vladsz83 · 2026-01-09T10:28:35Z

@alex-plekhanov , could you please merge master?

# Conflicts: # modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/AbstractCacheScan.java # modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java # modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableScan.java # modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/ProjectableFilterableTableScan.java # modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/AbstractPlannerTest.java

Vladsz83 · 2026-01-20T04:54:05Z

...ite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/ScanNode.java

+    /**
+     * @return Next row, or {@code null} if row was filtered out.
+     */
+    protected Row processNextRow() {


Suggestion: if it is able to produce null, let's mark the method result @Nullable

Vladsz83 · 2026-01-20T05:02:10Z

...ite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/ScanNode.java


    /** */
-    private Iterator<Row> it;
+    protected Iterator<?> it;


It is a src's iterator. Why not Row-typed?

For ScanTableRowNode it's <TableRow> iterator, not <Row>, while src - still <Row> iterable (and, additionaly TableRowIterable)

Vladsz83 · 2026-01-20T05:02:52Z

...ite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/ScanNode.java

    }

+    /**
+     * @return Next row, or {@code null} if row was filtered out.


Minor doc. 'Next row, or' -> 'Next row or'

AFAIK comma is required according to english grammer.

...main/java/org/apache/ignite/internal/processors/query/calcite/exec/rel/ScanTableRowNode.java

...ain/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java

...java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementorTest.java

sonarqubecloud · 2026-01-23T06:55:30Z

Quality Gate failed

Failed conditions
22 New Code Smells (required ≤ 1)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

alex-plekhanov force-pushed the ignite-27431 branch from 2994376 to b516462 Compare December 25, 2025 14:46

IGNITE-27431 SQL Calcite: Optimize scans with filter

96b2606

alex-plekhanov force-pushed the ignite-27431 branch from b516462 to 96b2606 Compare December 26, 2025 08:53

Vladsz83 reviewed Jan 9, 2026

View reviewed changes

alex-plekhanov added 4 commits January 12, 2026 11:23

IGNITE-27431 Review comments fixed

5913dbb

IGNITE-27431 Fix after merge

6b4ec6b

IGNITE-27431 Review comments fixed

08fcefa

Vladsz83 reviewed Jan 20, 2026

View reviewed changes

IGNITE-27431 Review comments fixed

da6cefc

Vladsz83 approved these changes Jan 21, 2026

View reviewed changes

...java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementorTest.java Outdated Show resolved Hide resolved

IGNITE-27431 Review comments fixed

15770aa

asf-gitbox-commits closed this in 8907ef8 Jan 23, 2026

Conversation

alex-plekhanov commented Dec 22, 2025

The Contribution Checklist

Notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Vladsz83 commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-plekhanov Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Jan 23, 2026

Quality Gate failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alex-plekhanov Jan 20, 2026 •

edited

Loading