You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "zstan (via GitHub)" <gi...@apache.org> on 2023/06/07 13:04:22 UTC
[GitHub] [ignite-3] zstan opened a new pull request, #2162: IGNITE-19493 Sql. Change query execution flow
zstan opened a new pull request, #2162:
URL: https://github.com/apache/ignite-3/pull/2162
(no 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.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #2162: IGNITE-19493 Sql. Change query execution flow
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2162:
URL: https://github.com/apache/ignite-3/pull/2162#discussion_r1221608888
##########
modules/runner/src/integrationTest/sql/types/timestamp/test_incorrect_timestamp.test:
##########
@@ -1,7 +1,6 @@
# name: test/sql/types/timestamp/test_incorrect_timestamp.test
# description: Test out of range/incorrect timestamp formats
# group: [timestamp]
-# Ignore https://issues.apache.org/jira/browse/IGNITE-15623
Review Comment:
Unrelated change.
--
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-3] AMashenkov commented on a diff in pull request #2162: IGNITE-19493 Sql. Change query execution flow
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2162:
URL: https://github.com/apache/ignite-3/pull/2162#discussion_r1221607145
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##########
@@ -425,6 +421,18 @@ private CompletableFuture<AsyncSqlCursor<List<Object>>> querySingle0(
.thenCompose(sqlNode -> {
boolean rwOp = dataModificationOp(sqlNode);
+ boolean implicitTxRequired = outerTx == null;
+
+ tx.set(implicitTxRequired ? txManager.begin(!rwOp) : outerTx);
+
+ SchemaPlus schema = sqlSchemaManager.schema(schemaName);
Review Comment:
SqlSchemaManager methods without version or timestamp must not be used, and it is better to drop them 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-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ignite-3] zstan commented on a diff in pull request #2162: IGNITE-19493 Sql. Change query execution flow
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #2162:
URL: https://github.com/apache/ignite-3/pull/2162#discussion_r1221610186
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##########
@@ -425,6 +421,18 @@ private CompletableFuture<AsyncSqlCursor<List<Object>>> querySingle0(
.thenCompose(sqlNode -> {
boolean rwOp = dataModificationOp(sqlNode);
+ boolean implicitTxRequired = outerTx == null;
+
+ tx.set(implicitTxRequired ? txManager.begin(!rwOp) : outerTx);
+
+ SchemaPlus schema = sqlSchemaManager.schema(schemaName);
Review Comment:
I can`t use this implementation for now :
org.apache.ignite.internal.sql.engine.schema.SqlSchemaManagerImpl#activeSchema
```
@Override
public SchemaPlus activeSchema(@Nullable String name, long timestamp) {
throw new UnsupportedOperationException();
}
```
--
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-3] zstan commented on a diff in pull request #2162: IGNITE-19493 Sql. Change query execution flow
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #2162:
URL: https://github.com/apache/ignite-3/pull/2162#discussion_r1221628218
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##########
@@ -425,6 +421,18 @@ private CompletableFuture<AsyncSqlCursor<List<Object>>> querySingle0(
.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 IgniteInternalException(SCHEMA_NOT_FOUND_ERR,
Review Comment:
done
--
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-3] AMashenkov commented on a diff in pull request #2162: IGNITE-19493 Sql. Change query execution flow
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2162:
URL: https://github.com/apache/ignite-3/pull/2162#discussion_r1221608178
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##########
@@ -425,6 +421,18 @@ private CompletableFuture<AsyncSqlCursor<List<Object>>> querySingle0(
.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 IgniteInternalException(SCHEMA_NOT_FOUND_ERR,
Review Comment:
Let's introduce a top-level class SchemaNotFoundException in public API.
--
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-3] zstan commented on a diff in pull request #2162: IGNITE-19493 Sql. Change query execution flow
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #2162:
URL: https://github.com/apache/ignite-3/pull/2162#discussion_r1221611264
##########
modules/runner/src/integrationTest/sql/types/timestamp/test_incorrect_timestamp.test:
##########
@@ -1,7 +1,6 @@
# name: test/sql/types/timestamp/test_incorrect_timestamp.test
# description: Test out of range/incorrect timestamp formats
# group: [timestamp]
-# Ignore https://issues.apache.org/jira/browse/IGNITE-15623
Review Comment:
yep, but can it be here ? issue already fixed but mention still exist
--
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-3] AMashenkov commented on a diff in pull request #2162: IGNITE-19493 Sql. Change query execution flow
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #2162:
URL: https://github.com/apache/ignite-3/pull/2162#discussion_r1221602608
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/SqlQueryProcessor.java:
##########
@@ -425,6 +421,18 @@ private CompletableFuture<AsyncSqlCursor<List<Object>>> querySingle0(
.thenCompose(sqlNode -> {
boolean rwOp = dataModificationOp(sqlNode);
+ boolean implicitTxRequired = outerTx == null;
+
+ tx.set(implicitTxRequired ? txManager.begin(!rwOp) : outerTx);
+
+ SchemaPlus schema = sqlSchemaManager.schema(schemaName);
Review Comment:
```suggestion
SchemaPlus schema = sqlSchemaManager.activeSchema(schemaName, tx.startTimestamp().longValue());
```
--
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-3] zstan closed pull request #2162: IGNITE-19493 Sql. Change query execution flow
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan closed pull request #2162: IGNITE-19493 Sql. Change query execution flow
URL: https://github.com/apache/ignite-3/pull/2162
--
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