Skip to content

fix: bad downcast of ArrowResultCollector in plan mapper re-enables skipped arrow/CSR tests (#881) #884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
adsharma merged 1 commit into from
Sep 1, 2026
Merged

fix: bad downcast of ArrowResultCollector in plan mapper re-enables skipped arrow/CSR tests (#881) #884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 8 additions & 5 deletions src/processor/map/plan_mapper.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,14 @@ std::unique_ptr<PhysicalPlan> PlanMapper::getPhysicalPlan(const LogicalPlan* log
} else {
root = createResultCollector(AccumulateType::REGULAR, expressions,
logicalPlan->getSchema(), std::move(root));
}
// The plan root collector is the only one whose table is handed to the client via
// getQueryResult(); every other ResultCollector feeds other operators of the same
// plan and must be cleared in place (not replaced) on reuse.
if (root->getOperatorType() == PhysicalOperatorType::RESULT_COLLECTOR) {
// The plan root collector is the only one whose table is handed to the client via
// getQueryResult(); every other ResultCollector feeds other operators of the same
// plan and must be cleared in place (not replaced) on reuse.
// NOTE: This must only run on the regular (factorized-table) ResultCollector.
// ArrowResultCollector shares PhysicalOperatorType::RESULT_COLLECTOR but is NOT a
// ResultCollector — downcasting it here is undefined behaviour (and trips the
// checked dynamic_cast under RUNTIME_CHECKS), so the flag is set inside the
// non-arrow branch instead of after the if/else via an operator-type check.
root->ptrCast<ResultCollector>()->setResultExposedToClient();
}
}
Expand Down
15 changes: 0 additions & 15 deletions test/api/arrow_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -495,9 +495,6 @@ TEST_F(ArrowTest, resultToArrow) {
}

TEST_F(ArrowTest, queryAsArrow) {
// TODO(#881): intermittent SIGSEGV race in the arrow collector task path (worker threads
// execute a corrupted task clone). Skip until the race is fixed.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
auto query = "MATCH (a:person) WHERE a.fName = 'Bob' RETURN a.fName";
auto result = conn->queryAsArrow(query, 1);
auto arrowArray = result->getNextArrowChunk(1);
Expand All @@ -511,8 +508,6 @@ TEST_F(ArrowTest, queryAsArrow) {
}

TEST_F(ArrowTest, getArrowResult) {
// TODO(#881): intermittent SIGSEGV race in the arrow collector task path. Skip until fixed.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
auto query = "MATCH (a:person) WHERE a.fName = 'Bob' RETURN a.fName";
auto result = conn->queryAsArrow(query, 1);
try {
Expand Down Expand Up @@ -599,8 +594,6 @@ TEST_F(ArrowTest, mapColumnArrowSchemaHasNonNullableEntriesAndKey) {
}

TEST_F(ArrowTest, queryAsArrowDirectCSRRowIDProjection) {
// TODO(#881): intermittent SIGSEGV / CSR-loss race in the arrow collector task path.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
ASSERT_TRUE(
conn->query("CREATE NODE TABLE DirectPerson(id INT64, PRIMARY KEY(id));")->isSuccess());
ASSERT_TRUE(conn->query("CREATE REL TABLE DirectKnows(FROM DirectPerson TO DirectPerson);")
Expand Down Expand Up @@ -653,8 +646,6 @@ TEST_F(ArrowTest, queryAsArrowDirectCSRRowIDProjection) {
}

TEST_F(ArrowTest, queryAsArrowDirectCSRRowIDProjectionKeepsCSRMetadataWithFourThreads) {
// TODO(#881): intermittent SIGSEGV / CSR-loss race in the arrow collector task path.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
auto query = "MATCH (a:person)-[b:knows]->(c:person) RETURN a.rowid, b.rowid, c.rowid "
"ORDER BY a.rowid, b.rowid, c.rowid";
conn->setMaxNumThreadForExec(4);
Expand All @@ -665,8 +656,6 @@ TEST_F(ArrowTest, queryAsArrowDirectCSRRowIDProjectionKeepsCSRMetadataWithFourTh
}

TEST_F(ArrowTest, queryAsArrowTracksCSRMetadataWithoutRelIDs) {
// TODO(#881): intermittent SIGSEGV / CSR-loss race in the arrow collector task path.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
auto query =
"MATCH (a:person)-[:knows]->(b:person) RETURN a.rowid, b.rowid ORDER BY a.rowid, b.rowid";
auto rowResult = conn->query(query);
Expand Down Expand Up @@ -712,8 +701,6 @@ TEST_F(ArrowTest, queryAsArrowTracksCSRMetadataWithoutRelIDs) {
}

TEST_F(ArrowTest, queryAsArrowTracksCSRMetadataWithRelIDsAndExtraColumns) {
// TODO(#881): intermittent SIGSEGV / CSR-loss race in the arrow collector task path.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
auto query = "MATCH (a:person)-[e:knows]->(b:person) "
"RETURN a.rowid, e.rowid, b.rowid, e.date, b.fName "
"ORDER BY a.rowid, e.rowid, b.rowid";
Expand Down Expand Up @@ -756,8 +743,6 @@ TEST_F(ArrowTest, queryAsArrowTracksCSRMetadataWithRelIDsAndExtraColumns) {
}

TEST_F(ArrowTest, queryAsArrowDoesNotTrackCSRMetadataForNonCSRShape) {
// TODO(#881): intermittent SIGSEGV / CSR-loss race in the arrow collector task path.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
auto query = "MATCH (a:person)-[e:knows]->(b:person) RETURN a.rowid, e.date ORDER BY a.rowid";
auto result = conn->queryAsArrow(query, 8);
auto* arrowResult = dynamic_cast<ArrowQueryResult*>(result.get());
Expand Down
4 changes: 0 additions & 4 deletions test/api/project_graph_csr_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ class ProjectGraphCsrTest : public ApiTest {
};

TEST_F(ProjectGraphCsrTest, materializesArrowCsr) {
// TODO(#881): intermittent SIGSEGV / CSR-loss race in the arrow collector task path.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
ASSERT_TRUE(conn->query("CALL PROJECT_GRAPH('CsrG', ['CsrNode'], ['CsrEdge'])")->isSuccess());
const auto& entry = getNativeEntry("CsrG");
ASSERT_EQ(entry.relCsrResults.size(), 1u);
Expand All @@ -51,8 +49,6 @@ TEST_F(ProjectGraphCsrTest, materializesArrowCsr) {
}

TEST_F(ProjectGraphCsrTest, materializedCsrSurvivesConsumingQueries) {
// TODO(#881): intermittent SIGSEGV / CSR-loss race in the arrow collector task path.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
ASSERT_TRUE(conn->query("CALL PROJECT_GRAPH('CsrG', ['CsrNode'], ['CsrEdge'])")->isSuccess());
// The pinned result must stay valid across later statements on the same connection.
ASSERT_TRUE(conn->query("MATCH (a:CsrNode) RETURN COUNT(*)")->isSuccess());
Expand Down
2 changes: 0 additions & 2 deletions test/api/read_only_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ TEST_F(ReadOnlyTest, Test) {
}

TEST_F(ReadOnlyTest, ProjectGraphOnReadOnlyDatabase) {
// TODO(#881): intermittent SIGSEGV / CSR-loss race in the arrow collector task path.
GTEST_SKIP() << "Flaky SIGSEGV in the arrow collector task path; see issue #881.";
if (databasePath == "" || databasePath == ":memory:") {
return;
}
Expand Down
Loading