From 2cd0e404e7b93048b5110899925b4ab8d3c57497 Mon Sep 17 00:00:00 2001 From: "Benedicto Franco Jr." Date: Fri, 6 Nov 2026 19:43:04 -0800 Subject: [PATCH] pxf: a timestamp ends at ')', as protowire-go's endsRun has it (#169) --- CHANGELOG.md | 1 + src/pxf/lexer.cc | 4 +++- test/pxf_dataset_reader_test.cc | 25 +++++++++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 328709d..a9ae9a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ number says nothing about any other port. See the Releases section of - **`DatasetReader` caps the bytes it holds while looking for a row's end at `max_message_size`** (#164), as protowire-go does: `dataset input holds N bytes without a row boundary, MaxMessageSize=M`. A row that never closed was buffered whole, whatever the limit. - **`DatasetReader::Scan` binds a row under the default `max_message_size`** (#164), as protowire-go does. The body a row is bound as repeats the column names, so it is wider than the row, and a row whose bytes fit the caller's limit could still be refused when bound. The row's bytes are still capped as they are read. The other limits apply as before. - **A broken `@dataset` header is refused with the error `Parse` gives** (#164), as protowire-go does. `Create` said `no @dataset directive in stream` for a header cut short, and gave its own text for an unterminated string in it. The reader now finds the header's and each row's end with the lexer `Parse` uses rather than a byte scanner of its own. Streaming is about 15% faster on a 300,000-row table. +- **A timestamp cell right before a row's `)` parses** (#169), as protowire-go does. `(2024-01-02T03:04:05Z)` was refused with `invalid timestamp: 2024-01-02T03:04:05Z)`, because the lexer read the `)` into the timestamp. A space before the `)`, or a later cell, hid it. ## [2.0.0] — 2026-11-05 diff --git a/src/pxf/lexer.cc b/src/pxf/lexer.cc index 830aff3..9129714 100644 --- a/src/pxf/lexer.cc +++ b/src/pxf/lexer.cc @@ -681,8 +681,10 @@ Token Lexer::LexFloat(Position pos, size_t start) { Token Lexer::LexTimestamp(Position pos, size_t start) { while (pos_ < input_.size()) { uint8_t c = Peek(); + // The bytes that end a run, as protowire-go's endsRun has them: `)` ends + // the last cell of a @dataset row (#169). if (c == ' ' || c == '\n' || c == '\t' || c == '\r' || c == ',' || c == ']' || c == '}' || - c == '#') { + c == ')' || c == '#') { break; } if (c == '/' && (Peek(1) == '/' || Peek(1) == '*')) break; diff --git a/test/pxf_dataset_reader_test.cc b/test/pxf_dataset_reader_test.cc index 795d9bc..d926df6 100644 --- a/test/pxf_dataset_reader_test.cc +++ b/test/pxf_dataset_reader_test.cc @@ -554,4 +554,29 @@ TEST(DatasetReaderEnds, MatchProtowireGo) { EXPECT_EQ(errors, 230); } +// A timestamp ends at a row's `)`, as protowire-go's lexer ends it (#169): +// the `)` was read into the timestamp, which then failed to parse. +TEST(DatasetTimestampCell, EndsAtTheRowsParen) { + const std::string doc = "@dataset x.Row ( a )\n(2024-01-02T03:04:05Z)\n"; + auto parsed = protowire::pxf::Parse(doc); + ASSERT_TRUE(parsed.ok()) << parsed.status().ToString(); + ASSERT_EQ(parsed->datasets.size(), 1u); + ASSERT_EQ(parsed->datasets[0].rows.size(), 1u); + const auto& cell = parsed->datasets[0].rows[0].cells[0]; + ASSERT_TRUE(cell.has_value()); + const auto* ts = std::get_if>(&*cell); + ASSERT_NE(ts, nullptr); + EXPECT_EQ((*ts)->raw, "2024-01-02T03:04:05Z"); + + std::istringstream src(doc); + auto tr = DatasetReader::Create(&src); + ASSERT_TRUE(tr.ok()) << tr.status().ToString(); + DatasetRow row; + ASSERT_TRUE((*tr)->Next(&row).ok()); + ASSERT_FALSE((*tr)->Done()); + ASSERT_EQ(row.cells.size(), 1u); + ASSERT_TRUE(row.cells[0].has_value()); + EXPECT_NE(std::get_if>(&*row.cells[0]), nullptr); +} + } // namespace