korlov42 commented on code in PR #2906:
URL: https://github.com/apache/ignite-3/pull/2906#discussion_r1426542933
##########
modules/client-common/src/main/java/org/apache/ignite/internal/jdbc/proto/event/JdbcQuerySingleResult.java:
##########
@@ -69,15 +69,30 @@ public JdbcQuerySingleResult(int status, String err) {
super(status, err);
}
+ /**
+ * Constructor.
+ *
+ * @param hasNext {@code true} if more results are present.
+ * @param updCount Update counter.
+ */
+ public JdbcQuerySingleResult(boolean hasNext, long updCount) {
+ hasResults = hasNext;
+ this.updateCnt = updCount;
+ }
+
/**
* Constructor.
*
* @param cursorId Cursor ID.
* @param rowTuples Serialized SQL result rows.
+ * @param columnTypes Ordered list of types of columns in serialized rows.
+ * @param decimalScales Decimal scales in appearance order.
+ * @param isQuery {@code true} if query is SELECT/EXPLAIN.
+ * @param updateCnt Update count.
* @param last Flag indicates the query has no unfetched results.
*/
public JdbcQuerySingleResult(long cursorId, List<BinaryTupleReader>
rowTuples, List<ColumnType> columnTypes, int[] decimalScales,
- boolean last) {
+ boolean last, boolean isQuery, long updateCnt) {
Review Comment:
we already have contractor for DML statements, so let's use that one
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java:
##########
@@ -369,56 +380,75 @@ private CompletionStage<JdbcQuerySingleResult>
createJdbcResult(AsyncSqlCursor<I
return cur.requestNextAsync(req.pageSize()).thenApply(batch -> {
boolean hasNext = batch.hasMore();
+ long cursorId;
+ try {
+ cursorId = resources.put(new ClientResource(cur,
cur::closeAsync));
+ } catch (IgniteInternalCheckedException e) {
+ cur.closeAsync();
+
+ return new JdbcQuerySingleResult(Response.STATUS_FAILED,
+ "Unable to store query cursor.");
+ }
+
switch (cur.queryType()) {
case EXPLAIN:
case QUERY: {
- long cursorId;
- try {
- cursorId = resources.put(new ClientResource(cur,
cur::closeAsync));
- } catch (IgniteInternalCheckedException e) {
- cur.closeAsync();
-
- return new
JdbcQuerySingleResult(Response.STATUS_FAILED,
- "Unable to store query cursor.");
- }
-
- List<BinaryTupleReader> rows = new
ArrayList<>(batch.items().size());
- for (InternalSqlRow item : batch.items()) {
- rows.add(item.asBinaryTuple());
- }
-
List<ColumnMetadata> columns = cur.metadata().columns();
- int[] decimalScales = new int[columns.size()];
- List<ColumnType> schema = new ArrayList<>(columns.size());
-
- int countOfDecimal = 0;
- for (ColumnMetadata column : columns) {
- schema.add(column.type());
- if (column.type() == ColumnType.DECIMAL) {
- decimalScales[countOfDecimal++] = column.scale();
- }
- }
- decimalScales = Arrays.copyOf(decimalScales,
countOfDecimal);
-
- return new JdbcQuerySingleResult(cursorId, rows, schema,
decimalScales, !hasNext);
+ return buildSingleRequest(batch, columns, cursorId,
!hasNext, cur.queryType());
}
- case DML:
+ case DML: {
Review Comment:
if DML is handled by `buildSingleRequest` as well, then we should merge
these branches
##########
modules/client-handler/src/main/java/org/apache/ignite/client/handler/JdbcQueryEventHandlerImpl.java:
##########
@@ -369,56 +380,75 @@ private CompletionStage<JdbcQuerySingleResult>
createJdbcResult(AsyncSqlCursor<I
return cur.requestNextAsync(req.pageSize()).thenApply(batch -> {
boolean hasNext = batch.hasMore();
+ long cursorId;
+ try {
+ cursorId = resources.put(new ClientResource(cur,
cur::closeAsync));
+ } catch (IgniteInternalCheckedException e) {
+ cur.closeAsync();
+
+ return new JdbcQuerySingleResult(Response.STATUS_FAILED,
+ "Unable to store query cursor.");
+ }
+
switch (cur.queryType()) {
case EXPLAIN:
case QUERY: {
- long cursorId;
- try {
- cursorId = resources.put(new ClientResource(cur,
cur::closeAsync));
- } catch (IgniteInternalCheckedException e) {
- cur.closeAsync();
-
- return new
JdbcQuerySingleResult(Response.STATUS_FAILED,
- "Unable to store query cursor.");
- }
-
- List<BinaryTupleReader> rows = new
ArrayList<>(batch.items().size());
- for (InternalSqlRow item : batch.items()) {
- rows.add(item.asBinaryTuple());
- }
-
List<ColumnMetadata> columns = cur.metadata().columns();
- int[] decimalScales = new int[columns.size()];
- List<ColumnType> schema = new ArrayList<>(columns.size());
-
- int countOfDecimal = 0;
- for (ColumnMetadata column : columns) {
- schema.add(column.type());
- if (column.type() == ColumnType.DECIMAL) {
- decimalScales[countOfDecimal++] = column.scale();
- }
- }
- decimalScales = Arrays.copyOf(decimalScales,
countOfDecimal);
-
- return new JdbcQuerySingleResult(cursorId, rows, schema,
decimalScales, !hasNext);
+ return buildSingleRequest(batch, columns, cursorId,
!hasNext, cur.queryType());
}
- case DML:
+ case DML: {
if (!validateDmlResult(cur.metadata(), hasNext)) {
return new
JdbcQuerySingleResult(Response.STATUS_FAILED,
"Unexpected result for DML [query=" +
req.sqlQuery() + ']');
}
- return new JdbcQuerySingleResult((Long)
batch.items().get(0).get(0));
+ return new JdbcQuerySingleResult(cursorId, (long)
batch.items().get(0).get(0));
+ }
case DDL:
- return new JdbcQuerySingleResult(0);
+ case TX_CONTROL:
+ return new JdbcQuerySingleResult(cursorId, 0);
default:
return new JdbcQuerySingleResult(UNSUPPORTED_OPERATION,
"Query type is not supported yet [queryType=" +
cur.queryType() + ']');
}
});
}
+ static JdbcQuerySingleResult buildSingleRequest(
+ BatchedResult<InternalSqlRow> batch,
+ List<ColumnMetadata> columns,
+ long cursorId,
+ boolean hasNext,
+ SqlQueryType queryType
+ ) {
+ List<BinaryTupleReader> rows = new ArrayList<>(batch.items().size());
+ for (InternalSqlRow item : batch.items()) {
+ rows.add(item.asBinaryTuple());
Review Comment:
we don't need to do it for DML
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcResultSet.java:
##########
@@ -215,6 +218,37 @@ public JdbcResultSet(List<List<Object>> rows,
List<JdbcColumnMeta> meta) throws
initColumnOrder();
}
+ @Nullable JdbcResultSet getNextResultSet() throws SQLException {
+ try {
+ JdbcQueryFetchRequest req = new JdbcQueryFetchRequest(cursorId,
fetchSize);
+ JdbcQuerySingleResult res =
cursorHandler.getMoreResultsAsync(req).get();
+
+ close0(true);
Review Comment:
I see you've added
`org.apache.ignite.jdbc.ItJdbcMultiStatementSelfTest#testCloseOnCompletion`,
but I don't quite understand how it is suppose to relate to my comment...
##########
modules/client-common/src/main/java/org/apache/ignite/internal/jdbc/proto/JdbcQueryCursorHandler.java:
##########
@@ -37,6 +39,14 @@ public interface JdbcQueryCursorHandler {
*/
CompletableFuture<JdbcQueryFetchResult> fetchAsync(JdbcQueryFetchRequest
req);
+ /**
+ * {@link Statement#getMoreResults()} command implementor.
+ *
+ * @param req Results request.
+ * @return Result future.
+ */
+ CompletableFuture<JdbcQuerySingleResult>
getMoreResultsAsync(JdbcQueryFetchRequest req);
Review Comment:
I don't mind to rename it, but have no good options for name.
btw, what is wrong with new class?
##########
modules/jdbc/src/main/java/org/apache/ignite/internal/jdbc/JdbcResultSet.java:
##########
@@ -215,6 +218,37 @@ public JdbcResultSet(List<List<Object>> rows,
List<JdbcColumnMeta> meta) throws
initColumnOrder();
}
+ @Nullable JdbcResultSet getNextResultSet() throws SQLException {
+ try {
+ JdbcQueryFetchRequest req = new JdbcQueryFetchRequest(cursorId,
fetchSize);
+ JdbcQuerySingleResult res =
cursorHandler.getMoreResultsAsync(req).get();
+
+ close0(true);
+
+ if (!res.hasResults()) {
Review Comment:
probably I'm missing something, but I still no sight of error handling.
Could you point out to me on particular test that covers failed response?
--
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]
