You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/11/03 14:05:34 UTC

[GitHub] [ignite] gvvinblade opened a new pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

gvvinblade opened a new pull request #8421:
URL: https://github.com/apache/ignite/pull/8421


   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](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers)) 
   - [ ] The pull request has been checked by the Teamcity Bot and 
   the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
   
   ### Notes
   - [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
   - [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
   - [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
   - [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
   
   If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525865477



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/IdGenerator.java
##########
@@ -0,0 +1,14 @@
+package org.apache.ignite.internal.processors.query.calcite.prepare;
+
+import java.util.concurrent.atomic.AtomicLong;
+
+public class IdGenerator {

Review comment:
       1) Licence
   2) Class comment




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525871716



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteIndexScan.java
##########
@@ -77,12 +82,62 @@ public IgniteIndexScan(
         @Nullable List<RexNode> lowerIdxCond,
         @Nullable List<RexNode> upperIdxCond,
         @Nullable ImmutableBitSet requiredColunms
+    ) {
+        this(-1L, cluster, traits, tbl, idxName, proj, cond, lowerIdxCond, upperIdxCond, requiredColunms);
+    }
+
+    /**
+     * Creates a TableScan.
+     * @param cluster Cluster that this relational expression belongs to

Review comment:
       Empty line




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] asfgit closed pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #8421:
URL: https://github.com/apache/ignite/pull/8421


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
AMashenkov commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525044278



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/ColocationGroup.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.metadata;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.UUID;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.internal.GridDirectCollection;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.distributed.dht.topology.GridDhtPartitionState;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshalableMessage;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshallingContext;
+import org.apache.ignite.internal.processors.query.calcite.message.MessageType;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteRel;
+import org.apache.ignite.internal.processors.query.calcite.util.Commons;
+import org.apache.ignite.internal.util.GridIntList;
+import org.apache.ignite.internal.util.UUIDCollectionMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageCollectionItemType;
+import org.apache.ignite.plugin.extensions.communication.MessageReader;
+import org.apache.ignite.plugin.extensions.communication.MessageWriter;
+import org.jetbrains.annotations.NotNull;
+
+/** */
+public class ColocationGroup implements MarshalableMessage {
+    /** */
+    private static final int SYNTHETIC_PARTITIONS_COUNT = IgniteSystemProperties.getInteger("IGNITE_CALCITE_SYNTHETIC_PARTITIONS_COUNT", 512);
+
+    /** */
+    @GridDirectCollection(Long.class)
+    private List<Long> sourceIds;

Review comment:
       You can use sorted GridLongList or long[] here and avoid unnecessary boxing/unbonxing and reduce garbage.
   Combine and intersect operation are trivial and can preserve sorting.
   

##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ExecutionServiceImpl.java
##########
@@ -253,14 +254,14 @@ public FailureProcessor failureProcessor() {
     /**
      * @param partSvc Partition service.
      */
-    public void partitionService(PartitionService partSvc) {
+    public void partitionService(AffinityService partSvc) {

Review comment:
       Let's rename the method to "affinityService" as well




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] AMashenkov closed pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
AMashenkov closed pull request #8421:
URL: https://github.com/apache/ignite/pull/8421


   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525871716



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteIndexScan.java
##########
@@ -77,12 +82,62 @@ public IgniteIndexScan(
         @Nullable List<RexNode> lowerIdxCond,
         @Nullable List<RexNode> upperIdxCond,
         @Nullable ImmutableBitSet requiredColunms
+    ) {
+        this(-1L, cluster, traits, tbl, idxName, proj, cond, lowerIdxCond, upperIdxCond, requiredColunms);
+    }
+
+    /**
+     * Creates a TableScan.
+     * @param cluster Cluster that this relational expression belongs to

Review comment:
       Empty line before first @param and dots.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525859895



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ExecutionPlan.java
##########
@@ -0,0 +1,49 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+

Review comment:
       Redundant line.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525873485



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/SourceAwareIgniteRel.java
##########
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.rel;
+
+public interface SourceAwareIgniteRel extends IgniteRel {

Review comment:
       Comment needed.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525854866



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/FragmentMapping.java
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.metadata;
+
+import java.nio.ByteBuffer;
+import java.util.Collections;
+import java.util.List;
+import java.util.UUID;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
+import org.apache.ignite.internal.GridDirectCollection;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshalableMessage;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshallingContext;
+import org.apache.ignite.internal.processors.query.calcite.message.MessageType;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteRel;
+import org.apache.ignite.internal.processors.query.calcite.util.Commons;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.plugin.extensions.communication.MessageCollectionItemType;
+import org.apache.ignite.plugin.extensions.communication.MessageReader;
+import org.apache.ignite.plugin.extensions.communication.MessageWriter;
+import org.jetbrains.annotations.NotNull;
+
+public class FragmentMapping implements MarshalableMessage {

Review comment:
       Comment




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525864662



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/Fragment.java
##########
@@ -140,12 +121,64 @@ public String serialized() {
     }
 
     /** */
-    public boolean local() {
+    public boolean rootFragment() {
         return !(root instanceof IgniteSender);
     }
 
     /** */
-    private NodesMapping localMapping(PlanningContext ctx) {
-        return new NodesMapping(Collections.singletonList(ctx.localNodeId()), null, NodesMapping.CLIENT);
+    public Fragment attach(PlanningContext ctx) {
+        RelOptCluster cluster = ctx.cluster();
+
+        return root.getCluster() == cluster ? this : new Cloner(cluster).go(this);
+    }
+
+    /** */
+    public Fragment detach() {
+        RelOptCluster cluster = PlanningContext.empty().cluster();
+
+        return root.getCluster() == cluster ? this : new Cloner(cluster).go(this);
+    }
+
+    /**
+     * Mapps the fragment to its data location.
+     * @param ctx Planner context.

Review comment:
       Empty line.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525850171



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/ColocationGroup.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.metadata;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.UUID;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.internal.GridDirectCollection;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.distributed.dht.topology.GridDhtPartitionState;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshalableMessage;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshallingContext;
+import org.apache.ignite.internal.processors.query.calcite.message.MessageType;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteRel;
+import org.apache.ignite.internal.processors.query.calcite.util.Commons;
+import org.apache.ignite.internal.util.GridIntList;
+import org.apache.ignite.internal.util.UUIDCollectionMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageCollectionItemType;
+import org.apache.ignite.plugin.extensions.communication.MessageReader;
+import org.apache.ignite.plugin.extensions.communication.MessageWriter;
+import org.jetbrains.annotations.NotNull;
+
+/** */
+public class ColocationGroup implements MarshalableMessage {
+    /** */
+    private static final int SYNTHETIC_PARTITIONS_COUNT = IgniteSystemProperties.getInteger("IGNITE_CALCITE_SYNTHETIC_PARTITIONS_COUNT", 512);
+
+    /** */
+    @GridDirectCollection(Long.class)
+    private List<Long> sourceIds;
+
+    /** */
+    @GridDirectCollection(UUID.class)
+    private List<UUID> nodeIds;
+
+    /** */
+    @GridDirectTransient
+    private List<List<UUID>> assignments;
+
+    /** */
+    @GridDirectCollection(Message.class)
+    private List<UUIDCollectionMessage> assignments0;
+
+    public static ColocationGroup forNodes(List<UUID> nodeIds) {
+        return new ColocationGroup(null, nodeIds, null);
+    }
+
+    public static ColocationGroup forAssignments(List<List<UUID>> assignments) {
+        return new ColocationGroup(null, null, assignments);
+    }
+
+    public static ColocationGroup forSourceId(long sourceId) {
+        return new ColocationGroup(Collections.singletonList(sourceId), null, null);
+    }
+
+    /** */
+    public ColocationGroup() {
+    }
+
+    /** */
+    private ColocationGroup(List<Long> sourceIds, List<UUID> nodeIds, List<List<UUID>> assignments) {
+        this.sourceIds = sourceIds;
+        this.nodeIds = nodeIds;
+        this.assignments = assignments;
+    }
+
+    /**
+     * @return Lists of colocation group sources.
+     */
+    public List<Long> sourceIds() {
+        return sourceIds == null ? Collections.emptyList() : sourceIds;
+    }
+
+    /**
+     * @return Lists of nodes capable to execute a query fragment for what the mapping is calculated.
+     */
+    public List<UUID> nodeIds() {
+        return nodeIds == null ? Collections.emptyList() : nodeIds;
+    }
+
+    /**
+     * @return List of partitions (index) and nodes (items) having an appropriate partition in
+     * {@link GridDhtPartitionState#OWNING} state, calculated for distributed tables, involved in query execution.
+     */
+    public List<List<UUID>> assignments() {
+        return assignments == null ? Collections.emptyList() : assignments;
+    }
+
+    /**
+     * Prunes involved partitions (hence nodes, involved in query execution) on the basis of filter,
+     * its distribution, query parameters and original nodes mapping.
+     * @param rel Filter.
+     * @return Resulting nodes mapping.
+     */
+    public ColocationGroup prune(IgniteRel rel) {
+        return this; // TODO https://issues.apache.org/jira/browse/IGNITE-12455
+    }
+
+    public boolean belongs(long sourceId) {
+        return sourceIds != null && sourceIds.contains(sourceId);
+    }
+
+    /**
+     * Merges this mapping with given one.
+     * @param other Mapping to merge with.
+     * @return Merged nodes mapping.
+     * @throws ColocationMappingException If involved nodes intersection is empty, hence there is no nodes capable to execute
+     * being calculated fragment.
+     */
+    public ColocationGroup colocate(ColocationGroup other) throws ColocationMappingException {
+        List<Long> sourceIds;
+        if (this.sourceIds == null || other.sourceIds == null)
+            sourceIds = U.firstNotNull(this.sourceIds, other.sourceIds);
+        else
+            sourceIds = Commons.combine(this.sourceIds, other.sourceIds);
+
+        List<UUID> nodeIds;
+        if (this.nodeIds == null || other.nodeIds == null)
+            nodeIds = U.firstNotNull(this.nodeIds, other.nodeIds);
+        else
+            nodeIds = Commons.intersect(other.nodeIds, this.nodeIds);
+
+        if (nodeIds!= null && nodeIds.isEmpty())
+            throw new ColocationMappingException("Failed to map fragment to location. Replicated query parts are not co-located on all nodes");
+
+        List<List<UUID>> assignments;
+        if (this.assignments == null || other.assignments == null) {
+            assignments = U.firstNotNull(this.assignments, other.assignments);
+
+            if (assignments != null && nodeIds != null) {
+                Set<UUID> filter = new HashSet<>(nodeIds);
+                List<List<UUID>> assignments0 = new ArrayList<>(assignments.size());
+                for (int i = 0; i < assignments.size(); i++) {
+                    List<UUID> assignment = Commons.intersect(filter, assignments.get(i));
+
+                    if (assignment.isEmpty()) // TODO check with partition filters
+                        throw new ColocationMappingException("Failed to map fragment to location. Partition mapping is empty [part=" + i + "]");
+
+                    assignments0.add(assignment);
+                }
+
+                assignments = assignments0;
+            }
+        }
+        else {
+            assert this.assignments.size() == other.assignments.size();
+            assignments = new ArrayList<>(this.assignments.size());
+            Set<UUID> filter = nodeIds == null ? null : new HashSet<>(nodeIds);
+            for (int i = 0; i < this.assignments.size(); i++) {
+                List<UUID> assignment = Commons.intersect(this.assignments.get(i), other.assignments.get(i));
+
+                if (filter != null)
+                    assignment.retainAll(filter);
+
+                if (assignment.isEmpty()) // TODO check with partition filters
+                    throw new ColocationMappingException("Failed to map fragment to location. Partition mapping is empty [part=" + i + "]");
+
+                assignments.add(assignment);
+            }
+        }
+
+        return new ColocationGroup(sourceIds, nodeIds, assignments);
+    }
+
+    public ColocationGroup finalaze() {

Review comment:
       1) Comment needed.
   2) final**i**ze?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525854765



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/FragmentMapping.java
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+

Review comment:
       Redundant line




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525849964



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/ColocationGroup.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.metadata;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.UUID;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.internal.GridDirectCollection;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.distributed.dht.topology.GridDhtPartitionState;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshalableMessage;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshallingContext;
+import org.apache.ignite.internal.processors.query.calcite.message.MessageType;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteRel;
+import org.apache.ignite.internal.processors.query.calcite.util.Commons;
+import org.apache.ignite.internal.util.GridIntList;
+import org.apache.ignite.internal.util.UUIDCollectionMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageCollectionItemType;
+import org.apache.ignite.plugin.extensions.communication.MessageReader;
+import org.apache.ignite.plugin.extensions.communication.MessageWriter;
+import org.jetbrains.annotations.NotNull;
+
+/** */
+public class ColocationGroup implements MarshalableMessage {
+    /** */
+    private static final int SYNTHETIC_PARTITIONS_COUNT = IgniteSystemProperties.getInteger("IGNITE_CALCITE_SYNTHETIC_PARTITIONS_COUNT", 512);
+
+    /** */
+    @GridDirectCollection(Long.class)
+    private List<Long> sourceIds;
+
+    /** */
+    @GridDirectCollection(UUID.class)
+    private List<UUID> nodeIds;
+
+    /** */
+    @GridDirectTransient
+    private List<List<UUID>> assignments;
+
+    /** */
+    @GridDirectCollection(Message.class)
+    private List<UUIDCollectionMessage> assignments0;
+
+    public static ColocationGroup forNodes(List<UUID> nodeIds) {
+        return new ColocationGroup(null, nodeIds, null);
+    }
+
+    public static ColocationGroup forAssignments(List<List<UUID>> assignments) {
+        return new ColocationGroup(null, null, assignments);
+    }
+
+    public static ColocationGroup forSourceId(long sourceId) {
+        return new ColocationGroup(Collections.singletonList(sourceId), null, null);
+    }
+
+    /** */
+    public ColocationGroup() {
+    }
+
+    /** */
+    private ColocationGroup(List<Long> sourceIds, List<UUID> nodeIds, List<List<UUID>> assignments) {
+        this.sourceIds = sourceIds;
+        this.nodeIds = nodeIds;
+        this.assignments = assignments;
+    }
+
+    /**
+     * @return Lists of colocation group sources.
+     */
+    public List<Long> sourceIds() {
+        return sourceIds == null ? Collections.emptyList() : sourceIds;
+    }
+
+    /**
+     * @return Lists of nodes capable to execute a query fragment for what the mapping is calculated.
+     */
+    public List<UUID> nodeIds() {
+        return nodeIds == null ? Collections.emptyList() : nodeIds;
+    }
+
+    /**
+     * @return List of partitions (index) and nodes (items) having an appropriate partition in
+     * {@link GridDhtPartitionState#OWNING} state, calculated for distributed tables, involved in query execution.
+     */
+    public List<List<UUID>> assignments() {
+        return assignments == null ? Collections.emptyList() : assignments;
+    }
+
+    /**
+     * Prunes involved partitions (hence nodes, involved in query execution) on the basis of filter,
+     * its distribution, query parameters and original nodes mapping.
+     * @param rel Filter.
+     * @return Resulting nodes mapping.
+     */
+    public ColocationGroup prune(IgniteRel rel) {
+        return this; // TODO https://issues.apache.org/jira/browse/IGNITE-12455
+    }
+
+    public boolean belongs(long sourceId) {
+        return sourceIds != null && sourceIds.contains(sourceId);
+    }
+
+    /**
+     * Merges this mapping with given one.
+     * @param other Mapping to merge with.
+     * @return Merged nodes mapping.
+     * @throws ColocationMappingException If involved nodes intersection is empty, hence there is no nodes capable to execute
+     * being calculated fragment.
+     */
+    public ColocationGroup colocate(ColocationGroup other) throws ColocationMappingException {
+        List<Long> sourceIds;
+        if (this.sourceIds == null || other.sourceIds == null)
+            sourceIds = U.firstNotNull(this.sourceIds, other.sourceIds);
+        else
+            sourceIds = Commons.combine(this.sourceIds, other.sourceIds);
+
+        List<UUID> nodeIds;
+        if (this.nodeIds == null || other.nodeIds == null)
+            nodeIds = U.firstNotNull(this.nodeIds, other.nodeIds);
+        else
+            nodeIds = Commons.intersect(other.nodeIds, this.nodeIds);
+
+        if (nodeIds!= null && nodeIds.isEmpty())
+            throw new ColocationMappingException("Failed to map fragment to location. Replicated query parts are not co-located on all nodes");
+
+        List<List<UUID>> assignments;

Review comment:
       How about using a different name to avoid confusing with this.assignments?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525847575



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/metadata/ColocationGroup.java
##########
@@ -0,0 +1,342 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.metadata;
+
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import java.util.UUID;
+import org.apache.ignite.IgniteSystemProperties;
+import org.apache.ignite.internal.GridDirectCollection;
+import org.apache.ignite.internal.GridDirectTransient;
+import org.apache.ignite.internal.processors.cache.distributed.dht.topology.GridDhtPartitionState;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshalableMessage;
+import org.apache.ignite.internal.processors.query.calcite.message.MarshallingContext;
+import org.apache.ignite.internal.processors.query.calcite.message.MessageType;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteRel;
+import org.apache.ignite.internal.processors.query.calcite.util.Commons;
+import org.apache.ignite.internal.util.GridIntList;
+import org.apache.ignite.internal.util.UUIDCollectionMessage;
+import org.apache.ignite.internal.util.typedef.F;
+import org.apache.ignite.internal.util.typedef.internal.U;
+import org.apache.ignite.plugin.extensions.communication.Message;
+import org.apache.ignite.plugin.extensions.communication.MessageCollectionItemType;
+import org.apache.ignite.plugin.extensions.communication.MessageReader;
+import org.apache.ignite.plugin.extensions.communication.MessageWriter;
+import org.jetbrains.annotations.NotNull;
+
+/** */
+public class ColocationGroup implements MarshalableMessage {
+    /** */
+    private static final int SYNTHETIC_PARTITIONS_COUNT = IgniteSystemProperties.getInteger("IGNITE_CALCITE_SYNTHETIC_PARTITIONS_COUNT", 512);
+
+    /** */
+    @GridDirectCollection(Long.class)
+    private List<Long> sourceIds;
+
+    /** */
+    @GridDirectCollection(UUID.class)
+    private List<UUID> nodeIds;
+
+    /** */
+    @GridDirectTransient
+    private List<List<UUID>> assignments;
+
+    /** */
+    @GridDirectCollection(Message.class)
+    private List<UUIDCollectionMessage> assignments0;
+
+    public static ColocationGroup forNodes(List<UUID> nodeIds) {
+        return new ColocationGroup(null, nodeIds, null);
+    }
+
+    public static ColocationGroup forAssignments(List<List<UUID>> assignments) {
+        return new ColocationGroup(null, null, assignments);
+    }
+
+    public static ColocationGroup forSourceId(long sourceId) {
+        return new ColocationGroup(Collections.singletonList(sourceId), null, null);
+    }
+
+    /** */
+    public ColocationGroup() {
+    }
+
+    /** */
+    private ColocationGroup(List<Long> sourceIds, List<UUID> nodeIds, List<List<UUID>> assignments) {
+        this.sourceIds = sourceIds;
+        this.nodeIds = nodeIds;
+        this.assignments = assignments;
+    }
+
+    /**
+     * @return Lists of colocation group sources.
+     */
+    public List<Long> sourceIds() {
+        return sourceIds == null ? Collections.emptyList() : sourceIds;
+    }
+
+    /**
+     * @return Lists of nodes capable to execute a query fragment for what the mapping is calculated.
+     */
+    public List<UUID> nodeIds() {
+        return nodeIds == null ? Collections.emptyList() : nodeIds;
+    }
+
+    /**
+     * @return List of partitions (index) and nodes (items) having an appropriate partition in
+     * {@link GridDhtPartitionState#OWNING} state, calculated for distributed tables, involved in query execution.
+     */
+    public List<List<UUID>> assignments() {
+        return assignments == null ? Collections.emptyList() : assignments;
+    }
+
+    /**
+     * Prunes involved partitions (hence nodes, involved in query execution) on the basis of filter,
+     * its distribution, query parameters and original nodes mapping.
+     * @param rel Filter.
+     * @return Resulting nodes mapping.
+     */
+    public ColocationGroup prune(IgniteRel rel) {
+        return this; // TODO https://issues.apache.org/jira/browse/IGNITE-12455
+    }
+
+    public boolean belongs(long sourceId) {
+        return sourceIds != null && sourceIds.contains(sourceId);
+    }
+
+    /**
+     * Merges this mapping with given one.
+     * @param other Mapping to merge with.
+     * @return Merged nodes mapping.
+     * @throws ColocationMappingException If involved nodes intersection is empty, hence there is no nodes capable to execute
+     * being calculated fragment.
+     */
+    public ColocationGroup colocate(ColocationGroup other) throws ColocationMappingException {
+        List<Long> sourceIds;
+        if (this.sourceIds == null || other.sourceIds == null)
+            sourceIds = U.firstNotNull(this.sourceIds, other.sourceIds);
+        else
+            sourceIds = Commons.combine(this.sourceIds, other.sourceIds);
+
+        List<UUID> nodeIds;
+        if (this.nodeIds == null || other.nodeIds == null)
+            nodeIds = U.firstNotNull(this.nodeIds, other.nodeIds);
+        else
+            nodeIds = Commons.intersect(other.nodeIds, this.nodeIds);
+
+        if (nodeIds!= null && nodeIds.isEmpty())

Review comment:
       Space before !=




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525872935



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rel/IgniteTableScan.java
##########
@@ -53,7 +58,27 @@ public IgniteTableScan(
         RelTraitSet traits,
         RelOptTable tbl
     ) {
-        super(cluster, traits, ImmutableList.of(), tbl);
+        this(cluster, traits, tbl, null, null, null);
+    }
+
+    /**
+     * Creates a TableScan.
+     * @param cluster Cluster that this relational expression belongs to

Review comment:
       Empty line and dots.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [ignite] Berkof commented on a change in pull request #8421: IGNITE-13541 Calcite integration. Support of Date/Time types

Posted by GitBox <gi...@apache.org>.
Berkof commented on a change in pull request #8421:
URL: https://github.com/apache/ignite/pull/8421#discussion_r525862532



##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/FieldsMetadata.java
##########
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.processors.query.calcite.prepare;
+
+import java.util.List;
+import com.google.common.collect.ImmutableList;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.ignite.internal.processors.query.GridQueryFieldMetadata;
+import org.apache.ignite.internal.processors.query.calcite.type.IgniteTypeFactory;
+import org.apache.ignite.internal.util.typedef.F;
+
+/**
+ *
+ */
+public interface FieldsMetadata {
+    /**
+     * @return Result row type.
+     */
+    RelDataType rowType();
+
+    /**
+     * @return Result row origins (or where a field value comes from).
+     */
+    List<List<String>> origins();
+
+    /**
+     * @param typeFactory Type factory.
+     * @return Query field descriptors collection&

Review comment:
       **.** instead of **&**




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org