This is an automated email from the ASF dual-hosted git repository.
ptupitsyn pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 354352ed7a IGNITE-21011 Fix SQL script API in Java client (#2917)
354352ed7a is described below
commit 354352ed7af1efcb7ee0a868390a32bd64a27549
Author: Pavel Tupitsyn <[email protected]>
AuthorDate: Mon Dec 4 17:20:12 2023 +0200
IGNITE-21011 Fix SQL script API in Java client (#2917)
* Fix protocol - propagate session details correctly, skip `pageSize` for
scripts
* Fix `ItSqlClientAsynchronousApiTest` and `ItSqlClientSynchronousApiTest`
- remove overloaded methods, they were missing `@Test` annotation, and we don't
need an override anyway
* Add script and properties propagation tests to `ClientSqlTest`
---
.../internal/client/proto/ClientMessagePacker.java | 2 +-
.../requests/sql/ClientSqlExecuteRequest.java | 10 ++++--
.../sql/ClientSqlExecuteScriptRequest.java | 26 ++++++++++++--
.../ignite/internal/client/sql/ClientSession.java | 37 ++++++++++++--------
.../org/apache/ignite/client/ClientSqlTest.java | 39 +++++++++++++++++++++
.../ignite/client/fakes/FakeAsyncResultSet.java | 10 ++++--
.../apache/ignite/client/fakes/FakeIgniteSql.java | 4 ++-
.../apache/ignite/client/fakes/FakeSession.java | 40 ++++++++++++++++++++--
.../ignite/client/fakes/FakeSessionBuilder.java | 8 ++++-
.../sql/api/ItSqlClientAsynchronousApiTest.java | 10 ------
.../sql/api/ItSqlClientSynchronousApiTest.java | 12 -------
11 files changed, 149 insertions(+), 49 deletions(-)
diff --git
a/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientMessagePacker.java
b/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientMessagePacker.java
index e2ef846964..5d660fe66b 100644
---
a/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientMessagePacker.java
+++
b/modules/client-common/src/main/java/org/apache/ignite/internal/client/proto/ClientMessagePacker.java
@@ -262,7 +262,7 @@ public class ClientMessagePacker implements AutoCloseable {
*
* @param v the value to be written.
*/
- public void packLongNullable(Long v) {
+ public void packLongNullable(@Nullable Long v) {
if (v == null) {
packNil();
} else {
diff --git
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteRequest.java
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteRequest.java
index 702c5f6325..993c4e97f9 100644
---
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteRequest.java
+++
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteRequest.java
@@ -157,7 +157,7 @@ public class ClientSqlExecuteRequest {
return statementBuilder.build();
}
- static Session readSession(ClientMessageUnpacker in, IgniteSql sql,
@Nullable IgniteTransactions transactions) {
+ private static Session readSession(ClientMessageUnpacker in, IgniteSql
sql, @Nullable IgniteTransactions transactions) {
SessionBuilder sessionBuilder = sql.sessionBuilder();
if (transactions != null && sessionBuilder instanceof
SessionBuilderImpl) {
@@ -179,14 +179,18 @@ public class ClientSqlExecuteRequest {
sessionBuilder.idleTimeout(in.unpackLong(), TimeUnit.MILLISECONDS);
}
+ readSessionProperties(in, sessionBuilder);
+
+ return sessionBuilder.build();
+ }
+
+ static void readSessionProperties(ClientMessageUnpacker in, SessionBuilder
sessionBuilder) {
var propCount = in.unpackInt();
var reader = new BinaryTupleReader(propCount * 4,
in.readBinaryUnsafe());
for (int i = 0; i < propCount; i++) {
sessionBuilder.property(reader.stringValue(i * 4),
ClientBinaryTupleUtils.readObject(reader, i * 4 + 1));
}
-
- return sessionBuilder.build();
}
private static void packMeta(ClientMessagePacker out, @Nullable
ResultSetMetadata meta) {
diff --git
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteScriptRequest.java
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteScriptRequest.java
index 0bdfe3dcbe..7b6a84dba0 100644
---
a/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteScriptRequest.java
+++
b/modules/client-handler/src/main/java/org/apache/ignite/client/handler/requests/sql/ClientSqlExecuteScriptRequest.java
@@ -17,15 +17,17 @@
package org.apache.ignite.client.handler.requests.sql;
-import static
org.apache.ignite.client.handler.requests.sql.ClientSqlExecuteRequest.readSession;
+import static
org.apache.ignite.client.handler.requests.sql.ClientSqlExecuteRequest.readSessionProperties;
import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.TimeUnit;
import org.apache.ignite.internal.client.proto.ClientMessageUnpacker;
import org.apache.ignite.internal.hlc.HybridTimestamp;
import org.apache.ignite.internal.tx.impl.IgniteTransactionsImpl;
import org.apache.ignite.internal.util.ArrayUtils;
import org.apache.ignite.sql.IgniteSql;
import org.apache.ignite.sql.Session;
+import org.apache.ignite.sql.Session.SessionBuilder;
/**
* Client SQL execute script request.
@@ -43,7 +45,7 @@ public class ClientSqlExecuteScriptRequest {
IgniteSql sql,
IgniteTransactionsImpl transactions
) {
- Session session = readSession(in, sql, null);
+ Session session = readSession(in, sql);
String script = in.unpackString();
Object[] arguments = in.unpackObjectArrayFromBinaryTuple();
@@ -59,4 +61,24 @@ public class ClientSqlExecuteScriptRequest {
return session.executeScriptAsync(script, arguments);
}
+
+ private static Session readSession(ClientMessageUnpacker in, IgniteSql
sql) {
+ SessionBuilder sessionBuilder = sql.sessionBuilder();
+
+ if (!in.tryUnpackNil()) {
+ sessionBuilder.defaultSchema(in.unpackString());
+ }
+
+ if (!in.tryUnpackNil()) {
+ sessionBuilder.defaultQueryTimeout(in.unpackLong(),
TimeUnit.MILLISECONDS);
+ }
+
+ if (!in.tryUnpackNil()) {
+ sessionBuilder.idleTimeout(in.unpackLong(), TimeUnit.MILLISECONDS);
+ }
+
+ readSessionProperties(in, sessionBuilder);
+
+ return sessionBuilder.build();
+ }
}
diff --git
a/modules/client/src/main/java/org/apache/ignite/internal/client/sql/ClientSession.java
b/modules/client/src/main/java/org/apache/ignite/internal/client/sql/ClientSession.java
index 448f2d7b09..1a6a10e3c4 100644
---
a/modules/client/src/main/java/org/apache/ignite/internal/client/sql/ClientSession.java
+++
b/modules/client/src/main/java/org/apache/ignite/internal/client/sql/ClientSession.java
@@ -154,7 +154,7 @@ public class ClientSession implements AbstractSession {
w.out().packLongNullable(defaultSessionTimeout);
- packProperties(w, clientStatement.properties());
+ packProperties(w, properties, clientStatement.properties());
w.out().packString(clientStatement.query());
@@ -228,6 +228,12 @@ public class ClientSession implements AbstractSession {
Objects.requireNonNull(query);
PayloadWriter payloadWriter = w -> {
+ w.out().packString(defaultSchema);
+ w.out().packLongNullable(defaultQueryTimeout);
+ w.out().packLongNullable(defaultSessionTimeout);
+
+ packProperties(w, properties, null);
+
w.out().packString(query);
w.out().packObjectArrayAsBinaryTuple(arguments);
w.out().packLong(ch.observableTimestamp());
@@ -303,39 +309,42 @@ public class ClientSession implements AbstractSession {
throw new UnsupportedOperationException("Not implemented yet.");
}
- private void packProperties(PayloadOutputChannel w, Map<String, Object>
props) {
+ private static void packProperties(
+ PayloadOutputChannel w,
+ @Nullable Map<String, Object> sessionProps,
+ @Nullable Map<String, Object> statementProps) {
int size = 0;
- if (props != null) {
- size += props.size();
+ if (statementProps != null) {
+ size += statementProps.size();
}
// Statement properties override session properties.
- if (properties != null) {
- if (props != null) {
- for (String k : properties.keySet()) {
- if (!props.containsKey(k)) {
+ if (sessionProps != null) {
+ if (statementProps != null) {
+ for (String k : sessionProps.keySet()) {
+ if (!statementProps.containsKey(k)) {
size++;
}
}
} else {
- size += properties.size();
+ size += sessionProps.size();
}
}
w.out().packInt(size);
var builder = new BinaryTupleBuilder(size * 4);
- if (props != null) {
- for (Entry<String, Object> entry : props.entrySet()) {
+ if (statementProps != null) {
+ for (Entry<String, Object> entry : statementProps.entrySet()) {
builder.appendString(entry.getKey());
ClientBinaryTupleUtils.appendObject(builder, entry.getValue());
}
}
- if (properties != null) {
- for (Entry<String, Object> entry : properties.entrySet()) {
- if (props == null || !props.containsKey(entry.getKey())) {
+ if (sessionProps != null) {
+ for (Entry<String, Object> entry : sessionProps.entrySet()) {
+ if (statementProps == null ||
!statementProps.containsKey(entry.getKey())) {
builder.appendString(entry.getKey());
ClientBinaryTupleUtils.appendObject(builder,
entry.getValue());
}
diff --git
a/modules/client/src/test/java/org/apache/ignite/client/ClientSqlTest.java
b/modules/client/src/test/java/org/apache/ignite/client/ClientSqlTest.java
index 3fe72fa7c1..ca567169f0 100644
--- a/modules/client/src/test/java/org/apache/ignite/client/ClientSqlTest.java
+++ b/modules/client/src/test/java/org/apache/ignite/client/ClientSqlTest.java
@@ -52,6 +52,7 @@ import org.junit.jupiter.api.Test;
/**
* SQL tests.
*/
+@SuppressWarnings("resource")
public class ClientSqlTest extends AbstractClientTableTest {
@Test
public void testExecuteAsync() {
@@ -215,4 +216,42 @@ public class ClientSqlTest extends AbstractClientTableTest
{
assertEquals(BigInteger.valueOf(42), row.value(17));
assertEquals(ColumnType.NUMBER, meta.columns().get(17).type());
}
+
+ @Test
+ public void testExecuteScript() {
+ Session session = client.sql().createSession();
+
+ session.executeScript("foo");
+
+ ResultSet<SqlRow> resultSet = session.execute(null, "SELECT LAST
SCRIPT");
+ SqlRow row = resultSet.next();
+
+ assertEquals(
+ "foo, arguments: [], properties: [], defaultPageSize=null,
defaultSchema=null, "
+ + "defaultQueryTimeout=null,
defaultSessionTimeout=null",
+ row.value(0));
+ }
+
+ @Test
+ public void testExecuteScriptWithPropertiesAndArguments() {
+ Session session = client.sql().sessionBuilder()
+ .property("prop1", "val1")
+ .property("prop2", -5)
+ .property("prop3", null)
+ .defaultPageSize(123) // Should be ignored - not applicable to
scripts.
+ .defaultQueryTimeout(456, TimeUnit.MILLISECONDS)
+ .defaultSchema("script-schema")
+ .idleTimeout(789, TimeUnit.SECONDS)
+ .build();
+
+ session.executeScript("do bar baz", "arg1", null, 2);
+
+ ResultSet<SqlRow> resultSet = session.execute(null, "SELECT LAST
SCRIPT");
+ SqlRow row = resultSet.next();
+
+ assertEquals(
+ "do bar baz, arguments: [arg1, null, 2, ], properties:
[prop2=-5, prop1=val1, prop3=null, ], "
+ + "defaultPageSize=null, defaultSchema=script-schema,
defaultQueryTimeout=456, defaultSessionTimeout=789000",
+ row.value(0));
+ }
}
diff --git
a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeAsyncResultSet.java
b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeAsyncResultSet.java
index 5e96bbb538..acb7d6d719 100644
---
a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeAsyncResultSet.java
+++
b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeAsyncResultSet.java
@@ -62,6 +62,8 @@ public class FakeAsyncResultSet implements AsyncResultSet {
private final boolean hasMorePages;
+ private final FakeIgniteSql sql;
+
/**
* Constructor.
*
@@ -70,7 +72,7 @@ public class FakeAsyncResultSet implements AsyncResultSet {
* @param statement Statement.
* @param arguments Arguments.
*/
- public FakeAsyncResultSet(Session session, Transaction transaction,
Statement statement, Object[] arguments) {
+ public FakeAsyncResultSet(Session session, Transaction transaction,
Statement statement, Object[] arguments, FakeIgniteSql sql) {
assert session != null;
assert statement != null;
@@ -78,6 +80,7 @@ public class FakeAsyncResultSet implements AsyncResultSet {
this.transaction = transaction;
this.statement = statement;
this.arguments = arguments;
+ this.sql = sql;
hasMorePages = session.property("hasMorePages") != null;
@@ -142,6 +145,9 @@ public class FakeAsyncResultSet implements AsyncResultSet {
BigInteger.valueOf(42));
rows = List.of(row);
+ } else if ("SELECT LAST SCRIPT".equals(statement.query())) {
+ rows = List.of(getRow(sql.lastScript));
+ columns = List.of(new FakeColumnMetadata("script",
ColumnType.STRING));
} else {
rows = List.of(getRow(1));
columns = List.of(new FakeColumnMetadata("col1",
ColumnType.INT32));
@@ -197,7 +203,7 @@ public class FakeAsyncResultSet implements AsyncResultSet {
/** {@inheritDoc} */
@Override
public CompletableFuture<? extends AsyncResultSet> fetchNextPage() {
- return CompletableFuture.completedFuture(new
FakeAsyncResultSet(session, transaction, statement, arguments));
+ return CompletableFuture.completedFuture(new
FakeAsyncResultSet(session, transaction, statement, arguments, sql));
}
/** {@inheritDoc} */
diff --git
a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeIgniteSql.java
b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeIgniteSql.java
index 2c3d69ed5b..7095c58fd5 100644
---
a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeIgniteSql.java
+++
b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeIgniteSql.java
@@ -28,6 +28,8 @@ import org.apache.ignite.sql.Statement.StatementBuilder;
* Fake SQL implementation.
*/
public class FakeIgniteSql implements IgniteSql {
+ String lastScript;
+
@Override
public Session createSession() {
return sessionBuilder().build();
@@ -35,7 +37,7 @@ public class FakeIgniteSql implements IgniteSql {
@Override
public SessionBuilder sessionBuilder() {
- return new FakeSessionBuilder();
+ return new FakeSessionBuilder(this);
}
@Override
diff --git
a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeSession.java
b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeSession.java
index 43f677b34d..9a0e79eed1 100644
---
a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeSession.java
+++
b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeSession.java
@@ -58,6 +58,8 @@ public class FakeSession implements AbstractSession {
@Nullable
private final Map<String, Object> properties;
+ private final FakeIgniteSql sql;
+
/**
* Constructor.
*
@@ -65,6 +67,7 @@ public class FakeSession implements AbstractSession {
* @param defaultSchema Default schema.
* @param defaultQueryTimeout Default timeout.
* @param properties Properties.
+ * @param sql SQL.
*/
@SuppressWarnings("AssignmentOrReturnOfFieldWithMutableType")
public FakeSession(
@@ -72,12 +75,14 @@ public class FakeSession implements AbstractSession {
@Nullable String defaultSchema,
@Nullable Long defaultQueryTimeout,
@Nullable Long defaultSessionTimeout,
- @Nullable Map<String, Object> properties) {
+ @Nullable Map<String, Object> properties,
+ FakeIgniteSql sql) {
this.defaultPageSize = defaultPageSize;
this.defaultSchema = defaultSchema;
this.defaultQueryTimeout = defaultQueryTimeout;
this.defaultSessionTimeout = defaultSessionTimeout;
this.properties = properties;
+ this.sql = sql;
}
/** {@inheritDoc} */
@@ -101,7 +106,7 @@ public class FakeSession implements AbstractSession {
return CompletableFuture.failedFuture(new
SqlException(STMT_VALIDATION_ERR, "Query failed"));
}
- return CompletableFuture.completedFuture(new FakeAsyncResultSet(this,
transaction, statement, arguments));
+ return CompletableFuture.completedFuture(new FakeAsyncResultSet(this,
transaction, statement, arguments, sql));
}
/** {@inheritDoc} */
@@ -178,7 +183,36 @@ public class FakeSession implements AbstractSession {
/** {@inheritDoc} */
@Override
public CompletableFuture<Void> executeScriptAsync(String query, @Nullable
Object... arguments) {
- throw new UnsupportedOperationException();
+ var sb = new StringBuilder(query);
+
+ if (arguments != null) {
+ sb.append(", arguments: [");
+
+ for (Object arg : arguments) {
+ sb.append(arg).append(", ");
+ }
+
+ sb.append(']');
+ }
+
+ if (properties != null) {
+ sb.append(", properties: [");
+
+ for (Map.Entry<String, Object> entry : properties.entrySet()) {
+
sb.append(entry.getKey()).append('=').append(entry.getValue()).append(", ");
+ }
+
+ sb.append(']');
+ }
+
+ sb.append(", ").append("defaultPageSize=").append(defaultPageSize);
+ sb.append(", ").append("defaultSchema=").append(defaultSchema);
+ sb.append(",
").append("defaultQueryTimeout=").append(defaultQueryTimeout);
+ sb.append(",
").append("defaultSessionTimeout=").append(defaultSessionTimeout);
+
+ sql.lastScript = sb.toString();
+
+ return nullCompletedFuture();
}
/** {@inheritDoc} */
diff --git
a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeSessionBuilder.java
b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeSessionBuilder.java
index 9c71662fb1..3d8876167a 100644
---
a/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeSessionBuilder.java
+++
b/modules/client/src/test/java/org/apache/ignite/client/fakes/FakeSessionBuilder.java
@@ -31,6 +31,8 @@ import org.jetbrains.annotations.Nullable;
public class FakeSessionBuilder implements SessionBuilder {
private final Map<String, Object> properties = new HashMap<>();
+ private final FakeIgniteSql sql;
+
private String defaultSchema;
private Long defaultQueryTimeoutMs;
@@ -39,6 +41,10 @@ public class FakeSessionBuilder implements SessionBuilder {
private Integer pageSize;
+ public FakeSessionBuilder(FakeIgniteSql sql) {
+ this.sql = sql;
+ }
+
/** {@inheritDoc} */
@Override
public long defaultQueryTimeout(TimeUnit timeUnit) {
@@ -118,6 +124,6 @@ public class FakeSessionBuilder implements SessionBuilder {
/** {@inheritDoc} */
@Override
public Session build() {
- return new FakeSession(pageSize, defaultSchema, defaultQueryTimeoutMs,
defaultSessionTimeoutMs, new HashMap<>(properties));
+ return new FakeSession(pageSize, defaultSchema, defaultQueryTimeoutMs,
defaultSessionTimeoutMs, new HashMap<>(properties), sql);
}
}
diff --git
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlClientAsynchronousApiTest.java
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlClientAsynchronousApiTest.java
index 200029fc80..2b2e174fd3 100644
---
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlClientAsynchronousApiTest.java
+++
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlClientAsynchronousApiTest.java
@@ -68,14 +68,4 @@ public class ItSqlClientAsynchronousApiTest extends
ItSqlAsynchronousApiTest {
public void testLockIsNotReleasedAfterTxRollback() {
super.testLockIsNotReleasedAfterTxRollback();
}
-
- @Override
- public void runScriptThatCompletesSuccessfully() {
- super.runScriptThatCompletesSuccessfully();
- }
-
- @Override
- public void runScriptThatFails() {
- super.runScriptThatFails();
- }
}
diff --git
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlClientSynchronousApiTest.java
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlClientSynchronousApiTest.java
index f69a054fc2..fa48bff50f 100644
---
a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlClientSynchronousApiTest.java
+++
b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlClientSynchronousApiTest.java
@@ -31,8 +31,6 @@ import org.junit.jupiter.api.Disabled;
public class ItSqlClientSynchronousApiTest extends ItSqlSynchronousApiTest {
private IgniteClient client;
- private static final int ROW_COUNT = 16;
-
@BeforeAll
public void startClient() {
client =
IgniteClient.builder().addresses(getClientAddresses(List.of(CLUSTER.aliveNode())).get(0)).build();
@@ -70,14 +68,4 @@ public class ItSqlClientSynchronousApiTest extends
ItSqlSynchronousApiTest {
public void testLockIsNotReleasedAfterTxRollback() {
super.testLockIsNotReleasedAfterTxRollback();
}
-
- @Override
- public void runScriptThatCompletesSuccessfully() {
- super.runScriptThatCompletesSuccessfully();
- }
-
- @Override
- public void runScriptThatFails() {
- super.runScriptThatFails();
- }
}
