xtern commented on code in PR #2811:
URL: https://github.com/apache/ignite-3/pull/2811#discussion_r1387613454
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/api/SessionImpl.java:
##########
@@ -447,8 +500,27 @@ private void touchAndCloseIfExpired() {
}
}
+ private int registerCursor(AsyncSqlCursor<?> cursor) {
+ int cursorId = cursorIdGen.incrementAndGet();
+
+ Object old = openedCursors.put(cursorId, cursor);
+
+ assert old == null;
+
+ return cursorId;
+ }
+
+ private static SqlException sessionIsClosedException() {
+ return new SqlException(SESSION_CLOSED_ERR, "Session is closed.");
+ }
+
@FunctionalInterface
interface SessionBuilderFactory {
SessionBuilder fromProperties(Map<String, Object> properties);
}
+
+ @TestOnly
+ List<AsyncSqlCursor<?>> openedCursors() {
Review Comment:
t seems that it is enough for this method to return int :sunglasses:
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/property/PropertyNotFoundException.java:
##########
@@ -20,10 +20,9 @@
import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
/**
- * Exception that is thrown by {@link PropertiesHolder} if given property
- * is not fount in the holder.
+ * Exception that is thrown by {@link SqlProperties} if given property is not
fount.
Review Comment:
```suggestion
* Exception that is thrown by {@link SqlProperties} if given property is
not found.
```
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/property/SqlPropertiesHelper.java:
##########
@@ -0,0 +1,155 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.sql.engine.property;
+
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
+import org.apache.ignite.internal.sql.engine.property.SqlProperties.Builder;
+import org.apache.ignite.internal.util.CollectionUtils;
+import org.apache.ignite.internal.util.FilteringIterator;
+import org.apache.ignite.internal.util.IgniteUtils;
+
+/**
+ * Utility class to work with {@link Property}.
+ */
+public final class SqlPropertiesHelper {
+ private SqlPropertiesHelper() {
+ throw new IllegalStateException();
+ }
+
+ /** Creates empty properties object. */
+ public static SqlProperties emptyProperties() {
+ return new SqlPropertiesImpl(Map.of());
+ }
+
+ /** Creates new builder. */
+ public static Builder newBuilder() {
+ return new BuilderImpl();
+ }
+
+ /**
+ * Merges two properties objects into single one.
+ *
+ * <p>If conflict arises, conflicted values of secondary object will be
overridden with values
+ * of primary object.
+ *
+ * @param primary A properties whose values will be preserved.
+ * @param secondary A properties whose values will be overridden in case
of conflict.
+ * @return A properties object containing properties from both objects.
+ */
+ public static SqlProperties merge(SqlProperties primary, SqlProperties
secondary) {
+ Builder builder = builderFromProperties(secondary);
+
+ for (Map.Entry<Property<?>, Object> entry : primary) {
+ builder.set((Property<Object>) entry.getKey(), entry.getValue());
+ }
+
+ return builder.build();
+ }
+
+ /**
+ * Creates a chained properties object. That is, on every access to the
property it will look up
+ * certain property in the primary object, and then, if property was not
found, in secondary.
+ *
+ * @param primary A primary object.
+ * @param secondary A secondary object.
+ * @return A chained properties object.
+ */
+ public static SqlProperties chain(SqlProperties primary, SqlProperties
secondary) {
Review Comment:
I didn't find a test for this method :sunglasses:
##########
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:
```suggestion
* will be started by provided transactions facade.
```
:thinking: :thinking:
##########
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:
This test now runs successfully without mocks :sunglasses:
--
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]
