[GitHub] [ignite-3] AMashenkov commented on a diff in pull request #1831: IGNITE-18983 Sql. Jdbc. Migrate JDBC handler on new internal API

["AMashenkov (via GitHub)" <gi...@apache.org>]

AMashenkov commented on code in PR #1831:
URL: https://github.com/apache/ignite-3/pull/1831#discussion_r1147318689


##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java:
##########
@@ -90,62 +95,76 @@ public class JdbcQueryEventHandlerImpl implements JdbcQueryEventHandler {
      * @param meta JdbcMetadataInfo.
      * @param resources Client resources.
      */
-    public JdbcQueryEventHandlerImpl(QueryProcessor processor, JdbcMetadataCatalog meta,
-            ClientResourceRegistry resources) {
+    public JdbcQueryEventHandlerImpl(
+            QueryProcessor processor,
+            JdbcMetadataCatalog meta,
+            ClientResourceRegistry resources
+    ) {
         this.processor = processor;
         this.meta = meta;
         this.resources = resources;
     }
 
     /** {@inheritDoc} */
     @Override
-    public CompletableFuture<? extends Response> queryAsync(JdbcQueryExecuteRequest req) {
+    public CompletableFuture<JdbcConnectResult> connect() {
+        try {
+            JdbcConnectionContext connectionContext = new JdbcConnectionContext(
+                    processor::createSession,
+                    processor::closeSession
+            );
+
+            long connectionId = resources.put(new ClientResource(
+                    connectionContext,
+                    connectionContext::close
+            ));
+
+            return CompletableFuture.completedFuture(new JdbcConnectResult(connectionId));
+        } catch (IgniteInternalCheckedException exception) {
+            StringWriter sw = getWriterWithStackTrace(exception);
+
+            return CompletableFuture.completedFuture(new JdbcConnectResult(Response.STATUS_FAILED, "Unable to connect: " + sw));
+        }
+    }
+
+    /** {@inheritDoc} */
+    @Override
+    public CompletableFuture<? extends Response> queryAsync(long connectionId, JdbcQueryExecuteRequest req) {
         if (req.pageSize() <= 0) {
             return CompletableFuture.completedFuture(new JdbcQueryExecuteResult(Response.STATUS_FAILED,
                     "Invalid fetch size [fetchSize=" + req.pageSize() + ']'));
         }
 
-        QueryContext context = createQueryContext(req.getStmtType());
-
-        var results = new ArrayList<CompletableFuture<JdbcQuerySingleResult>>();
-        for (var cursorFut : processor.queryAsync(context, req.schemaName(), req.sqlQuery(),
-                req.arguments() == null ? OBJECT_EMPTY_ARRAY : req.arguments())) {
-            results.add(
-                    cursorFut.thenApply(cursor -> new JdbcQueryCursor<>(req.maxRows(), cursor))
-                            .thenCompose(cursor -> createJdbcResult(cursor, req))
-            );
+        // TODO: IGNITE-16960 support script execution
+        if (req.getStmtType() == JdbcStatementType.ANY_STATEMENT_TYPE) {
+            return CompletableFuture.completedFuture(new JdbcQueryExecuteResult(Response.STATUS_FAILED, "Scripts are not supported"));
         }
 
-        if (results.isEmpty()) {
+        JdbcConnectionContext connectionContext;
+        try {
+            connectionContext = resources.get(connectionId).get(JdbcConnectionContext.class);

Review Comment:
   `resources.get(connectionId)` may return null if connection was closed concurrently/
   Should we ignore request in that case?
   



-- 
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