Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -382,4 +382,39 @@ public void testPrepareAndExecuteWithStringParameter() {
fail("testPrepareAndExecuteWithStringParameter failed: " + e.getMessage());
}
}

@Test
public void testPrepareAndExecuteWithReorderedProjection() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case and testPrepareAndExecuteWithRepeatedProjection below do not exercise the code changed by this PR. SQL-level PREPARE / EXECUTE goes through IoTDBStatement.executeQuery() and the full IoTDBJDBCResultSet constructor, which already forwarded columnIndex2TsBlockColumnIndexList before this PR. The client-side IoTDBTablePreparedStatement (the ?-parameter path through executePreparedStatement and processQueryResult) is never involved.

Verified with the master client on a live 1C1D server, using the same two projections on an equivalent table (filtering on an INT32 column instead of id): PREPARE ... FROM SELECT value, name ... followed by EXECUTE ... USING already returns 200.3, Bob, and the name AS x, name AS y variant already returns Bob, Bob. So both new cases pass with and without this fix.

They are still useful as coverage for the server-side prepared-statement path, but please do not count them as regression tests for this fix in the PR description. The regression coverage for the changed code is the new unit tests plus IoTDBTablePreparedStatementJDBCIT, whose reordered / repeated / TIMESTAMP queries I confirmed fail against the master client.

String[] expectedHeader = new String[] {"value", "name"};
String[] retArray = new String[] {"200.3,Bob,"};
try (Connection connection = EnvFactory.getEnv().getTableConnection();
Statement statement = connection.createStatement()) {
statement.execute("USE " + DATABASE_NAME);
// Projection order differs from the table's column order
statement.execute("PREPARE stmt9 FROM SELECT value, name FROM test_table WHERE id = ?");
executePreparedStatementAndVerify(
connection, statement, "EXECUTE stmt9 USING 2", expectedHeader, retArray);
statement.execute("DEALLOCATE PREPARE stmt9");
} catch (SQLException e) {
fail("testPrepareAndExecuteWithReorderedProjection failed: " + e.getMessage());
}
}

@Test
public void testPrepareAndExecuteWithRepeatedProjection() {
String[] expectedHeader = new String[] {"x", "y"};
String[] retArray = new String[] {"Bob,Bob,"};
try (Connection connection = EnvFactory.getEnv().getTableConnection();
Statement statement = connection.createStatement()) {
statement.execute("USE " + DATABASE_NAME);
// The same physical column is projected twice under different aliases
statement.execute(
"PREPARE stmt10 FROM SELECT name AS x, name AS y FROM test_table WHERE id = ?");
executePreparedStatementAndVerify(
connection, statement, "EXECUTE stmt10 USING 2", expectedHeader, retArray);
statement.execute("DEALLOCATE PREPARE stmt10");
} catch (SQLException e) {
fail("testPrepareAndExecuteWithRepeatedProjection failed: " + e.getMessage());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -495,4 +495,109 @@ public void testPreparedStatementWithAllDataTypes() throws SQLException {
}
}
}

@Test
public void testPreparedStatementWithTimestampProjection() throws SQLException {
try (Connection connection = getConnection();
Statement stmt = connection.createStatement()) {
stmt.execute("USE " + DATABASE_NAME);
// Self-contained fixture: the all-types table is created by another test method
stmt.execute(
"CREATE TABLE IF NOT EXISTS timestamp_projection_table("
+ "int32_col INT32 FIELD, timestamp_col TIMESTAMP FIELD)");
stmt.execute(
"INSERT INTO timestamp_projection_table VALUES "
+ "(2025-06-01T00:00:00, 123, 2025-06-15T10:30:00)");

// TIMESTAMP is a valid table-model field type and must keep its mapping as well
try (PreparedStatement ps =
connection.prepareStatement(
"SELECT timestamp_col, int32_col FROM timestamp_projection_table "
+ "WHERE int32_col = ?")) {
ps.setInt(1, 123);
try (ResultSet rs = ps.executeQuery()) {
assertTrue(rs.next());
assertEquals(
Timestamp.valueOf("2025-06-15 10:30:00").getTime(), rs.getTimestamp(1).getTime());
assertEquals(123, rs.getInt(2));
assertEquals(
Timestamp.valueOf("2025-06-15 10:30:00").getTime(),
rs.getTimestamp("timestamp_col").getTime());
assertEquals(123, rs.getInt("int32_col"));
assertFalse(rs.next());
}
}
}
}

