IGNITE-27431 SQL Calcite: Optimize scans with filter #12600
Conversation
alex-plekhanov
force-pushed
the
ignite-27431
branch
from
2994376 to
b516462
Compare
alex-plekhanov
force-pushed
the
ignite-27431
branch
from
b516462 to
96b2606
Compare
.../java/org/apache/ignite/internal/processors/query/calcite/exec/AbstractCacheColumnsScan.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableRowIterable.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableRowIterable.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableRowIterable.java
Outdated
Show resolved
Hide resolved
| /** Participating columns. */ | ||
| protected final ImmutableBitSet requiredColumns; | ||
| /** Row field to column mapping. */ | ||
| protected final int[] fieldColMapping; |
There was a problem hiding this comment.
Suggestion: it looks bit confusing. Let's rename pameters and fields like this everywhere to something like:
rawToRelColMapping and/or relToRawColMapping
There was a problem hiding this comment.
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?
| * @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. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| * @param <TableRow>> Raw table row type. | ||
| * @param <Row>> Relational node row type. | ||
| */ | ||
| public interface TableRowIterable<TableRow, Row> extends Iterable<Row> { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
Show resolved
Hide resolved
| * @param <TableRow>> Raw table row type. | ||
| * @param <Row>> Relational node row type. | ||
| */ | ||
| public interface TableRowIterable<TableRow, Row> extends Iterable<Row> { |
There was a problem hiding this comment.
Naming. If we convert, the name might be smth. like TableRawIterableConverter or TableRowFilterableIterable
There was a problem hiding this comment.
But we don't convert here
|
@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
|
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 [email protected] or ask anу advice on http://asf.slack.com #ignite channel.