tp: add structured query mode to query subcommand (#5198)

## Summary

- Add `--structured-query-id` and `--summary-spec` flags to the `query`
  subcommand, using the Summarizer API to materialize and print a single
  structured query result. Intended for debugging/testing.
- Remove the legacy `ExecuteStructuredQuery()` from summary.{h,cc} and
the
  old `--structured-query-spec`/`--structured-query-id` classic flags.
- Migrate diff test infra to use the new subcommand syntax.

## Stack
- **#5198 tp: add structured query mode to query subcommand** (this PR)
- #5149 tp: classic-to-subcommand translation layer

## Test plan
- [x] `perfetto_integrationtests
--gtest_filter="TraceProcessorShellIntegrationTest.*"` (45 tests pass)
- [x] `tools/diff_test_trace_processor.py
--name-filter=".*structured.*"` (4 tests pass)

Author: LalitMaganti

python/generators/diff_tests/test_executor.py

@@ -318,10 +318,11 @@ def run(self, test: TestCase, trace_path: str) -> TestResult:
       try:
         cmd = [
             self.trace_processor_path,
+            'query',
             '--analyze-trace-proto-content',
             '--crop-track-events',
             '--extra-checks',
-            '--structured-query-spec',
+            '--summary-spec',
             spec_file_path,
             '--structured-query-id',
             test.blueprint.query.query_id,

src/trace_processor/shell/BUILD.gn

@@ -51,10 +51,12 @@ source_set("shell") {
     "../../../protos/perfetto/trace_processor:zero",
     "../../base",
     "../../base:version",
+    "../../protozero/text_to_proto:text_to_proto",
     "../metrics",
     "../rpc",
     "../rpc:stdiod",
     "../trace_summary",
+    "../trace_summary:gen_cc_trace_summary_descriptor",
     "../util:stdlib",
     "../util/deobfuscation:deobfuscator",
     "../util/symbolizer",

src/trace_processor/shell/query_subcommand.cc

@@ -16,7 +16,11 @@
 
 #include "src/trace_processor/shell/query_subcommand.h"
 
+#include <algorithm>
+#include <cstdint>
+#include <memory>
 #include <string>
+#include <string_view>
 #include <vector>
 
 #include "perfetto/base/build_config.h"
@@ -25,10 +29,14 @@
 #include "perfetto/base/time.h"
 #include "perfetto/ext/base/file_utils.h"
 #include "perfetto/ext/base/status_macros.h"
+#include "perfetto/ext/base/string_utils.h"
+#include "perfetto/trace_processor/summarizer.h"
+#include "src/protozero/text_to_proto/text_to_proto.h"
 #include "src/trace_processor/shell/common_flags.h"
 #include "src/trace_processor/shell/metatrace.h"
 #include "src/trace_processor/shell/query.h"
 #include "src/trace_processor/shell/subcommand.h"
+#include "src/trace_processor/trace_summary/trace_summary.descriptor.h"
 
 #if PERFETTO_BUILDFLAG(PERFETTO_OS_WIN)
 #include <io.h>
@@ -41,6 +49,63 @@
 #endif
 
 namespace perfetto::trace_processor::shell {
+namespace {
+
+// Returns true if the file at |path| with |content| should be treated as
+// textproto rather than binary proto. Uses the same heuristic as the
+// classic codepath: .pb → binary, .textproto → text, otherwise
+// content-sniff the first 128 bytes (all printable/whitespace → text).
+bool IsTextproto(const std::string& path, const std::string& content) {
+  if (base::EndsWith(path, ".pb")) {
+    return false;
+  }
+  if (base::EndsWith(path, ".textproto")) {
+    return true;
+  }
+  std::string_view prefix(content.c_str(),
+                          std::min<size_t>(content.size(), 128));
+  return std::all_of(prefix.begin(), prefix.end(),
+                     [](char c) { return std::isspace(c) || std::isprint(c); });
+}
+
+// Reads a TraceSummarySpec file and passes it to |summarizer|, converting
+// textproto to binary proto when necessary.
+base::Status LoadSpecIntoSummarizer(Summarizer* summarizer,
+                                    const std::string& path) {
+  std::string content;
+  if (!base::ReadFile(path, &content)) {
+    return base::ErrStatus("Unable to read spec file %s", path.c_str());
+  }
+
+  const uint8_t* spec_data;
+  size_t spec_size;
+  std::vector<uint8_t> binary_proto;
+  if (IsTextproto(path, content)) {
+    ASSIGN_OR_RETURN(binary_proto,
+                     protozero::TextToProto(kTraceSummaryDescriptor.data(),
+                                            kTraceSummaryDescriptor.size(),
+                                            ".perfetto.protos.TraceSummarySpec",
+                                            "-", std::string_view(content)));
+    spec_data = binary_proto.data();
+    spec_size = binary_proto.size();
+  } else {
+    spec_data = reinterpret_cast<const uint8_t*>(content.data());
+    spec_size = content.size();
+  }
+
+  SummarizerUpdateSpecResult update_result;
+  RETURN_IF_ERROR(summarizer->UpdateSpec(spec_data, spec_size, &update_result));
+  for (const auto& q : update_result.queries) {
+    if (q.error.has_value()) {
+      return base::ErrStatus("Error in query '%s' from spec '%s': %s",
+                             q.query_id.c_str(), path.c_str(),
+                             q.error->c_str());
+    }
+  }
+  return base::OkStatus();
+}
+
+}  // namespace
 
 const char* QuerySubcommand::name() const {
   return "query";
@@ -63,13 +128,25 @@ SQL can be provided in three ways:
   3. From stdin:           cat q.sql | tp query trace.pb
 
 Multiple semicolon-separated statements are supported. Use -i to drop into
-an interactive shell after the queries complete.)";
+an interactive shell after the queries complete.
+
+Advanced (for debugging/testing structured queries):
+  --structured-query-id ID --summary-spec FILE [...]
+  Executes a single structured query by ID from the given summary spec
+  files. The spec files replace -f/stdin/positional SQL. Output is the
+  query result table.)";
 }
 
 std::vector<FlagSpec> QuerySubcommand::GetFlags() {
   return {
       StringFlag("query-file", 'f', "FILE",
                  "Read SQL from FILE (use '-' for stdin).", &query_file_),
+      StringFlag("structured-query-id", '\0', "ID",
+                 "[Advanced] Run a single structured query by ID.",
+                 &structured_query_id_),
+      {"summary-spec", '\0', true, "FILE",
+       "[Advanced] Summary spec file for structured queries (repeatable).",
+       [this](const char* v) { structured_query_specs_.emplace_back(v); }},
       BoolFlag("interactive", 'i', "Start interactive shell after query.",
                &interactive_),
       BoolFlag("wide", 'W', "Double column width for output.", &wide_),
@@ -84,6 +161,11 @@ base::Status QuerySubcommand::Run(const SubcommandContext& ctx) {
   }
   std::string trace_file = ctx.positional_args[0];
 
+  // Advanced: structured query mode.
+  if (!structured_query_id_.empty()) {
+    return RunStructuredQuery(ctx, trace_file);
+  }
+
   // Determine SQL source:
   //   1. Positional:  query trace.pb "SELECT ..."
   //   2. File:        query -f file.sql trace.pb
@@ -112,8 +194,6 @@ base::Status QuerySubcommand::Run(const SubcommandContext& ctx) {
   ASSIGN_OR_RETURN(auto t_load,
                    LoadTraceFile(tp.get(), ctx.platform, trace_file));
 
-  // If we have a file, read it into sql. After this point, sql always has
-  // the SQL to execute.
   if (!query_file_.empty()) {
     if (!base::ReadFile(query_file_, &sql)) {
       return base::ErrStatus("query: unable to read file '%s'",
@@ -128,12 +208,51 @@ base::Status QuerySubcommand::Run(const SubcommandContext& ctx) {
     MaybeWriteMetatrace(tp.get(), ctx.global->metatrace_path);
     return status;
   }
-
   base::TimeNanos t_query = base::GetWallTimeNs() - t_query_start;
 
-  if (!perf_file_.empty())
+  if (!perf_file_.empty()) {
     RETURN_IF_ERROR(PrintPerfFile(perf_file_, t_load, t_query));
+  }
+  RETURN_IF_ERROR(MaybeWriteMetatrace(tp.get(), ctx.global->metatrace_path));
+  return base::OkStatus();
+}
+
+base::Status QuerySubcommand::RunStructuredQuery(
+    const SubcommandContext& ctx,
+    const std::string& trace_file) {
+  if (structured_query_specs_.empty()) {
+    return base::ErrStatus(
+        "query: --structured-query-id requires at least one --summary-spec");
+  }
+
+  auto config = BuildConfig(*ctx.global, ctx.platform);
+  ASSIGN_OR_RETURN(auto tp,
+                   SetupTraceProcessor(*ctx.global, config, ctx.platform));
+  ASSIGN_OR_RETURN(auto t_load,
+                   LoadTraceFile(tp.get(), ctx.platform, trace_file));
 
+  std::unique_ptr<Summarizer> summarizer;
+  RETURN_IF_ERROR(tp->CreateSummarizer(&summarizer));
+  for (const auto& path : structured_query_specs_) {
+    RETURN_IF_ERROR(LoadSpecIntoSummarizer(summarizer.get(), path));
+  }
+
+  base::TimeNanos t_query_start = base::GetWallTimeNs();
+  SummarizerQueryResult query_result;
+  RETURN_IF_ERROR(summarizer->Query(structured_query_id_, &query_result));
+  if (!query_result.exists) {
+    return base::ErrStatus(
+        "Structured query ID '%s' not found in the provided spec files",
+        structured_query_id_.c_str());
+  }
+
+  RETURN_IF_ERROR(
+      RunQueries(tp.get(), "SELECT * FROM " + query_result.table_name, true));
+  base::TimeNanos t_query = base::GetWallTimeNs() - t_query_start;
+
+  if (!perf_file_.empty()) {
+    RETURN_IF_ERROR(PrintPerfFile(perf_file_, t_load, t_query));
+  }
   RETURN_IF_ERROR(MaybeWriteMetatrace(tp.get(), ctx.global->metatrace_path));
   return base::OkStatus();
 }

src/trace_processor/shell/query_subcommand.h

@@ -35,7 +35,12 @@ class QuerySubcommand : public Subcommand {
   base::Status Run(const SubcommandContext& ctx) override;
 
  private:
+  base::Status RunStructuredQuery(const SubcommandContext& ctx,
+                                  const std::string& trace_file);
+
   std::string query_file_;
+  std::string structured_query_id_;
+  std::vector<std::string> structured_query_specs_;
   bool interactive_ = false;
   bool wide_ = false;
   std::string perf_file_;

src/trace_processor/trace_processor_shell.cc

@@ -72,7 +72,6 @@
 #include "src/trace_processor/shell/sql_packages.h"
 #include "src/trace_processor/shell/subcommand.h"
 #include "src/trace_processor/shell/summarize_subcommand.h"
-#include "src/trace_processor/trace_summary/summary.h"
 #include "src/trace_processor/util/deobfuscation/deobfuscator.h"
 #include "src/trace_processor/util/symbolizer/symbolize_database.h"
 
@@ -127,8 +126,6 @@ struct CommandLineOptions {
 
   std::string query_file_path;
   std::string query_string;
-  std::vector<std::string> structured_query_specs;
-  std::string structured_query_id;
   std::vector<std::string> sql_package_paths;
   std::vector<std::string> override_sql_package_paths;
 
@@ -467,10 +464,6 @@ const option kLongOptions[] = {
 
     {"query-file", required_argument, nullptr, 'q'},
     {"query-string", required_argument, nullptr, 'Q'},
-    {"structured-query-spec", required_argument, nullptr,
-     OPT_STRUCTURED_QUERY_SPEC},
-    {"structured-query-id", required_argument, nullptr,
-     OPT_STRUCTURED_QUERY_ID},
     {"add-sql-package", required_argument, nullptr, OPT_ADD_SQL_PACKAGE},
     {"override-sql-package", required_argument, nullptr,
      OPT_OVERRIDE_SQL_PACKAGE},
@@ -641,16 +634,6 @@ CommandLineOptions ParseCommandLineOptions(int argc, char** argv) {
       continue;
     }
 
-    if (option == OPT_STRUCTURED_QUERY_SPEC) {
-      command_line_options.structured_query_specs.emplace_back(optarg);
-      continue;
-    }
-
-    if (option == OPT_STRUCTURED_QUERY_ID) {
-      command_line_options.structured_query_id = optarg;
-      continue;
-    }
-
     if (option == OPT_OVERRIDE_STDLIB) {
       command_line_options.override_stdlib_path = optarg;
       continue;
@@ -724,13 +707,11 @@ CommandLineOptions ParseCommandLineOptions(int argc, char** argv) {
   }
 
   command_line_options.launch_shell =
-      explicit_interactive ||
-      (command_line_options.metric_v1_names.empty() &&
-       command_line_options.query_file_path.empty() &&
-       command_line_options.query_string.empty() &&
-       command_line_options.structured_query_id.empty() &&
-       command_line_options.export_file_path.empty() &&
-       !command_line_options.summary);
+      explicit_interactive || (command_line_options.metric_v1_names.empty() &&
+                               command_line_options.query_file_path.empty() &&
+                               command_line_options.query_string.empty() &&
+                               command_line_options.export_file_path.empty() &&
+                               !command_line_options.summary);
 
   // Only allow non-interactive queries to emit perf data.
   if (!command_line_options.perf_file_path.empty() &&
@@ -1262,44 +1243,6 @@ base::Status TraceProcessorShell::Run(int argc, char** argv) {
     }
   }
 
-  if (!options.structured_query_id.empty()) {
-    // Load spec files.
-    std::vector<std::string> spec_content;
-    spec_content.reserve(options.structured_query_specs.size());
-    for (const auto& s : options.structured_query_specs) {
-      spec_content.emplace_back();
-      if (!base::ReadFile(s, &spec_content.back())) {
-        return base::ErrStatus("Unable to read structured query spec file %s",
-                               s.c_str());
-      }
-    }
-
-    // Convert to TraceSummarySpecBytes.
-    std::vector<TraceSummarySpecBytes> specs;
-    specs.reserve(options.structured_query_specs.size());
-    for (uint32_t i = 0; i < options.structured_query_specs.size(); ++i) {
-      specs.emplace_back(TraceSummarySpecBytes{
-          reinterpret_cast<const uint8_t*>(spec_content[i].data()),
-          spec_content[i].size(),
-          GuessSummarySpecFormat(options.structured_query_specs[i],
-                                 spec_content[i]),
-      });
-    }
-
-    // Execute the structured query.
-    std::string output;
-    base::Status status = summary::ExecuteStructuredQuery(
-        tp.get(), specs, options.structured_query_id, &output);
-    if (!status.ok()) {
-      // Write metatrace if needed before exiting.
-      RETURN_IF_ERROR(MaybeWriteMetatrace(tp.get(), options.metatrace_path));
-      return status;
-    }
-
-    // Print the result.
-    fprintf(stdout, "%s", output.c_str());
-  }
-
   base::TimeNanos t_query = base::GetWallTimeNs() - t_query_start;
 
   if (!options.export_file_path.empty()) {