@Test
public void testPreparedStatementWithReorderedProjection() throws SQLException {
try (Connection connection = getConnection();
Statement stmt = connection.createStatement()) {
stmt.execute("USE " + DATABASE_NAME);

// Projection order ("value", "name") differs from the materialized order
try (PreparedStatement ps =
connection.prepareStatement("SELECT value, name FROM test_table WHERE int_value = ?")) {
ps.setInt(1, 20);
try (ResultSet rs = ps.executeQuery()) {
assertTrue(rs.next());
ResultSetMetaData metaData = rs.getMetaData();
assertEquals(2, metaData.getColumnCount());
assertEquals("value", metaData.getColumnName(1));
assertEquals("name", metaData.getColumnName(2));
assertEquals(200.3, rs.getDouble(1), 0.001);
assertEquals("Bob", rs.getString(2));
assertEquals("Bob", rs.getString("name"));
assertEquals(200.3, rs.getDouble("value"), 0.001);
assertFalse(rs.next());
}
}
}
}

@Test
public void testPreparedStatementWithRepeatedProjection() throws SQLException {
try (Connection connection = getConnection();
Statement stmt = connection.createStatement()) {
stmt.execute("USE " + DATABASE_NAME);

// The same physical column is projected twice under different aliases
try (PreparedStatement ps =
connection.prepareStatement(
"SELECT name AS x, name AS y FROM test_table WHERE int_value = ?")) {
ps.setInt(1, 20);
try (ResultSet rs = ps.executeQuery()) {
assertTrue(rs.next());
assertEquals("Bob", rs.getString(1));
assertEquals("Bob", rs.getString(2));
assertEquals("Bob", rs.getString("x"));
assertEquals("Bob", rs.getString("y"));
assertFalse(rs.next());
}
}
}
}

