Skip to content

Commit 301d3d8

Browse files
ennuitelidavidm
andauthored
GH-1063: Add is_update field to ActionCreatePreparedStatementResult (#1064)
## What's Changed A new field, `optional bool is_update = 4;`, was added to `message ActionCreatePreparedStatementResult`. When this field is sent by the server, its value indicates whether the proper network flow to execute the query that the driver should follow uses `CommandPreparedStatementQuery` or `CommandPreparedStatementUpdate`. For outdated servers that don't send the field, the driver maintains its current behavior of using `CommandPreparedStatementQuery` when the `dataset_schema` is not empty, thus ensuring the backward compatibility of the new driver with old servers. This change was created with AI assistance (Augment Code and Claude code). All lines were manually reviewed by a human. The output is not copyrightable subject matter. - Closes #1063 --------- Co-authored-by: David Li <li.davidm96@gmail.com>
1 parent b0e51af commit 301d3d8

9 files changed

Lines changed: 212 additions & 8 deletions

File tree

arrow-format/FlightSql.proto

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1550,6 +1550,11 @@ message ActionCreatePreparedStatementResult {
15501550
// If the query provided contained parameters, parameter_schema contains the
15511551
// schema of the expected parameters. It should be an IPC-encapsulated Schema, as described in Schema.fbs.
15521552
bytes parameter_schema = 3;
1553+
1554+
// When set to true, the query should be executed with CommandPreparedStatementUpdate,
1555+
// when set to false, the query should be executed with CommandPreparedStatementQuery.
1556+
// If not set, the client can choose how to execute the query.
1557+
optional bool is_update = 4;
15531558
}
15541559

15551560
/*

flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcFlightStreamResultSet.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ static ArrowFlightJdbcFlightStreamResultSet fromFlightInfo(
106106
final TimeZone timeZone = TimeZone.getDefault();
107107
final QueryState state = new QueryState();
108108

109-
final Meta.Signature signature = ArrowFlightMetaImpl.newSignature(null, null, null);
109+
final Meta.Signature signature = ArrowFlightMetaImpl.newSignature(null, null, null, null);
110110

111111
final AvaticaResultSetMetaData resultSetMetaData =
112112
new AvaticaResultSetMetaData(null, null, signature);

flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightJdbcVectorSchemaRootResultSet.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ public static ArrowFlightJdbcVectorSchemaRootResultSet fromVectorSchemaRoot(
7373
final TimeZone timeZone = TimeZone.getDefault();
7474
final QueryState state = new QueryState();
7575

76-
final Meta.Signature signature = ArrowFlightMetaImpl.newSignature(null, null, null);
76+
final Meta.Signature signature = ArrowFlightMetaImpl.newSignature(null, null, null, null);
7777

7878
final AvaticaResultSetMetaData resultSetMetaData =
7979
new AvaticaResultSetMetaData(null, null, signature);

flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightMetaImpl.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,8 @@ public ArrowFlightMetaImpl(final AvaticaConnection connection) {
5353
}
5454

5555
/** Construct a signature. */
56-
static Signature newSignature(final String sql, Schema resultSetSchema, Schema parameterSchema) {
56+
static Signature newSignature(
57+
final String sql, Schema resultSetSchema, Schema parameterSchema, Boolean isUpdate) {
5758
List<ColumnMetaData> columnMetaData =
5859
resultSetSchema == null
5960
? new ArrayList<>()
@@ -62,10 +63,17 @@ static Signature newSignature(final String sql, Schema resultSetSchema, Schema p
6263
parameterSchema == null
6364
? new ArrayList<>()
6465
: ConvertUtils.convertArrowFieldsToAvaticaParameters(parameterSchema.getFields());
65-
StatementType statementType =
66-
resultSetSchema == null || resultSetSchema.getFields().isEmpty()
67-
? StatementType.IS_DML
68-
: StatementType.SELECT;
66+
// If the server provided the is_update field, use it to determine the statement type
67+
StatementType statementType;
68+
if (isUpdate != null) {
69+
statementType = isUpdate ? StatementType.IS_DML : StatementType.SELECT;
70+
} else {
71+
// Fall back to the legacy logic: check if the result set schema is empty
72+
statementType =
73+
resultSetSchema == null || resultSetSchema.getFields().isEmpty()
74+
? StatementType.IS_DML
75+
: StatementType.SELECT;
76+
}
6977
return new Signature(
7078
columnMetaData,
7179
sql,
@@ -178,7 +186,10 @@ private PreparedStatement prepareForHandle(final String query, StatementHandle h
178186
((ArrowFlightConnection) connection).getClientHandler().prepare(query);
179187
handle.signature =
180188
newSignature(
181-
query, preparedStatement.getDataSetSchema(), preparedStatement.getParameterSchema());
189+
query,
190+
preparedStatement.getDataSetSchema(),
191+
preparedStatement.getParameterSchema(),
192+
preparedStatement.isUpdate());
182193
statementHandlePreparedStatementMap.put(new StatementHandleKey(handle), preparedStatement);
183194
return preparedStatement;
184195
}

flight/flight-sql-jdbc-core/src/main/java/org/apache/arrow/driver/jdbc/client/ArrowFlightSqlClientHandler.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,14 @@ public interface PreparedStatement extends AutoCloseable {
388388
*/
389389
Schema getParameterSchema();
390390

391+
/**
392+
* Gets whether this {@link PreparedStatement} is an update statement.
393+
*
394+
* @return {@code true} if this is an update statement, {@code false} if it's a query, or {@code
395+
* null} if the server did not provide this information.
396+
*/
397+
@Nullable Boolean isUpdate();
398+
391399
void setParameters(VectorSchemaRoot parameters);
392400

393401
@Override
@@ -456,6 +464,12 @@ public long executeUpdate() {
456464

457465
@Override
458466
public StatementType getType() {
467+
// If the server provided the is_update field, use it to determine the statement type
468+
final Boolean isUpdate = preparedStatement.isUpdate();
469+
if (isUpdate != null) {
470+
return isUpdate ? StatementType.UPDATE : StatementType.SELECT;
471+
}
472+
// Fall back to the legacy logic: check if the result set schema is empty
459473
final Schema schema = preparedStatement.getResultSetSchema();
460474
return schema.getFields().isEmpty() ? StatementType.UPDATE : StatementType.SELECT;
461475
}
@@ -475,6 +489,11 @@ public void setParameters(VectorSchemaRoot parameters) {
475489
preparedStatement.setParameters(parameters);
476490
}
477491

492+
@Override
493+
public Boolean isUpdate() {
494+
return preparedStatement.isUpdate();
495+
}
496+
478497
@Override
479498
public void close() {
480499
try {

flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightPreparedStatementTest.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,40 @@ public void testSimpleQueryNoParameterBindingWithExecute() throws SQLException {
9898
}
9999
}
100100

101+
@Test
102+
public void testSimpleQueryNoParameterBindingWithExecuteV2() throws SQLException {
103+
final String query = "SELECT * FROM TEST_V2";
104+
final Schema schema =
105+
new Schema(Collections.singletonList(Field.nullable("", Types.MinorType.INT.getType())));
106+
PRODUCER.addSelectQuery(
107+
query,
108+
schema,
109+
Collections.singletonList(
110+
listener -> {
111+
try (final BufferAllocator allocator = new RootAllocator(Long.MAX_VALUE);
112+
final VectorSchemaRoot root = VectorSchemaRoot.create(schema, allocator)) {
113+
root.allocateNew();
114+
((IntVector) root.getVector(0)).setSafe(0, 123);
115+
root.setRowCount(1);
116+
listener.start(root);
117+
listener.putNext();
118+
} finally {
119+
listener.completed();
120+
}
121+
}),
122+
false);
123+
try (final PreparedStatement preparedStatement = connection.prepareStatement(query)) {
124+
boolean isResultSet = preparedStatement.execute();
125+
assertTrue(isResultSet);
126+
final ResultSet resultSet = preparedStatement.getResultSet();
127+
assertTrue(resultSet.next());
128+
assertEquals(123, resultSet.getInt(1));
129+
assertFalse(resultSet.next());
130+
assertFalse(preparedStatement.getMoreResults());
131+
assertEquals(-1, preparedStatement.getUpdateCount());
132+
}
133+
}
134+
101135
@Test
102136
public void testQueryWithParameterBinding() throws SQLException {
103137
final String query = "Fake query with parameters";
@@ -203,6 +237,20 @@ public void testUpdateQueryWithExecute() throws SQLException {
203237
}
204238
}
205239

240+
@Test
241+
public void testUpdateQueryWithExecuteV2() throws SQLException {
242+
String query = "Fake update with execute V2";
243+
PRODUCER.addUpdateQuery(query, /*updatedRows*/ 99, true);
244+
try (final PreparedStatement stmt = connection.prepareStatement(query)) {
245+
boolean isResultSet = stmt.execute();
246+
assertFalse(isResultSet);
247+
int updated = stmt.getUpdateCount();
248+
assertEquals(99, updated);
249+
assertFalse(stmt.getMoreResults());
250+
assertEquals(-1, stmt.getUpdateCount());
251+
}
252+
}
253+
206254
@Test
207255
public void testUpdateQueryWithParameters() throws SQLException {
208256
String query = "Fake update with parameters";

flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/ArrowFlightStatementExecuteTest.java

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ public class ArrowFlightStatementExecuteTest {
6262
private static final String SAMPLE_LARGE_UPDATE_QUERY =
6363
"UPDATE this_large_table SET this_large_field = that_large_field FROM this_large_test WHERE this_large_condition";
6464
private static final long SAMPLE_LARGE_UPDATE_COUNT = Long.MAX_VALUE;
65+
private static final String SAMPLE_QUERY_CMD_V2 = "SELECT * FROM this_test_v2";
66+
private static final String SAMPLE_LARGE_UPDATE_QUERY_V2 =
67+
"UPDATE this_large_table_v2 SET this_large_field = that_large_field FROM this_large_test WHERE this_large_condition";
6568
private static final MockFlightSqlProducer PRODUCER = new MockFlightSqlProducer();
6669

6770
@RegisterExtension
@@ -96,6 +99,31 @@ public static void setUpBeforeClass() {
9699
}));
97100
PRODUCER.addUpdateQuery(SAMPLE_UPDATE_QUERY, SAMPLE_UPDATE_COUNT);
98101
PRODUCER.addUpdateQuery(SAMPLE_LARGE_UPDATE_QUERY, SAMPLE_LARGE_UPDATE_COUNT);
102+
103+
// V2 queries with is_update field set
104+
PRODUCER.addSelectQuery(
105+
SAMPLE_QUERY_CMD_V2,
106+
SAMPLE_QUERY_SCHEMA,
107+
Collections.singletonList(
108+
listener -> {
109+
try (final BufferAllocator allocator = new RootAllocator(Long.MAX_VALUE);
110+
final VectorSchemaRoot root =
111+
VectorSchemaRoot.create(SAMPLE_QUERY_SCHEMA, allocator)) {
112+
final UInt1Vector vector = (UInt1Vector) root.getVector(VECTOR_NAME);
113+
IntStream.range(0, SAMPLE_QUERY_ROWS)
114+
.forEach(index -> vector.setSafe(index, index));
115+
vector.setValueCount(SAMPLE_QUERY_ROWS);
116+
root.setRowCount(SAMPLE_QUERY_ROWS);
117+
listener.start(root);
118+
listener.putNext();
119+
} catch (final Throwable throwable) {
120+
listener.error(throwable);
121+
} finally {
122+
listener.completed();
123+
}
124+
}),
125+
false);
126+
PRODUCER.addUpdateQuery(SAMPLE_LARGE_UPDATE_QUERY_V2, SAMPLE_LARGE_UPDATE_COUNT, true);
99127
}
100128

101129
@BeforeEach
@@ -168,4 +196,42 @@ public void testUpdateCountShouldStartOnZero() throws SQLException {
168196
is(allOf(equalTo(statement.getLargeUpdateCount()), equalTo(0L))));
169197
assertThat(statement.getResultSet(), is(nullValue()));
170198
}
199+
200+
@Test
201+
public void testExecuteShouldRunSelectQueryV2() throws SQLException {
202+
assertThat(statement.execute(SAMPLE_QUERY_CMD_V2), is(true));
203+
final Set<Byte> numbers =
204+
IntStream.range(0, SAMPLE_QUERY_ROWS)
205+
.boxed()
206+
.map(Integer::byteValue)
207+
.collect(Collectors.toCollection(HashSet::new));
208+
try (final ResultSet resultSet = statement.getResultSet()) {
209+
final int columnCount = resultSet.getMetaData().getColumnCount();
210+
assertThat(columnCount, is(1));
211+
int rowCount = 0;
212+
for (; resultSet.next(); rowCount++) {
213+
assertThat(numbers.remove(resultSet.getByte(1)), is(true));
214+
}
215+
assertThat(rowCount, is(equalTo(SAMPLE_QUERY_ROWS)));
216+
}
217+
assertThat(numbers, is(Collections.emptySet()));
218+
assertThat(
219+
(long) statement.getUpdateCount(),
220+
is(allOf(equalTo(statement.getLargeUpdateCount()), equalTo(-1L))));
221+
}
222+
223+
@Test
224+
public void testExecuteShouldRunUpdateQueryForLargeUpdateV2() throws SQLException {
225+
assertThat(statement.execute(SAMPLE_LARGE_UPDATE_QUERY_V2), is(false)); // UPDATE query.
226+
final long updateCountSmall = statement.getUpdateCount();
227+
final long updateCountLarge = statement.getLargeUpdateCount();
228+
assertThat(updateCountLarge, is(equalTo(SAMPLE_LARGE_UPDATE_COUNT)));
229+
assertThat(
230+
updateCountSmall,
231+
is(
232+
allOf(
233+
equalTo((long) AvaticaUtils.toSaturatedInt(updateCountLarge)),
234+
not(equalTo(updateCountLarge)))));
235+
assertThat(statement.getResultSet(), is(nullValue()));
236+
}
171237
}

flight/flight-sql-jdbc-core/src/test/java/org/apache/arrow/driver/jdbc/utils/MockFlightSqlProducer.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@
8787
import org.apache.arrow.vector.types.pojo.Schema;
8888
import org.apache.arrow.vector.util.JsonStringArrayList;
8989
import org.apache.calcite.avatica.Meta.StatementType;
90+
import org.checkerframework.checker.nullness.qual.Nullable;
9091

9192
/** An ad-hoc {@link FlightSqlProducer} for tests. */
9293
public final class MockFlightSqlProducer implements FlightSqlProducer {
@@ -101,6 +102,7 @@ public final class MockFlightSqlProducer implements FlightSqlProducer {
101102
private final SqlInfoBuilder sqlInfoBuilder = new SqlInfoBuilder();
102103
private final Map<String, Schema> parameterSchemas = new HashMap<>();
103104
private final Map<String, List<List<Object>>> expectedParameterValues = new HashMap<>();
105+
private final Map<String, Boolean> isUpdateMap = new HashMap<>();
104106

105107
private final Map<String, Integer> actionTypeCounter = new HashMap<>();
106108

@@ -176,6 +178,40 @@ public void addUpdateQuery(final String sqlCommand, final long updatedRows) {
176178
});
177179
}
178180

181+
/**
182+
* Registers a new {@link StatementType#SELECT} SQL query, optionally setting the is_update field.
183+
*
184+
* @param sqlCommand the SQL command under which to register the new query.
185+
* @param schema the schema to use for the query result.
186+
* @param resultProviders the result provider for this query.
187+
* @param isUpdate value to report for the is_update field, or {@code null} to leave it unset.
188+
*/
189+
public void addSelectQuery(
190+
final String sqlCommand,
191+
final Schema schema,
192+
final List<Consumer<ServerStreamListener>> resultProviders,
193+
final @Nullable Boolean isUpdate) {
194+
addSelectQuery(sqlCommand, schema, resultProviders);
195+
if (isUpdate != null) {
196+
isUpdateMap.put(sqlCommand, isUpdate);
197+
}
198+
}
199+
200+
/**
201+
* Registers a new {@link StatementType#UPDATE} SQL query, optionally setting the is_update field.
202+
*
203+
* @param sqlCommand the SQL command.
204+
* @param updatedRows the number of rows affected.
205+
* @param isUpdate value to report for the is_update field, or {@code null} to leave it unset.
206+
*/
207+
public void addUpdateQuery(
208+
final String sqlCommand, final long updatedRows, final @Nullable Boolean isUpdate) {
209+
addUpdateQuery(sqlCommand, updatedRows);
210+
if (isUpdate != null) {
211+
isUpdateMap.put(sqlCommand, isUpdate);
212+
}
213+
}
214+
179215
/**
180216
* Adds a catalog query to the results.
181217
*
@@ -247,6 +283,12 @@ public void createPreparedStatement(
247283
resultBuilder.setParameterSchema(ByteString.copyFrom(outputStream.toByteArray()));
248284
}
249285

286+
// Set is_update field if present
287+
final Boolean isUpdate = isUpdateMap.get(query);
288+
if (isUpdate != null) {
289+
resultBuilder.setIsUpdate(isUpdate);
290+
}
291+
250292
listener.onNext(new Result(pack(resultBuilder.build()).toByteArray()));
251293
} catch (final Throwable t) {
252294
listener.onError(t);

flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/FlightSqlClient.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1284,6 +1284,19 @@ public Schema getParameterSchema() {
12841284
return parameterSchema;
12851285
}
12861286

1287+
/**
1288+
* Returns whether the server indicated this prepared statement is an update query.
1289+
*
1290+
* @return true if the server indicated this is an update query, false if the server indicated
1291+
* this is a select query, or null if the server did not provide this information.
1292+
*/
1293+
public Boolean isUpdate() {
1294+
if (preparedStatementResult.hasIsUpdate()) {
1295+
return preparedStatementResult.getIsUpdate();
1296+
}
1297+
return null;
1298+
}
1299+
12871300
/** Get the schema of the result set (should be identical to {@link #getResultSetSchema()}). */
12881301
public SchemaResult fetchSchema(CallOption... options) {
12891302
checkOpen();

0 commit comments

Comments
 (0)