From 12e30cff812ee57b650acf92788467ac38261776 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Thu, 22 Oct 2026 20:10:14 +0200 Subject: [PATCH 1/2] [df] Consider quoted strings in collection size short syntax The regex pattern present before was not refined enough to consider the case where the target #varname could happen to be within a quoted string. Improve the regex pattern in ParseRDFExpression and add a more complicated test case in dataframe_interface.cxx Fixes #23790 (cherry picked from commit cb1b496f8cf1d5bad3fdb2ac850e251190f4677d) --- tree/dataframe/src/RDFInterfaceUtils.cxx | 17 +++++++++++--- tree/dataframe/test/dataframe_interface.cxx | 25 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/tree/dataframe/src/RDFInterfaceUtils.cxx b/tree/dataframe/src/RDFInterfaceUtils.cxx index 30a694b089a7f..f76e1070e13d7 100644 --- a/tree/dataframe/src/RDFInterfaceUtils.cxx +++ b/tree/dataframe/src/RDFInterfaceUtils.cxx @@ -171,10 +171,21 @@ ParsedExpression ParseRDFExpression(std::string_view expr, const ROOT::Internal: { // transform `#var` into `R_rdf_sizeof_var` TString preProcessedExpr(expr); - // match #varname at beginning of the sentence or after not-a-word, but exclude preprocessor directives like #ifdef + // regex explanation: + // # + // - matches the literal character # + // (?!(ifdef|ifndef|if|else|elif|endif|pragma|define|undef|include|line)) + // - negative lookahead (i.e. don't match these occurrences) to exclude preprocessor directives e.g. #ifdef + // ([a-zA-Z_][a-zA-Z0-9_]*) + // - match characters that make up the variable name after the # + // (?=(?:[^"]*"[^"]*")*[^"]*$) + // - positive lookahead (i.e. match these occurrences) checking that there is always an even number of quotes + // *after* the matched character sequence, using the non-capturing group (?:[^"]*"[^"]*"). We leverage the + // fact that for any character sequence to be within quotes there must be an odd number of quotes after the + // sequence and before the end of the line. TPRegexp colSizeReplacer( - "(^|\\W)#(?!(ifdef|ifndef|if|else|elif|endif|pragma|define|undef|include|line))([a-zA-Z_][a-zA-Z0-9_]*)"); - colSizeReplacer.Substitute(preProcessedExpr, "$1R_rdf_sizeof_$3", "g"); + R"REGEX(#(?!(ifdef|ifndef|if|else|elif|endif|pragma|define|undef|include|line))([a-zA-Z_][a-zA-Z0-9_]*)(?=(?:[^"]*"[^"]*")*[^"]*$))REGEX"); + colSizeReplacer.Substitute(preProcessedExpr, "R_rdf_sizeof_$2", "g"); ColumnNames_t usedCols; ColumnNames_t usedAliases; diff --git a/tree/dataframe/test/dataframe_interface.cxx b/tree/dataframe/test/dataframe_interface.cxx index 3411403ffcba5..5c9f4f9d2c304 100644 --- a/tree/dataframe/test/dataframe_interface.cxx +++ b/tree/dataframe/test/dataframe_interface.cxx @@ -845,6 +845,31 @@ TEST(RDataFrameInterface, StressShortSyntaxForCollectionSizes) EXPECT_EQ(c, 42ull); } +// Regression tests for https://github.com/root-project/root/issues/23790 +TEST(RDataFrameInterface, ShortSyntaxForCollectionSizesMultipleStringsAndVars) +{ + // We need to define first four target columns that have the longer name for collection size + auto df = ROOT::RDF::RNode(ROOT::RDataFrame(1)); + for (auto i = 1; i <= 4; i++) + df = df.Define("R_rdf_sizeof_var" + std::to_string(i), []() -> int { return 1; }); + std::string code{R"CODE( + auto value1 = #var1; // comment + #ifdef NONEXISTENTMACRO + // comment with "quoted string" + #endif + std::string simplestr("#simplestr"); // comment + std::string mystr("anything#hello"); // comment + auto value2 = #var2; // comment with "another string" + std::string anotherstr("We don't want #var3 and #var4 to be replaced"); + auto value5 = #var1 + #var2; std::string innerRawString{R"INNER( + This raw string contains #myvar which should + not be changed.)INNER"}; + return #var1 + #var2 + #var3 + #var4 + value1 + value2 + value5; + )CODE"}; + auto sum = df.Define("target_col", code).Sum("target_col"); + EXPECT_EQ(*sum, 8); +} + TEST(RDataFrameInterface, MutableForeach) { int i = 0; From 98a20ff2e1e121ac6dd6c512b30078a547fb4987 Mon Sep 17 00:00:00 2001 From: Vincenzo Eduardo Padulano Date: Fri, 23 Oct 2026 11:04:21 +0200 Subject: [PATCH 2/2] [df] Test potential string ambiguity with short syntax Also add a test where the user defines a column of type std::string with a value that could be ambiguous w.r.t. the short syntax for collection sizes. RDataFrame should not substitute the string value in this case, it should be returned as-is to downstream actions. Co-authored-by: muntasir-396 (cherry picked from commit c4011b297ee17e503f4109dc7fc5993279704f82) --- tree/dataframe/test/dataframe_interface.cxx | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tree/dataframe/test/dataframe_interface.cxx b/tree/dataframe/test/dataframe_interface.cxx index 5c9f4f9d2c304..5ee2235ab236c 100644 --- a/tree/dataframe/test/dataframe_interface.cxx +++ b/tree/dataframe/test/dataframe_interface.cxx @@ -870,6 +870,14 @@ TEST(RDataFrameInterface, ShortSyntaxForCollectionSizesMultipleStringsAndVars) EXPECT_EQ(*sum, 8); } +// Regression test for https://github.com/root-project/root/issues/23790 +TEST(RDataFrameInterface, StringWithPotentialAmbiguityWithSortSyntaxForCollectionSizes) +{ + auto df = ROOT::RDataFrame(1).Define("col", "std::string(\"#a\")").Filter("col == \"#a\""); + EXPECT_EQ(df.Count().GetValue(), 1ull); + EXPECT_EQ(*df.Take("col")->begin(), "#a"); +} + TEST(RDataFrameInterface, MutableForeach) { int i = 0;