From 6580c938b67c2b6b847847d6eecb2ca16e6aa6ea Mon Sep 17 00:00:00 2001 From: Prakhar Singh Date: Wed, 3 Jun 2026 12:45:02 +0530 Subject: [PATCH 1/2] [df] fix method call mistaken for column in Define() A method call on an object can share its name with a column. When that happens, the JIT expression becomes invalid and the Define() call throws a runtime error, even though the user's C++ is correct. Fixes https://github.com/root-project/root/issues/22295 --- tree/dataframe/src/RDFInterfaceUtils.cxx | 10 ++++- tree/dataframe/test/dataframe_colnames.cxx | 51 ++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/tree/dataframe/src/RDFInterfaceUtils.cxx b/tree/dataframe/src/RDFInterfaceUtils.cxx index 30a694b089a7f..11f2f955e7e2f 100644 --- a/tree/dataframe/src/RDFInterfaceUtils.cxx +++ b/tree/dataframe/src/RDFInterfaceUtils.cxx @@ -106,6 +106,12 @@ std::pair FindUsedColsAndAliases(const std::string // token is not a potential variable name, skip it continue; } + // Skip symbols that are member accesses (obj.method) — they are not column references. + // lexertk does not produce dot-prefixed tokens, so the token immediately before a method + // name is always the literal "." token when it is a member access. + if (i > 0 && tokens[i - 1].value == ".") { + continue; + } ColumnNames_t potentialColNames({tok.value}); @@ -159,7 +165,7 @@ TString ResolveAliases(const TString &expr, const ColumnNames_t &usedAliases, for (const auto &alias : usedAliases) { const auto &col = colRegister.ResolveAlias(alias); - TPRegexp replacer("\\b" + EscapeDots(alias) + "\\b"); + TPRegexp replacer("(? b.size(); }); for (const auto &col : usedCols) { const auto varIdx = std::distance(usedCols.begin(), std::find(usedCols.begin(), usedCols.end(), col)); - TPRegexp replacer("\\b" + EscapeDots(col) + "\\b"); + TPRegexp replacer("(? #include #include "gtest/gtest.h" @@ -89,3 +90,53 @@ TEST(Aliases, FilterOnAliasJit) auto c = tdf.Define("c0", [&i]() { return i++; }).Alias("c1", "c0").Filter("c1>1").Count(); EXPECT_EQ(1U, *c); } + +// Regression test for https://github.com/root-project/root/issues/22295 +// A column named "phi" must not be confused with a .phi() method call in a JIT expression. +TEST(ColNames, MethodCallNotConfusedWithColumn) +{ + // Declare minimal helper types for use in JIT strings. + // Using gInterpreter::Declare avoids any dependency on GenVector or other libraries. + gInterpreter->Declare("struct RDF22295Vec { double phi() const { return 0.0; } };"); + gInterpreter->Declare("struct RDF22295Mem { double phi = 1.5; };"); + + // Case 1: column "phi" exists, expression calls .phi() method — must not throw. + // Before the fix this crashed with "no member named 'var0'" because the parser + // replaced .phi() with .var0(). + { + ROOT::RDataFrame df(1); + auto df2 = df.Define("phi", "1.0"); + EXPECT_NO_THROW({ + auto df3 = df2.Define("result", "RDF22295Vec{}.phi()"); + (void)*df3.Take("result"); + }); + } + + // Case 2: column "phi" exists, expression uses BOTH .phi() method call AND standalone + // phi column reference — only the standalone reference must be substituted. + { + ROOT::RDataFrame df(1); + auto df2 = df.Define("phi", "0.5"); + std::vector v; + EXPECT_NO_THROW({ + auto df3 = df2.Define("result", "RDF22295Vec{}.phi() + phi"); + v = *df3.Take("result"); + }); + // .phi() returns 0.0; standalone phi column = 0.5 + EXPECT_NEAR(0.5, v[0], 1e-9); + } + + // Case 3: data-member access obj.phi (not a function call) must still work after the + // fix — the preceding-dot guard must not break legitimate dot-chain expressions. + { + ROOT::RDataFrame df(1); + auto df2 = df.Define("phi", "0.5"); + std::vector v; + EXPECT_NO_THROW({ + auto df3 = df2.Define("result", "RDF22295Mem{}.phi + phi"); + v = *df3.Take("result"); + }); + // data member .phi = 1.5; standalone phi column = 0.5 + EXPECT_NEAR(2.0, v[0], 1e-9); + } +} From 63a66a386c55cf2747ce4f1d13c0d32f39481de6 Mon Sep 17 00:00:00 2001 From: Prakhar Singh Date: Wed, 3 Jun 2026 14:02:13 +0530 Subject: [PATCH 2/2] [df] address review comments on test Use a non-zero sentinel value in the helper struct and add a value assertion so the test proves the correct method result is returned. --- tree/dataframe/test/dataframe_colnames.cxx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/tree/dataframe/test/dataframe_colnames.cxx b/tree/dataframe/test/dataframe_colnames.cxx index 2610d7e9a3e55..66960f92fe282 100644 --- a/tree/dataframe/test/dataframe_colnames.cxx +++ b/tree/dataframe/test/dataframe_colnames.cxx @@ -97,19 +97,22 @@ TEST(ColNames, MethodCallNotConfusedWithColumn) { // Declare minimal helper types for use in JIT strings. // Using gInterpreter::Declare avoids any dependency on GenVector or other libraries. - gInterpreter->Declare("struct RDF22295Vec { double phi() const { return 0.0; } };"); + gInterpreter->Declare("struct RDF22295Vec { double phi() const { return 42.0; } };"); gInterpreter->Declare("struct RDF22295Mem { double phi = 1.5; };"); - // Case 1: column "phi" exists, expression calls .phi() method — must not throw. - // Before the fix this crashed with "no member named 'var0'" because the parser - // replaced .phi() with .var0(). + // Case 1: column "phi" exists, expression calls .phi() method — must not throw, + // and the result must be the value returned by the method (42.0), not a substitution + // artifact. Before the fix this crashed with "no member named 'var0'" because the + // parser replaced .phi() with .var0(). { ROOT::RDataFrame df(1); auto df2 = df.Define("phi", "1.0"); + std::vector v; EXPECT_NO_THROW({ auto df3 = df2.Define("result", "RDF22295Vec{}.phi()"); - (void)*df3.Take("result"); + v = *df3.Take("result"); }); + EXPECT_NEAR(42.0, v[0], 1e-9); } // Case 2: column "phi" exists, expression uses BOTH .phi() method call AND standalone @@ -122,8 +125,8 @@ TEST(ColNames, MethodCallNotConfusedWithColumn) auto df3 = df2.Define("result", "RDF22295Vec{}.phi() + phi"); v = *df3.Take("result"); }); - // .phi() returns 0.0; standalone phi column = 0.5 - EXPECT_NEAR(0.5, v[0], 1e-9); + // .phi() returns 42.0; standalone phi column = 0.5 + EXPECT_NEAR(42.5, v[0], 1e-9); } // Case 3: data-member access obj.phi (not a function call) must still work after the