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/02/06 15:33:24 UTC
[GitHub] [ignite] gvvinblade opened a new pull request #7377: IGNITE-12602
Calcite integration. JDBC Thin driver support.
gvvinblade opened a new pull request #7377: IGNITE-12602 Calcite integration. JDBC Thin driver support.
URL: https://github.com/apache/ignite/pull/7377
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] rkondakov commented on a change in pull request #7377:
IGNITE-12602 Calcite integration. JDBC Thin driver support.
Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7377: IGNITE-12602 Calcite integration. JDBC Thin driver support.
URL: https://github.com/apache/ignite/pull/7377#discussion_r378194065
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/message/MessageServiceImpl.java
##########
@@ -189,32 +187,16 @@ public FailureProcessor failureProcessor() {
}
/** {@inheritDoc} */
- @Override public void send(Collection<UUID> nodeIds, CalciteMessage msg) {
- for (UUID nodeId : nodeIds)
- send(nodeId, msg);
- }
-
- /** {@inheritDoc} */
- @Override public void send(UUID nodeId, CalciteMessage msg) {
+ @Override public void send(UUID nodeId, CalciteMessage msg) throws IgniteCheckedException {
if (localNodeId().equals(nodeId))
onMessage(nodeId, msg, true);
Review comment:
Can we get into some troubles (races?) here because of handling the message in the current thread instead of submitting it into the specified pool?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] rkondakov commented on a change in pull request #7377:
IGNITE-12602 Calcite integration. JDBC Thin driver support.
Posted by GitBox <gi...@apache.org>.
rkondakov commented on a change in pull request #7377: IGNITE-12602 Calcite integration. JDBC Thin driver support.
URL: https://github.com/apache/ignite/pull/7377#discussion_r378220740
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/odbc/jdbc/JdbcRequestHandler.java
##########
@@ -723,6 +745,21 @@ private JdbcResponse executeQuery(JdbcQueryExecuteRequest req) {
}
}
+ /** */
+ private List<FieldsQueryCursor<List<?>>> querySqlFields(SqlFieldsQueryEx qry, GridQueryCancel cancel) {
+ if (experimentalQueryEngine != null) {
+ try {
+ return experimentalQueryEngine.query(QueryContext.of(qry, cancel), qry.getSchema(), qry.getSql(), qry.getArgs());
+ }
+ catch (IgniteSQLException e) {
+ U.warn(log, "Failed to execute SQL query using experimental engine. [qry=" + qry + ']', e);
Review comment:
I think we should not write this warning to log for DDL/DML queries.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] gvvinblade commented on a change in pull request #7377:
IGNITE-12602 Calcite integration. JDBC Thin driver support.
Posted by GitBox <gi...@apache.org>.
gvvinblade commented on a change in pull request #7377: IGNITE-12602 Calcite integration. JDBC Thin driver support.
URL: https://github.com/apache/ignite/pull/7377#discussion_r380091480
##########
File path: modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/message/MessageServiceImpl.java
##########
@@ -189,32 +187,16 @@ public FailureProcessor failureProcessor() {
}
/** {@inheritDoc} */
- @Override public void send(Collection<UUID> nodeIds, CalciteMessage msg) {
- for (UUID nodeId : nodeIds)
- send(nodeId, msg);
- }
-
- /** {@inheritDoc} */
- @Override public void send(UUID nodeId, CalciteMessage msg) {
+ @Override public void send(UUID nodeId, CalciteMessage msg) throws IgniteCheckedException {
if (localNodeId().equals(nodeId))
onMessage(nodeId, msg, true);
Review comment:
We submit it to another thread (async=true)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] gvvinblade commented on a change in pull request #7377:
IGNITE-12602 Calcite integration. JDBC Thin driver support.
Posted by GitBox <gi...@apache.org>.
gvvinblade commented on a change in pull request #7377: IGNITE-12602 Calcite integration. JDBC Thin driver support.
URL: https://github.com/apache/ignite/pull/7377#discussion_r380094342
##########
File path: modules/core/src/main/java/org/apache/ignite/internal/processors/odbc/jdbc/JdbcRequestHandler.java
##########
@@ -723,6 +745,21 @@ private JdbcResponse executeQuery(JdbcQueryExecuteRequest req) {
}
}
+ /** */
+ private List<FieldsQueryCursor<List<?>>> querySqlFields(SqlFieldsQueryEx qry, GridQueryCancel cancel) {
+ if (experimentalQueryEngine != null) {
+ try {
+ return experimentalQueryEngine.query(QueryContext.of(qry, cancel), qry.getSchema(), qry.getSql(), qry.getArgs());
+ }
+ catch (IgniteSQLException e) {
+ U.warn(log, "Failed to execute SQL query using experimental engine. [qry=" + qry + ']', e);
Review comment:
Not sure, actually here we highlight an issue - it's been handled but still an issue and a user should be aware on it, because this mean you were wrong setting "useExperimentalEngine" flag to true.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [ignite] gvvinblade closed pull request #7377: IGNITE-12602 Calcite
integration. JDBC Thin driver support.
Posted by GitBox <gi...@apache.org>.
gvvinblade closed pull request #7377: IGNITE-12602 Calcite integration. JDBC Thin driver support.
URL: https://github.com/apache/ignite/pull/7377
----------------------------------------------------------------
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
With regards,
Apache Git Services