@Test
public void testPreparedStatementWithIdentityProjection() throws SQLException {
try (Connection connection = getConnection();
Statement stmt = connection.createStatement()) {
stmt.execute("USE " + DATABASE_NAME);

// Identity mapping: a regression control for the two cases above
try (PreparedStatement ps =
connection.prepareStatement("SELECT name, value FROM test_table WHERE int_value = ?")) {
ps.setInt(1, 20);
try (ResultSet rs = ps.executeQuery()) {
assertTrue(rs.next());
assertEquals("Bob", rs.getString(1));
assertEquals(200.3, rs.getDouble(2), 0.001);
assertEquals("Bob", rs.getString("name"));
assertEquals(200.3, rs.getDouble("value"), 0.001);
assertFalse(rs.next());
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,12 @@ private ResultSet processQueryResult(TSExecuteStatementResp resp) throws SQLExce
resp.tracingInfo,
(long) queryTimeout * 1000,
resp.isSetMoreData() && resp.isMoreData(),
zoneId);
zoneId,
charset,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forwarding charset is a genuine behaviour fix, but it has no test. Measured on a live server with a connection opened with charset=GBK, reading a row whose value is 中文:

  • master client, getBytes(1) on a PreparedStatement: [-28, -72, -83, -26, -106, -121] (UTF-8, wrong)
  • master client, same value through a plain Statement: [-42, -48, -50, -60] (GBK, correct)
  • PR client, both paths: [-42, -48, -50, -60]

Please add a mock-based unit test for this (a STRING/TEXT column plus a non-default charset passed to the six-argument constructor, asserting getBytes), otherwise a future refactor can drop the argument again without any test failing.

resp.isSetTableModel() && resp.isTableModel(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two scope notes on this call:

  1. tableModel is inert. IoTDBRpcDataSet takes the parameter but never reads it (declared at IoTDBRpcDataSet.java:112, no other use), so the previous hard-coded false had no observable effect. Forwarding it is fine for consistency with IoTDBStatement, but the PR description should not present it as one of the fixed bugs.
  2. The defensive copy is harmless but not required. The in-place columnIndex2TsBlockColumnIndexList.add(0, -1) in IoTDBRpcDataSet runs only when ignoreTimeStamp == false, which does not happen for the table model, and IoTDBStatement passes the Thrift list through the same constructor without copying. Keeping the copy is fine; please soften the "required" wording in the description, since readers may otherwise take it as evidence of a mutation path that does not exist here.

resp.getColumnIndex2TsBlockColumnIndexList() == null
? null
: new ArrayList<>(resp.getColumnIndex2TsBlockColumnIndexList()));
return resultSet;
}
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,17 @@
import org.apache.iotdb.rpc.stmt.PreparedParameterSerde;
import org.apache.iotdb.service.rpc.thrift.IClientRPCService.Iface;
import org.apache.iotdb.service.rpc.thrift.TSCancelOperationReq;
import org.apache.iotdb.service.rpc.thrift.TSCloseOperationReq;
import org.apache.iotdb.service.rpc.thrift.TSExecutePreparedReq;
import org.apache.iotdb.service.rpc.thrift.TSExecuteStatementReq;
import org.apache.iotdb.service.rpc.thrift.TSExecuteStatementResp;
import org.apache.iotdb.service.rpc.thrift.TSPrepareReq;
import org.apache.iotdb.service.rpc.thrift.TSPrepareResp;

import org.apache.tsfile.enums.TSDataType;
import org.apache.tsfile.read.common.block.TsBlockBuilder;
import org.apache.tsfile.read.common.block.column.TsBlockSerde;
import org.apache.tsfile.utils.Binary;
import org.junit.Before;
import org.junit.Test;
import org.mockito.ArgumentCaptor;
Expand All @@ -39,17 +44,23 @@
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;

import java.io.IOException;
import java.nio.ByteBuffer;
import java.nio.charset.Charset;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Timestamp;
import java.sql.Types;
import java.time.Instant;
import java.time.LocalDate;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
Expand All @@ -75,6 +86,7 @@ public class IoTDBTablePreparedStatementTest {
public void before() throws Exception {
MockitoAnnotations.initMocks(this);
when(connection.getSqlDialect()).thenReturn("table");
when(connection.getTimeFactor()).thenReturn(1_000);
when(execStatementResp.getStatus()).thenReturn(Status_SUCCESS);
when(execStatementResp.getQueryId()).thenReturn(queryId);

Expand All @@ -98,6 +110,8 @@ public TSPrepareResp answer(InvocationOnMock invocation) throws Throwable {
// Mock for executePreparedStatement
when(client.executePreparedStatement(any(TSExecutePreparedReq.class)))
.thenReturn(execStatementResp);

when(client.closeOperation(any(TSCloseOperationReq.class))).thenReturn(Status_SUCCESS);
}

/** Count the number of '?' placeholders in a SQL string, ignoring those inside quotes */
Expand Down Expand Up @@ -255,6 +269,174 @@ public void testTableModelStringWithNull() throws Exception {
assertTrue(argument.getValue().getParameters() != null);
}

// ========== Table Model result column mapping tests ==========

private static TsBlockBuilder int32BlockBuilder(int columnCount) {
TsBlockBuilder builder = new TsBlockBuilder(Collections.nCopies(columnCount, TSDataType.INT32));
builder.getTimeColumnBuilder().writeLong(1000L);
builder.declarePosition();
return builder;
}

private TSExecuteStatementResp buildQueryResp(
List<String> columns, List<Integer> mapping, TsBlockBuilder blockBuilder) throws IOException {
return buildQueryResp(
columns, Collections.nCopies(columns.size(), TSDataType.INT32), mapping, blockBuilder);
}

private TSExecuteStatementResp buildQueryResp(
List<String> columns,
List<TSDataType> dataTypes,
List<Integer> mapping,
TsBlockBuilder blockBuilder)
throws IOException {
TSExecuteStatementResp resp = new TSExecuteStatementResp();
resp.setStatus(Status_SUCCESS);
resp.setQueryId(1L);
resp.setColumns(columns);
List<String> dataTypeList = new ArrayList<>();
for (TSDataType dataType : dataTypes) {
dataTypeList.add(dataType.name());
}
resp.setDataTypeList(dataTypeList);
resp.setIgnoreTimeStamp(true);
resp.setTableModel(true);
if (mapping != null) {
resp.setColumnIndex2TsBlockColumnIndexList(mapping);
}
resp.setQueryResult(
Collections.singletonList(new TsBlockSerde().serialize(blockBuilder.build())));
resp.setMoreData(false);
return resp;
}

@SuppressWarnings("resource")
@Test
public void testTableModelPreparedStatementReorderedColumns() throws Exception {
TsBlockBuilder blockBuilder = int32BlockBuilder(2);
blockBuilder.getColumnBuilder(0).writeInt(10); // physical column 0
blockBuilder.getColumnBuilder(1).writeInt(20); // physical column 1

when(client.executePreparedStatement(any(TSExecutePreparedReq.class)))
.thenReturn(buildQueryResp(Arrays.asList("b", "a"), Arrays.asList(1, 0), blockBuilder));

IoTDBTablePreparedStatement ps =
new IoTDBTablePreparedStatement(
connection, client, sessionId, "SELECT b, a FROM t WHERE a > ?", zoneId);
ps.setInt(1, 0);
try (ResultSet rs = ps.executeQuery()) {
assertTrue(rs.next());
assertEquals(20, rs.getInt(1));
assertEquals(10, rs.getInt(2));
assertEquals(20, rs.getInt("b"));
assertEquals(10, rs.getInt("a"));
}
}

@SuppressWarnings("resource")
@Test
public void testTableModelPreparedStatementRepeatedProjection() throws Exception {
TsBlockBuilder blockBuilder = int32BlockBuilder(1);
blockBuilder.getColumnBuilder(0).writeInt(10); // physical column 0, referenced twice

when(client.executePreparedStatement(any(TSExecutePreparedReq.class)))
.thenReturn(buildQueryResp(Arrays.asList("x", "y"), Arrays.asList(0, 0), blockBuilder));

IoTDBTablePreparedStatement ps =
new IoTDBTablePreparedStatement(
connection, client, sessionId, "SELECT a AS x, a AS y FROM t WHERE a > ?", zoneId);
ps.setInt(1, 0);
try (ResultSet rs = ps.executeQuery()) {
assertTrue(rs.next());
assertEquals(10, rs.getInt(1));
assertEquals(10, rs.getInt(2));
assertEquals(10, rs.getInt("x"));
assertEquals(10, rs.getInt("y"));
}
}

@SuppressWarnings("resource")
@Test
public void testTableModelPreparedStatementIdentityMapping() throws Exception {
TsBlockBuilder blockBuilder = int32BlockBuilder(2);
blockBuilder.getColumnBuilder(0).writeInt(10);
blockBuilder.getColumnBuilder(1).writeInt(20);

when(client.executePreparedStatement(any(TSExecutePreparedReq.class)))
.thenReturn(buildQueryResp(Arrays.asList("a", "b"), Arrays.asList(0, 1), blockBuilder));

IoTDBTablePreparedStatement ps =
new IoTDBTablePreparedStatement(
connection, client, sessionId, "SELECT a, b FROM t WHERE a > ?", zoneId);
ps.setInt(1, 0);
try (ResultSet rs = ps.executeQuery()) {
assertTrue(rs.next());
assertEquals(10, rs.getInt(1));
assertEquals(20, rs.getInt(2));
assertEquals(10, rs.getInt("a"));
assertEquals(20, rs.getInt("b"));
}
}

@SuppressWarnings("resource")
@Test
public void testTableModelPreparedStatementMissingMappingFallsBack() throws Exception {
TsBlockBuilder blockBuilder = int32BlockBuilder(2);
blockBuilder.getColumnBuilder(0).writeInt(10);
blockBuilder.getColumnBuilder(1).writeInt(20);

// No columnIndex2TsBlockColumnIndexList: the client must keep its own derivation
when(client.executePreparedStatement(any(TSExecutePreparedReq.class)))
.thenReturn(buildQueryResp(Arrays.asList("a", "b"), null, blockBuilder));

IoTDBTablePreparedStatement ps =
new IoTDBTablePreparedStatement(
connection, client, sessionId, "SELECT a, b FROM t WHERE a > ?", zoneId);
ps.setInt(1, 0);
try (ResultSet rs = ps.executeQuery()) {
assertTrue(rs.next());
assertEquals(10, rs.getInt(1));
assertEquals(20, rs.getInt(2));
assertEquals(10, rs.getInt("a"));
assertEquals(20, rs.getInt("b"));
}
}

// ========== Charset forwarding ==========

@SuppressWarnings("resource")
@Test
public void testTableModelPreparedStatementGetBytesUsesConnectionCharset() throws Exception {
// A STRING column holding 中文; the value travels as UTF-8 in the TsBlock, but getBytes must
// encode the decoded String with the charset the connection was opened with.
TsBlockBuilder blockBuilder = new TsBlockBuilder(Collections.singletonList(TSDataType.STRING));
blockBuilder.getTimeColumnBuilder().writeLong(1000L);
blockBuilder.declarePosition();
blockBuilder.getColumnBuilder(0).writeBinary(new Binary("中文", Charset.forName("UTF-8")));
when(client.executePreparedStatement(any(TSExecutePreparedReq.class)))
.thenReturn(
buildQueryResp(
Collections.singletonList("name"),
Collections.singletonList(TSDataType.STRING),
null,
blockBuilder));

IoTDBTablePreparedStatement ps =
new IoTDBTablePreparedStatement(
connection,
client,
sessionId,
"SELECT name FROM t WHERE name = ?",
zoneId,
Charset.forName("GBK"));
ps.setString(1, "中文");
try (ResultSet rs = ps.executeQuery()) {
assertTrue(rs.next());
assertArrayEquals("中文".getBytes(Charset.forName("GBK")), rs.getBytes(1));
assertArrayEquals("中文".getBytes(Charset.forName("GBK")), rs.getBytes("name"));
}
}

// ========== Cancel, fetch size and timestamp precision ==========

@SuppressWarnings("resource")
Expand Down