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 2021/09/01 12:17:17 UTC
[GitHub] [ignite] tledkov-gridgain opened a new pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
tledkov-gridgain opened a new pull request #9369:
URL: https://github.com/apache/ignite/pull/9369
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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r702852920
##########
File path: modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java
##########
@@ -82,7 +84,7 @@ protected QueryChecker assertQuery(String qry) {
}
/** */
- protected IgniteCache<Integer, Employer> createAndPopulateTable() {
+ protected IgniteCache<Integer, Employer> createAndPopulateTable() throws InterruptedException {
Review comment:
Actually it's not throwing any InterruptedException. Please fix it and any test using this method
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r702870476
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/Fragment.java
##########
@@ -121,15 +122,8 @@ public boolean rootFragment() {
}
/** */
- 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();
+ public Fragment copy() {
+ RelOptCluster cluster = Commons.cluster();
return root.getCluster() == cluster ? this : new Cloner(cluster).go(this);
Review comment:
To preserve the old semantic we need to do a deep copy unconditionally now
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r702868969
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/QueryTemplate.java
##########
@@ -89,10 +91,11 @@ public ExecutionPlan map(PlanningContext ctx) {
}
/** */
- @NotNull private List<Fragment> map(List<Fragment> fragments, PlanningContext ctx, RelMetadataQuery mq) {
+ @NotNull private List<Fragment> map(List<Fragment> fragments, MappingQueryContext ctx, RelMetadataQuery mq) {
ImmutableList.Builder<Fragment> b = ImmutableList.builder();
+
for (Fragment fragment : fragments)
- b.add(fragment.map(mappingService, ctx, mq).detach());
+ b.add(fragment.map(mappingService, ctx, mq).copy());
Review comment:
look like this `copy()` is unnecessary here
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r702901070
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/PlanningContext.java
##########
@@ -17,131 +17,50 @@
package org.apache.ignite.internal.processors.query.calcite.prepare;
-import java.util.Properties;
-import java.util.UUID;
import java.util.function.Function;
-import org.apache.calcite.config.CalciteConnectionConfig;
-import org.apache.calcite.config.CalciteConnectionConfigImpl;
-import org.apache.calcite.config.CalciteConnectionProperty;
-import org.apache.calcite.jdbc.CalciteSchema;
import org.apache.calcite.plan.Context;
import org.apache.calcite.plan.Contexts;
import org.apache.calcite.plan.RelOptCluster;
import org.apache.calcite.prepare.CalciteCatalogReader;
-import org.apache.calcite.rel.type.RelDataTypeSystem;
+import org.apache.calcite.rex.RexBuilder;
import org.apache.calcite.schema.SchemaPlus;
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.validate.SqlConformance;
import org.apache.calcite.tools.FrameworkConfig;
-import org.apache.calcite.tools.Frameworks;
import org.apache.calcite.tools.RuleSet;
-import org.apache.ignite.IgniteLogger;
-import org.apache.ignite.internal.processors.affinity.AffinityTopologyVersion;
-import org.apache.ignite.internal.processors.query.GridQueryCancel;
import org.apache.ignite.internal.processors.query.calcite.type.IgniteTypeFactory;
-import org.apache.ignite.logger.NullLogger;
import org.jetbrains.annotations.NotNull;
-import static org.apache.calcite.tools.Frameworks.createRootSchema;
-import static org.apache.ignite.internal.processors.query.calcite.CalciteQueryProcessor.FRAMEWORK_CONFIG;
-
/**
* Planning context.
*/
public final class PlanningContext implements Context {
- /** */
- private static final PlanningContext EMPTY = builder().build();
-
- /** */
- private final FrameworkConfig cfg;
-
/** */
private final Context parentCtx;
- /** */
- private final UUID locNodeId;
-
- /** */
- private final UUID originatingNodeId;
-
/** */
private final String qry;
/** */
private final Object[] parameters;
- /** */
- private final AffinityTopologyVersion topVer;
-
- /** */
- private final GridQueryCancel qryCancel;
-
- /** */
- private final IgniteLogger log;
-
- /** */
- private final IgniteTypeFactory typeFactory;
-
/** */
private Function<RuleSet, RuleSet> rulesFilter;
/** */
private IgnitePlanner planner;
- /** */
- private CalciteConnectionConfig connCfg;
-
- /** */
- private CalciteCatalogReader catalogReader;
-
/**
* Private constructor, used by a builder.
*/
private PlanningContext(
- FrameworkConfig cfg,
Context parentCtx,
- UUID locNodeId,
- UUID originatingNodeId,
String qry,
- Object[] parameters,
- AffinityTopologyVersion topVer,
- IgniteLogger log) {
- this.locNodeId = locNodeId;
- this.originatingNodeId = originatingNodeId;
+ Object[] parameters) {
this.qry = qry;
Review comment:
```suggestion
Object[] parameters
) {
this.qry = qry;
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r702893823
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/Commons.java
##########
@@ -174,24 +177,22 @@ public static IgniteTypeFactory typeFactory(RelOptCluster cluster) {
}
/**
- * Extracts planner context.
+ * Extracts query context.
*/
- public static PlanningContext context(RelNode rel) {
+ public static BaseQueryContext context(RelNode rel) {
return context(rel.getCluster());
}
/**
- * Extracts planner context.
+ * Extracts query context.
*/
- public static PlanningContext context(RelOptCluster cluster) {
- return context(cluster.getPlanner().getContext());
- }
-
- /**
- * Extracts planner context.
- */
- public static PlanningContext context(Context ctx) {
- return Objects.requireNonNull(ctx.unwrap(PlanningContext.class));
+ public static BaseQueryContext context(RelOptCluster cluster) {
+ try {
+ return Objects.requireNonNull((cluster.getPlanner().getContext().unwrap(BaseQueryContext.class)));
+ }
+ catch (NullPointerException e) {
+ throw e;
+ }
Review comment:
```suggestion
return Objects.requireNonNull(cluster.getPlanner().getContext().unwrap(BaseQueryContext.class));
```
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r702871674
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/Fragment.java
##########
@@ -139,7 +133,7 @@ public Fragment detach() {
* @param ctx Planner context.
* @param mq Metadata query.
*/
- Fragment map(MappingService mappingSrvc, PlanningContext ctx, RelMetadataQuery mq) throws FragmentMappingException {
+ Fragment map(MappingService mappingSrvc, MappingQueryContext ctx, RelMetadataQuery mq) throws FragmentMappingException {
assert root.getCluster() == ctx.cluster() : "Fragment is detached [fragment=" + this + "]";
Review comment:
currently this assertion will be always true, hence, probably, it's better to remove it. This allows us to remove `cluster` from a MappingQueryContext since this place is the only usage
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r702909999
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ExecutionContext.java
##########
@@ -183,20 +201,46 @@ public MvccSnapshot mvccSnapshot() {
}
/**
- * @return Originating node ID.
+ * @return Local node ID.
+ */
+ public UUID localNodeId() {
+ return locNodeId;
+ }
+
+ /**
+ * @return Originating node ID (the node, who started the execution).
*/
public UUID originatingNodeId() {
- return planningContext().originatingNodeId();
+ return originatingNodeId == null ? locNodeId : originatingNodeId;
Review comment:
I would prefer to have `originatingNodeId` set explicitly
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] tledkov-gridgain commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
tledkov-gridgain commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r701694635
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ExecutionServiceImpl.java
##########
@@ -667,12 +691,15 @@ private FieldsMetadata explainFieldsMetadata(PlanningContext ctx) {
plan.remotes(fragment));
ExecutionContext<Row> ectx = new ExecutionContext<>(
+ qctx,
taskExecutor(),
- pctx,
qryId,
+ locNodeId,
+ locNodeId,
+ topologyVersion(),
Review comment:
Thanks for attention! Fixed.
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] alex-plekhanov commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r701064387
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ExecutionServiceImpl.java
##########
@@ -462,23 +464,29 @@ protected AffinityTopologyVersion topologyVersion() {
/** */
private PlanningContext createContext(QueryContext ctx, @Nullable String schema, String qry, Object[] params) {
- return createContext(Commons.convert(ctx), topologyVersion(), localNodeId(), schema, qry, params);
+ return createContext(Commons.convert(ctx), topologyVersion(), schema, qry, params);
}
/** */
- private PlanningContext createContext(Context parent, AffinityTopologyVersion topVer, UUID originator,
+ private BaseQueryContext createQueryContext(Context parent, @Nullable String schema) {
+ return BaseQueryContext.builder()
+ .parentContext(parent)
+ .frameworkConfig(
+ Frameworks.newConfigBuilder(FRAMEWORK_CONFIG)
+ .defaultSchema(getDefaultSchema(schema))
+ .build()
+ )
+ .logger(log)
+ .build();
+ }
+
+ /** */
+ private PlanningContext createContext(Context parent, AffinityTopologyVersion topVer,
Review comment:
`topVer` is not required here anymore.
Method arguments declaration should be on one line, or each argument on their own line according to codestyle.
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/Commons.java
##########
@@ -176,22 +179,20 @@ public static IgniteTypeFactory typeFactory(RelOptCluster cluster) {
/**
* Extracts planner context.
*/
- public static PlanningContext context(RelNode rel) {
+ public static BaseQueryContext context(RelNode rel) {
return context(rel.getCluster());
}
/**
* Extracts planner context.
*/
- public static PlanningContext context(RelOptCluster cluster) {
- return context(cluster.getPlanner().getContext());
- }
-
- /**
- * Extracts planner context.
- */
- public static PlanningContext context(Context ctx) {
- return Objects.requireNonNull(ctx.unwrap(PlanningContext.class));
+ public static BaseQueryContext context(RelOptCluster cluster) {
Review comment:
Fix javadoc
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/BaseQueryContext.java
##########
@@ -0,0 +1,291 @@
+/*
+ * 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.Properties;
+import org.apache.calcite.config.CalciteConnectionConfig;
+import org.apache.calcite.config.CalciteConnectionConfigImpl;
+import org.apache.calcite.config.CalciteConnectionProperty;
+import org.apache.calcite.jdbc.CalciteSchema;
+import org.apache.calcite.plan.Context;
+import org.apache.calcite.plan.Contexts;
+import org.apache.calcite.plan.ConventionTraitDef;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelTraitDef;
+import org.apache.calcite.plan.volcano.VolcanoPlanner;
+import org.apache.calcite.prepare.CalciteCatalogReader;
+import org.apache.calcite.rel.RelCollationTraitDef;
+import org.apache.calcite.rel.metadata.CachingRelMetadataProvider;
+import org.apache.calcite.rel.type.RelDataTypeSystem;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.schema.SchemaPlus;
+import org.apache.calcite.tools.FrameworkConfig;
+import org.apache.calcite.tools.Frameworks;
+import org.apache.ignite.IgniteLogger;
+import org.apache.ignite.internal.processors.query.GridQueryCancel;
+import org.apache.ignite.internal.processors.query.calcite.metadata.IgniteMetadata;
+import org.apache.ignite.internal.processors.query.calcite.metadata.RelMetadataQueryEx;
+import org.apache.ignite.internal.processors.query.calcite.metadata.cost.IgniteCostFactory;
+import org.apache.ignite.internal.processors.query.calcite.trait.CorrelationTraitDef;
+import org.apache.ignite.internal.processors.query.calcite.trait.DistributionTraitDef;
+import org.apache.ignite.internal.processors.query.calcite.trait.RewindabilityTraitDef;
+import org.apache.ignite.internal.processors.query.calcite.type.IgniteTypeFactory;
+import org.apache.ignite.logger.NullLogger;
+import org.jetbrains.annotations.NotNull;
+
+import static org.apache.calcite.tools.Frameworks.createRootSchema;
+import static org.apache.ignite.internal.processors.query.calcite.CalciteQueryProcessor.FRAMEWORK_CONFIG;
+
+/**
+ * Base query context.
+ */
+public final class BaseQueryContext extends AbstractQueryContext {
+ /** */
+ private static final IgniteCostFactory COST_FACTORY = new IgniteCostFactory();
+
+ /** */
+ public static final CalciteConnectionConfig CALCITE_CONNECTION_CONFIG;
+
+ /** */
+ private static final BaseQueryContext EMPTY_CONTEXT;
+
+ /** */
+ private static final VolcanoPlanner EMPTY_PLANNER;
+
+ /** */
+ private static final RexBuilder DFLT_REX_BUILDER;
+
+ /** */
+ public static final RelOptCluster CLUSTER;
+
+ /** */
+ public static final IgniteTypeFactory TYPE_FACTORY;
+
+ static {
+ Properties props = new Properties();
+
+ props.setProperty(CalciteConnectionProperty.CASE_SENSITIVE.camelName(),
+ String.valueOf(FRAMEWORK_CONFIG.getParserConfig().caseSensitive()));
+ props.setProperty(CalciteConnectionProperty.CONFORMANCE.camelName(),
+ String.valueOf(FRAMEWORK_CONFIG.getParserConfig().conformance()));
+ props.setProperty(CalciteConnectionProperty.MATERIALIZATIONS_ENABLED.camelName(),
+ String.valueOf(true));
+
+ CALCITE_CONNECTION_CONFIG = new CalciteConnectionConfigImpl(props);
+
+ EMPTY_CONTEXT = builder().build();
+
+ EMPTY_PLANNER = new VolcanoPlanner(COST_FACTORY, EMPTY_CONTEXT);
+
+ RelDataTypeSystem typeSys = CALCITE_CONNECTION_CONFIG.typeSystem(RelDataTypeSystem.class, FRAMEWORK_CONFIG.getTypeSystem());
+ TYPE_FACTORY = new IgniteTypeFactory(typeSys);
+
+ DFLT_REX_BUILDER = new RexBuilder(TYPE_FACTORY);
+
+ CLUSTER = RelOptCluster.create(EMPTY_PLANNER, DFLT_REX_BUILDER);
+
+ CLUSTER.setMetadataProvider(new CachingRelMetadataProvider(IgniteMetadata.METADATA_PROVIDER, EMPTY_PLANNER));
+ CLUSTER.setMetadataQuerySupplier(RelMetadataQueryEx::create);
+ }
+
+ /** */
+ private final FrameworkConfig cfg;
+
+ /** */
+ private final IgniteLogger log;
+
+ /** */
+ private final IgniteTypeFactory typeFactory;
+
+ /** */
+ private final RexBuilder rexBuilder;
+
+ /** */
+ private CalciteCatalogReader catalogReader;
+
+ /** */
+ private final GridQueryCancel qryCancel;
+
+ /**
+ * Private constructor, used by a builder.
+ */
+ private BaseQueryContext(
+ FrameworkConfig cfg,
+ Context parentCtx,
+ IgniteLogger log
+ ) {
+ super(Contexts.chain(parentCtx, cfg.getContext()));
+
+ // link frameworkConfig#context() to this.
+ this.cfg = Frameworks.newConfigBuilder(cfg).context(this).build();
+
+ this.log = log;
+
+ RelDataTypeSystem typeSys = CALCITE_CONNECTION_CONFIG.typeSystem(RelDataTypeSystem.class, cfg.getTypeSystem());
+
+ typeFactory = new IgniteTypeFactory(typeSys);
+
+ qryCancel = unwrap(GridQueryCancel.class);
+
+ rexBuilder = new RexBuilder(typeFactory);
+ }
+
+ /**
+ * @return Framework config.
+ */
+ public FrameworkConfig config() {
+ return cfg;
+ }
+
+ /**
+ * @return Logger.
+ */
+ public IgniteLogger logger() {
+ return log;
+ }
+
+ /**
+ * @return Schema name.
+ */
+ public String schemaName() {
+ return schema().getName();
+ }
+
+ /**
+ * @return Schema.
+ */
+ public SchemaPlus schema() {
+ return cfg.getDefaultSchema();
+ }
+
+ /**
+ * @return Type factory.
+ */
+ public IgniteTypeFactory typeFactory() {
+ return typeFactory;
+ }
+
+ /** */
+ public RexBuilder rexBuilder() {
+ return rexBuilder;
+ }
+
+ /**
+ * @return New catalog reader.
+ */
+ public CalciteCatalogReader catalogReader() {
+ if (catalogReader != null)
+ return catalogReader;
+
+ SchemaPlus dfltSchema = schema(), rootSchema = dfltSchema;
+
+ while (rootSchema.getParentSchema() != null)
+ rootSchema = rootSchema.getParentSchema();
+
+ return catalogReader = new CalciteCatalogReader(
+ CalciteSchema.from(rootSchema),
+ CalciteSchema.from(dfltSchema).path(null),
+ typeFactory(), CALCITE_CONNECTION_CONFIG);
+ }
+
+ /**
+ * @return Query cancel.
+ */
+ public GridQueryCancel queryCancel() {
+ return qryCancel;
+ }
+
+ /**
+ * @return Context builder.
+ */
+ public static Builder builder() {
+ return new Builder();
+ }
+
+ /** */
+ public static BaseQueryContext empty() {
+ return EMPTY_CONTEXT;
+ }
+
+ /**
+ * Planner context builder.
+ */
+ @SuppressWarnings("PublicInnerClass")
+ public static class Builder {
+ /** */
+ private static final RelTraitDef<?>[] TRAIT_DEFS = {
+ ConventionTraitDef.INSTANCE,
+ RelCollationTraitDef.INSTANCE,
+ DistributionTraitDef.INSTANCE,
+ RewindabilityTraitDef.INSTANCE,
+ CorrelationTraitDef.INSTANCE,
+ };
+
+ /** */
+ private static final FrameworkConfig EMPTY_CONFIG =
+ Frameworks.newConfigBuilder(FRAMEWORK_CONFIG)
+ .defaultSchema(createRootSchema(false))
+ .traitDefs(TRAIT_DEFS)
Review comment:
`traitDefs` are already set in `FRAMEWORK_CONFIG`, it's not required to set it here.
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ExecutionServiceImpl.java
##########
@@ -640,8 +663,9 @@ private FieldsMetadata explainFieldsMetadata(PlanningContext ctx) {
}
/** */
- private ListFieldsQueryCursor<?> executeQuery(UUID qryId, MultiStepPlan plan, PlanningContext pctx) {
- plan.init(pctx);
+ private ListFieldsQueryCursor<?> mapAndExecutePlan(UUID qryId, MultiStepPlan plan, BaseQueryContext qctx,
Review comment:
Method arguments declaration should be on one line, or each argument on their own line according to codestyle.
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ExecutionServiceImpl.java
##########
@@ -667,12 +691,15 @@ private FieldsMetadata explainFieldsMetadata(PlanningContext ctx) {
plan.remotes(fragment));
ExecutionContext<Row> ectx = new ExecutionContext<>(
+ qctx,
taskExecutor(),
- pctx,
qryId,
+ locNodeId,
+ locNodeId,
+ topologyVersion(),
Review comment:
Topology version can be changed between `plan.init()` and `ectx` creation, in this case, query will be mapped to the wrong nodes and not fail on the execution step. Let's store `topologyVersion()` to variable and use it in both calls.
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/Commons.java
##########
@@ -176,22 +179,20 @@ public static IgniteTypeFactory typeFactory(RelOptCluster cluster) {
/**
* Extracts planner context.
*/
- public static PlanningContext context(RelNode rel) {
+ public static BaseQueryContext context(RelNode rel) {
Review comment:
Fix javadoc
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] korlov42 commented on a change in pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
korlov42 commented on a change in pull request #9369:
URL: https://github.com/apache/ignite/pull/9369#discussion_r702915075
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ExecutionServiceImpl.java
##########
@@ -726,12 +758,9 @@ private FieldsMetadata explainFieldsMetadata(PlanningContext ctx) {
}
/** */
- private void executeFragment(UUID qryId, FragmentPlan plan, PlanningContext pctx, FragmentDescription fragmentDesc) {
- ExecutionContext<Row> ectx = new ExecutionContext<>(taskExecutor(), pctx, qryId,
- fragmentDesc, handler, Commons.parametersMap(pctx.parameters()));
-
+ private void executeFragment(UUID qryId, FragmentPlan plan, FragmentDescription fragmentDesc, ExecutionContext<Row> ectx) {
long frId = fragmentDesc.fragmentId();
Review comment:
you could get a fragmentId from executionCtx
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite] tledkov-gridgain merged pull request #9369: IGNITE-15394 Calcite integration. Query contexts refactoring
Posted by GitBox <gi...@apache.org>.
tledkov-gridgain merged pull request #9369:
URL: https://github.com/apache/ignite/pull/9369
--
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: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org