Skip to content

Commit 2c9924d

Browse files
jerrinotclaude
andcommitted
Reject timestamp name set while rows are buffered
QwpTableBuffer.setDesignatedTimestampName guarded only the rename branch on rowCount, so a first set was accepted with rows already buffered. On the UDP sender, TableOptionsImpl.designatedTimestamp then bumped committedDatagramEstimate by the trailer size without flushing or re-checking the cap. Since only commitCurrentRow checks maxDatagramSize, a subsequent flush() shipped a datagram exceeding the configured cap: IP fragmentation, or EMSGSIZE and silent data loss near the 65507-byte limit. setDesignatedTimestampName now rejects a first set while the buffer holds rows, symmetric with the rename guard. This makes the estimate bump in TableOptionsImpl unreachable, so this commit deletes it and keeps only the base-estimate invalidation. Javadocs on QwpTableBuffer and Sender.TableOptions now state the name may be set or re-declared only while the table has no buffered rows. Regression tests cover the buffer-level throw and the sender-level scenario: the late set throws, the flushed datagram carries no table-options trailer, and all packets stay within maxDatagramSize. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 7a81add commit 2c9924d

5 files changed

Lines changed: 62 additions & 14 deletions

File tree

core/src/main/java/io/questdb/client/Sender.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -870,9 +870,9 @@ interface TableOptions {
870870
* silently ignore it and create the conventional {@code timestamp}
871871
* column instead.
872872
* <p>
873-
* The hint is sticky per table. It may be re-declared with a different
874-
* name while the table has no buffered rows; changing it while rows
875-
* are buffered throws.
873+
* The hint is sticky per table. It may be set or re-declared only
874+
* while the table has no buffered rows; setting it while rows are
875+
* buffered throws.
876876
*
877877
* @param columnName non-empty column name of at most 127 UTF-8 bytes
878878
* @return this instance for method chaining

core/src/main/java/io/questdb/client/cutlass/qwp/client/QwpUdpSender.java

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1548,20 +1548,14 @@ public TableOptions designatedTimestamp(CharSequence columnName) {
15481548
"table options reference is stale; call tableOptions() after table()"
15491549
);
15501550
}
1551-
String previousName = tableBuffer.getDesignatedTimestampName();
15521551
tableBuffer.setDesignatedTimestampName(columnName);
15531552
// The cached base estimate folds in the trailer size, which depends on
1554-
// the name length, so every accepted set/change must invalidate it. The
1555-
// committedDatagramEstimate bump stays under previousName == null: a name
1556-
// change requires rowCount == 0, where the committed estimate is already 0.
1553+
// the name length, so every accepted set/change must invalidate it. Any
1554+
// accepted set requires rowCount == 0, where the committed estimate is
1555+
// already 0, so no committedDatagramEstimate adjustment is needed.
15571556
if (currentTableHeadroomState != null) {
15581557
currentTableHeadroomState.invalidateBaseEstimate();
15591558
}
1560-
if (previousName == null && trackDatagramEstimate && committedDatagramEstimate > 0) {
1561-
committedDatagramEstimate += QwpColumnWriter.getSingleTableOptionsTrailerSize(
1562-
tableBuffer.getDesignatedTimestampName()
1563-
);
1564-
}
15651559
return this;
15661560
}
15671561

