Skip to content
Merged
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
15 changes: 8 additions & 7 deletions data/include/data/issues.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#include <string>

namespace fastlanes {
using issues_dataset_t = std::array<std::pair<std::string_view, std::string_view>, 3>;
using issues_dataset_t = std::array<std::pair<std::string_view, std::string_view>, 4>;

class issues {
public:
Expand All @@ -19,12 +19,13 @@ class issues {
static constexpr std::string_view issues_cwida_alp_37_diff_data {FASTLANES_DATA_DIR
"/issues/cwida/alp/37/diff_data"};
static constexpr std::string_view ISSUE_000 {FLS_CMAKE_SOURCE_DIR "/data/issues/issue_000/"};
//
static constexpr issues_dataset_t dataset = {{
{"issues_cwida_alp_37_kv_cache_original", issues_cwida_alp_37_kv_cache_original},
{"issues_cwida_alp_37_diff_data", issues_cwida_alp_37_diff_data},
{"ISSUE_000", ISSUE_000},
}};
static constexpr std::string_view ISSUE_055 {FLS_CMAKE_SOURCE_DIR "/data/issues/000055/"};

static constexpr issues_dataset_t dataset = {
{{"issues_cwida_alp_37_kv_cache_original", issues_cwida_alp_37_kv_cache_original},
{"issues_cwida_alp_37_diff_data", issues_cwida_alp_37_diff_data},
{"ISSUE_000", ISSUE_000},
{ISSUE_055, ISSUE_055}}};
};
} // namespace fastlanes

Expand Down
2 changes: 2 additions & 0 deletions data/issues/000055/data.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
001|2014-09-24 18:00:00|12|2|24|17|0|
001|2014-08-24 18:00:00|3|0|21|19|0|0
44 changes: 44 additions & 0 deletions data/issues/000055/schema.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
{
"columns": [
{
"name": "Column_1",
"type": "BIGINT",
"index": "0"
},
{
"name": "Column_2",
"type": "TIMESTAMP",
"index": "1"
},
{
"name": "Column_3",
"type": "BIGINT",
"index": "2"
},
{
"name": "Column_4",
"type": "BIGINT",
"index": "3"
},
{
"name": "Column_5",
"type": "BIGINT",
"index": "4"
},
{
"name": "Column_6",
"type": "BIGINT",
"index": "5"
},
{
"name": "Column_7",
"type": "DOUBLE",
"index": "6"
},
{
"name": "Column_7",
"type": "BIGINT",
"index": "6"
}
]
}
24 changes: 13 additions & 11 deletions src/include/fls/csv/csv-parser/parser.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
// https://github.com/AriaFallah/csv-parser

// FLS_CHG
// ADD LAST_FIELD_EMPTY for cases like ||\n
// Handle empty last field at row end without a separate LAST_FIELD_EMPTY state,
// and ensure pending ROW_END is emitted before CSV_END at EOF.
//
// NOLINTBEGIN

Expand Down Expand Up @@ -62,7 +63,6 @@ class CsvParser {
// CSV state for state machine
enum class State {
START_OF_FIELD, //
LAST_FIELD_EMPTY,
IN_FIELD,
IN_QUOTED_FIELD,
IN_ESCAPED_QUOTE,
Expand Down Expand Up @@ -136,6 +136,12 @@ class CsvParser {

// Reads a single field from the CSV
Field next_field() {
// If we just finished a row previously, emit its ROW_END now
if (m_state == State::END_OF_ROW) {
m_state = State::START_OF_FIELD;
return Field(FieldType::ROW_END);
}

if (empty()) {
return Field(FieldType::CSV_END);
}
Expand All @@ -158,16 +164,13 @@ class CsvParser {
switch (m_state) {
case State::START_OF_FIELD:
m_cursor++;
// FLS_CHG
// if (c == m_terminator) {
// handle_crlf(c);
// return Field(FieldType::ROW_END);
// }
// Handles multiline strings. Disabled by default, but we enable it as we have multiline strings.
if (c == m_quote) { // && false) { // forget about quoting, our csv input is not legal anyway
m_state = State::IN_QUOTED_FIELD;
} else if (c == m_terminator) {
m_state = State::LAST_FIELD_EMPTY;
// Empty last field at end of row
handle_crlf(c);
m_state = State::END_OF_ROW;
return Field(m_fieldbuf);
} else if (c == m_delimiter) {
return Field(m_fieldbuf);
Expand Down Expand Up @@ -227,12 +230,11 @@ class CsvParser {
break;

case State::END_OF_ROW:
// Normally unreachable because we emit ROW_END early at the top,
// but keep the fallback for safety.
m_state = State::START_OF_FIELD;
return Field(FieldType::ROW_END);

case State::LAST_FIELD_EMPTY:
return Field(FieldType::ROW_END);

case State::EMPTY:
throw std::logic_error("You goofed");
}
Expand Down
5 changes: 0 additions & 5 deletions test/src/dataset_tests/issue.cpp

This file was deleted.

4 changes: 4 additions & 0 deletions test/src/dataset_tests/issues.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,8 @@ TEST_F(FastLanesReaderTester, issue_000) {
TestCorrectness(issues::ISSUE_000);
}

TEST_F(FastLanesReaderTester, issue_055) {
TestCorrectness(issues::ISSUE_055);
}

} // namespace fastlanes
2 changes: 1 addition & 1 deletion test/src/quick_fuzz_tests/fuzz_config.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"num_cases": 10,
"base_seed": 6,
"base_seed": 7,
"delimiter": "|",
"min_cols": 1,
"max_cols": 2,
Expand Down
1 change: 1 addition & 0 deletions test/src/unit_tests/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
add_executable(unit_test
csv_parser_trailing_delimiter_test.cpp
csv_reader_test.cpp
double_test.cpp
json_test.cpp
Expand Down
80 changes: 80 additions & 0 deletions test/src/unit_tests/csv_parser_trailing_delimiter_test.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
// ────────────────────────────────────────────────────────
// | FastLanes |
// ────────────────────────────────────────────────────────
// test/src/unit_tests/csv_parser_trailing_delimiter_test.cpp
// ────────────────────────────────────────────────────────
#include "fls/common/alias.hpp"
#include "fls/common/assert.hpp"
#include "fls/csv/csv-parser/parser.hpp"
#include "fls/csv/csv.hpp"
#include "gtest/gtest.h"
#include <sstream>

namespace fastlanes {

std::vector<std::string> scanTokens(aria::csv::CsvParser& p, size_t safety_limit = 10000) {
std::vector<std::string> out;
size_t steps = 0;
for (;;) {
if (++steps > safety_limit) {
// If we ever hit this, the parser is stuck (buggy behavior).
// This makes the test fail fast instead of hanging.
out.push_back("<SAFETY_LIMIT_HIT>");
break;
}
aria::csv::Field f = p.next_field();
if (f.type == aria::csv::FieldType::CSV_END) {
out.emplace_back("CSV_END");
break;
} else if (f.type == aria::csv::FieldType::ROW_END) {
out.emplace_back("ROW_END");
} else {
out.emplace_back(*f.data);
}
}
return out;
}

} // namespace fastlanes

// Bring types & helper into global scope for the TESTs:
using aria::csv::CsvParser;
using fastlanes::scanTokens;

TEST(CsvParserBug, TrailingDelimiterLF) {
// A single row that ends with a delimiter before '\n' → last field is empty.
const std::string csv = "001|2014-08-24 18:00:00|3|0|21|19|0|\n";

std::istringstream iss(csv);
CsvParser parser(iss);
parser.delimiter('|'); // important for this dataset

auto tokens = scanTokens(parser);

// With the bug: tokens ends with many "ROW_END" and finally "<SAFETY_LIMIT_HIT>"
// With the fix: we should see fields, then one empty field (""), one ROW_END, then CSV_END.
std::vector<std::string> expected = {
"001", "2014-08-24 18:00:00", "3", "0", "21", "19", "0", "", "ROW_END", "CSV_END"};

// If the parser is stuck, the sentinel appears and this assertion helps diagnose.
ASSERT_EQ(tokens.back(), "CSV_END") << "Parser did not reach CSV_END; tokens: " << tokens.back();

// Compare full sequence.
EXPECT_EQ(tokens, expected);
}

TEST(CsvParserBug, TrailingDelimiterCRLF) {
// Same scenario, but with Windows line ending to ensure CRLF handling works.
const std::string csv = "A|B|C|\r\n";

std::istringstream iss(csv);
CsvParser parser(iss);
parser.delimiter('|'); // default terminator is CRLF

auto tokens = scanTokens(parser);

std::vector<std::string> expected = {"A", "B", "C", "", "ROW_END", "CSV_END"};

ASSERT_EQ(tokens.back(), "CSV_END") << "Parser did not reach CSV_END; tokens: " << tokens.back();
EXPECT_EQ(tokens, expected);
}
Loading