AMashenkov commented on code in PR #2164:
URL: https://github.com/apache/ignite-3/pull/2164#discussion_r1226680971
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##########
@@ -407,70 +423,83 @@ private CompletableFuture<AsyncSqlCursor<List<Object>>>
querySingle0(
CompletableFuture<Void> start = new CompletableFuture<>();
+ boolean implicitTxRequired = outerTx == null;
AtomicReference<InternalTransaction> tx = new AtomicReference<>();
CompletableFuture<AsyncSqlCursor<List<Object>>> stage = start
- .thenApply(v -> {
- StatementParseResult parseResult =
IgniteSqlParser.parse(sql, StatementParseResult.MODE);
- SqlNode sqlNode = parseResult.statement();
-
- validateParsedStatement(context, outerTx, parseResult,
sqlNode, params);
-
- return sqlNode;
- })
- .thenCompose(sqlNode -> {
- boolean rwOp = dataModificationOp(sqlNode);
-
- boolean implicitTxRequired = outerTx == null;
-
- tx.set(implicitTxRequired ? txManager.begin(!rwOp) :
outerTx);
-
- SchemaPlus schema = sqlSchemaManager.schema(schemaName);
-
- if (schema == null) {
- return CompletableFuture.failedFuture(new
SchemaNotFoundException(schemaName));
- }
-
- BaseQueryContext ctx = BaseQueryContext.builder()
- .frameworkConfig(
-
Frameworks.newConfigBuilder(FRAMEWORK_CONFIG)
- .defaultSchema(schema)
- .build()
- )
+ .thenCompose(v -> {
+ Builder contextBuilder = BaseQueryContext.builder()
.logger(LOG)
.cancel(queryCancel)
.parameters(params)
- .plannerTimeout(PLANNER_TIMEOUT)
- .build();
-
- return prepareSvc.prepareAsync(sqlNode, ctx)
- .thenApply(plan -> {
- var dataCursor =
executionSrvc.executePlan(tx.get(), plan, ctx);
-
- SqlQueryType queryType = plan.type();
- assert queryType != null : "Expected a full
plan but got a fragment: " + plan;
-
- return new AsyncSqlCursorImpl<>(
- queryType,
- plan.metadata(),
- implicitTxRequired ? tx.get() : null,
- new AsyncCursor<List<Object>>() {
- @Override
- public
CompletableFuture<BatchedResult<List<Object>>> requestNextAsync(int rows) {
- session.touch();
-
- return
dataCursor.requestNextAsync(rows);
- }
-
- @Override
- public CompletableFuture<Void>
closeAsync() {
- session.touch();
-
- return dataCursor.closeAsync();
- }
- }
- );
- });
+ .plannerTimeout(PLANNER_TIMEOUT);
+
+ CompletableFuture<PlanWithContext>[] newPlanHolder = new
CompletableFuture[1];
+
+ CompletableFuture<QueryPlan> cachedPlan =
queryCache.computeIfAbsent(new CacheKey(schemaName, sql, params), (k) -> {
Review Comment:
The lamda that is passed to `queryCache.computeIfAbsent` is run inside the
synchronized block. For queries, which shouldn't be cached (like DDL), this
maybe overkill.
The lamda that is passed to `queryCache.computeIfAbsent` is run inside the
synchronized block. For queries, which shouldn't be cached (like DDL), this
maybe overkill.
`queryCache.computeIfAbsent` already returns the future. Thus, there is no
need to do any long operation inside the synchronized block.
Instead, you can always return a future from lamda for QUERY and DML and put
computation to another thread. For non-QUERY or non-DML just return null and do
computation in the current thread. Or maybe avoid using compute at all.
--
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-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
