korlov42 commented on a change in pull request #9671:
URL: https://github.com/apache/ignite/pull/9671#discussion_r788851578
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/logical/ExposeIndexRule.java
##########
@@ -71,7 +71,10 @@ private static boolean preMatch(IgniteLogicalTableScan scan)
{
.map(idx -> idx.toRel(cluster, optTable, proj, condition,
requiredCols))
.collect(Collectors.toList());
- assert !indexes.isEmpty();
+ indexes.removeIf(Objects::isNull);
Review comment:
perhaps, it would be a bit better to filter out null with a stream API
`.filter(Objects::nonNull)` before collecting them to a `indexes` collection
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteIndexScan.java
##########
@@ -126,7 +121,9 @@ private IgniteIndexScan(
/** {@inheritDoc} */
@Override protected RelWriter explainTerms0(RelWriter pw) {
return super.explainTerms0(pw)
- .itemIf("sourceId", sourceId, sourceId != -1);
+ .itemIf("sourceId", sourceId, sourceId != -1)
+ .item("collation", collation())
+ .item("idxCollation", idxCollation);
Review comment:
why do we need both `collation` and `idxCollation`?
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteIndexScan.java
##########
@@ -39,13 +41,18 @@
/** */
private final long sourceId;
+ /** */
+ private final RelCollation idxCollation;
+
/**
* Constructor used for deserialization.
*
* @param input Serialized representation.
*/
public IgniteIndexScan(RelInput input) {
- super(changeTraits(input, IgniteConvention.INSTANCE));
+ super(changeTraits(input, IgniteConvention.INSTANCE,
input.getCollation()));
Review comment:
I don't sure it's a good idea to restore traits after deserialization.
We either should do this for every node or don't do it at all.
Well, there is a few nodes (e.g. Exchange) which have an assertion inside
their constructor, so `changeTraits` is used to overcome this problem. But for
other cases I would prefer to avoid using it
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteIndexScan.java
##########
@@ -39,13 +41,18 @@
/** */
private final long sourceId;
+ /** */
+ private final RelCollation idxCollation;
Review comment:
let's rename this to just `collation`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
