alex-plekhanov commented on a change in pull request #9758:
URL: https://github.com/apache/ignite/pull/9758#discussion_r791806082
##########
File path:
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/MappingQueryContext.java
##########
@@ -45,4 +53,16 @@ public UUID localNodeId() {
public AffinityTopologyVersion topologyVersion() {
return topVer;
}
+
+ /** Creates a cluster. */
+ RelOptCluster cluster() {
+ if (cluster == null) {
+ cluster =
RelOptCluster.create(Commons.emptyCluster().getPlanner(),
Commons.emptyCluster().getRexBuilder());
Review comment:
The planner shouldn't be used by the empty cluster at all, there are
assertions to prevent this (I've looked at the fix to ignite-3, and looks like
the same check was implemented but in a different way, by prohibiting
registerSchema invocation). I can add the same check to the planner as well,
but now from my point of view, it looks redundant.
The current fix prohibits not only planning but also mapping (mapping use
metadataQuerySupplier, I'm not sure mapping uses planner somehow).
Also, I think the fix for ignite-3 is not complete, we should attach
fragments to the empty cluster again before storing it to `executionPlan`
variable (see
https://github.com/apache/ignite/pull/9758/files#diff-07fea6ecf0f238df45e8d6fdffabf83cf9a5fdbd90b31f897868a282c0694f0eR99),
otherwise cluster with cached metadata can't be garbage collected (referenced
from query cache -> query template -> execution plan -> fragments -> cluster)
--
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]