core/src/main/java/io/questdb/client/cutlass/qwp/protocol/QwpTableBuffer.java

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -366,8 +366,8 @@ public void rollbackUncommittedColumns() {
366366
/**
367367
* Sets the create-only designated timestamp column name hint for this
368368
* table. The value is sticky across rows and buffer resets. It may be
369-
* re-declared while the buffer holds no rows; changing it while rows are
370-
* buffered throws.
369+
* set or re-declared only while the buffer holds no rows; any set while
370+
* rows are buffered throws.
371371
*
372372
* @param columnName non-empty name of at most 127 UTF-8 bytes
373373
*/
@@ -383,6 +383,12 @@ public void setDesignatedTimestampName(CharSequence columnName) {
383383
);
384384
}
385385
if (designatedTimestampName == null) {
386+
if (rowCount != 0) {
387+
throw new LineSenderException(
388+
"cannot set designated timestamp name for table '" + tableName
389+
+ "' after rows are buffered [new=" + columnName + ']'
390+
);
391+
}
386392
designatedTimestampName = Chars.toString(columnName);
387393
} else if (!Chars.equals(designatedTimestampName, columnName)) {
388394
if (rowCount != 0) {

core/src/test/java/io/questdb/client/test/cutlass/qwp/client/QwpUdpSenderTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1729,6 +1729,33 @@ public void testTableOptionsDesignatedTimestampEmitsTrailer() throws Exception {
17291729
});
17301730
}
17311731

1732+
@Test
1733+
public void testTableOptionsDesignatedTimestampRejectedAfterRowsBuffered() throws Exception {
1734+
assertMemoryLeak(() -> {
1735+
CapturingNetworkFacade nf = new CapturingNetworkFacade();
1736+
int maxDatagramSize = 1024;
1737+
try (QwpUdpSender sender = new QwpUdpSender(nf, 0, 0, 9000, 1, maxDatagramSize)) {
1738+
sender.table("t");
1739+
sender.longColumn("x", 1).atNow();
1740+
1741+
// a late hint would grow the committed datagram past the cap
1742+
// with no re-check on the flush path, so it must be rejected
1743+
assertThrowsContains(
1744+
"after rows are buffered",
1745+
() -> sender.tableOptions().designatedTimestamp("event_ts")
1746+
);
1747+
1748+
sender.flush();
1749+
Assert.assertEquals(1, nf.packets.size());
1750+
Assert.assertEquals(0, (byte) (nf.packets.get(0)[HEADER_OFFSET_FLAGS] & FLAG_TABLE_OPTIONS));
1751+
assertPacketsWithinLimit(new RunResult(nf.packets, nf.lengths, nf.sendCount), maxDatagramSize);
1752+
1753+
// rowCount == 0 after the flush: the hint is accepted again
1754+
sender.tableOptions().designatedTimestamp("event_ts");
1755+
}
1756+
});
1757+
}
1758+
17321759
@Test
17331760
public void testTableOptionsOnlyRowIsDiscarded() throws Exception {
17341761
assertMemoryLeak(() -> {

core/src/test/java/io/questdb/client/test/cutlass/qwp/protocol/QwpTableBufferTest.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,6 +1371,27 @@ public void testRetainInProgressRowFastClearsUnstagedNullableColumn() throws Exc
13711371
});
13721372
}
13731373

1374+
@Test
1375+
public void testDesignatedTimestampNameFirstSetAfterRowsBufferedThrows() throws Exception {
1376+
assertMemoryLeak(() -> {
1377+
try (QwpTableBuffer table = new QwpTableBuffer("test")) {
1378+
table.getOrCreateColumn("x", QwpConstants.TYPE_LONG, false).addLong(1);
1379+
table.nextRow();
1380+
try {
1381+
table.setDesignatedTimestampName("event_time");
1382+
fail("expected rejection of first set after rows are buffered");
1383+
} catch (LineSenderException e) {
1384+
assertTrue(e.getMessage().contains("after rows are buffered"));
1385+
}
1386+
assertNull(table.getDesignatedTimestampName());
1387+
1388+
table.reset();
1389+
table.setDesignatedTimestampName("event_time");
1390+
assertEquals("event_time", table.getDesignatedTimestampName());
1391+
}
1392+
});
1393+
}
1394+
13741395
@Test
13751396
public void testDesignatedTimestampNameIsStickyAndConsistent() throws Exception {
13761397
assertMemoryLeak(() -> {

0 commit comments

Comments
 (0)