korlov42 commented on code in PR #2811:
URL: https://github.com/apache/ignite-3/pull/2811#discussion_r1387848164


##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/api/IgniteSqlImplTest.java:
##########
@@ -111,9 +107,6 @@ void itsImpossibleToCreateSessionsAfterFacadeIsStopped() 
throws Exception {
     private static IgniteSqlImpl newSqlFacade() {
         QueryProcessor queryProcessor = mock(QueryProcessor.class);
 
-        when(queryProcessor.createSession(any()))
-                .thenAnswer(ignored -> new SessionId(UUID.randomUUID()));
-
         return new IgniteSqlImpl("test", queryProcessor, 
mock(IgniteTransactions.class));

Review Comment:
   although it works now if we pass `null` instead of mocks, such an approach 
doesn't feel safe. Neither QueryProcessor nor IgniteTransactions are marked as 
`@Nullable`. I don't want this test to fail if someone will decide to add 
additional assertions to ensure null-safety



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/QueryProcessor.java:
##########
@@ -20,61 +20,36 @@
 import java.util.List;
 import java.util.concurrent.CompletableFuture;
 import org.apache.ignite.internal.manager.IgniteComponent;
-import org.apache.ignite.internal.sql.engine.property.PropertiesHolder;
-import org.apache.ignite.internal.sql.engine.session.SessionId;
-import org.apache.ignite.internal.sql.engine.session.SessionInfo;
+import org.apache.ignite.internal.sql.engine.property.SqlProperties;
+import org.apache.ignite.internal.tx.InternalTransaction;
 import org.apache.ignite.lang.IgniteException;
 import org.apache.ignite.tx.IgniteTransactions;
+import org.jetbrains.annotations.Nullable;
 
 /**
  * QueryProcessor interface.
  */
 public interface QueryProcessor extends IgniteComponent {
-    /**
-     * Creates a session with given properties.
-     *
-     * @param properties Properties to store within a new session.
-     * @return An identifier of a created session.
-     */
-    SessionId createSession(PropertiesHolder properties);
-
-    /**
-     * Closes the session with given id.
-     *
-     * <p>This method just return a completed future in case the session was 
already closed or never exists.
-     *
-     * @param sessionId An identifier of a session to close.
-     * @return A future representing result of an operation.
-     */
-    CompletableFuture<Void> closeSession(SessionId sessionId);
-
-    /**
-     * Provide list of live sessions.
-     *
-     * <p>This method return the information is actual only on method 
invocation time.
-     *
-     * @return List of active sessions.
-     */
-    List<SessionInfo> liveSessions();
-
     /**
      * Execute the single statement query with given schema name and 
parameters.
      *
      * <p>If the query string contains more than one statement the 
IgniteException will be thrown.
      *
-     * @param sessionId A session identifier.
-     * @param context User query context.
+     * @param properties User query properties. See {@link QueryProperty} for 
available properties.
      * @param transactions Transactions facade.
+     * @param transaction A transaction to use for query execution. If null, 
an implicit transaction
+     *      will be started by provided transactional facade.

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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to