Skip to content

bench: yard_export — memo alternatives priced and timed - #1

Merged
Aly700 merged 4 commits into
mainfrom
yard-export
Sep 1, 2026
Merged

bench: yard_export — memo alternatives priced and timed#1
Aly700 merged 4 commits into
mainfrom
yard-export

Conversation

@Aly700

@Aly700 Aly700 commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Adds a standalone yard_export benchmark target that, for four SQL workings, enumerates every memo alternative, prices each with the cost model, and times each through vectorized execution, writing docs/yard/yard-routes.json and a "Yard Routes" section in docs/benchmarks.md.

  • benchmarks/bench_common.hpp shares the table generators, checksum signature, and timing loop between sql_bench and yard_export; every existing sql_bench checksum is unchanged.
  • New e2e_class table and three_way_route workload.
  • Alternative equality is a row-sorted bag checksum, because an unordered query lets equivalent plans emit rows in different deterministic orders; the chosen plan keeps the ordered checksum.
  • Not a CTest test; CI unchanged.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-01T23:20:44.982074Z a885ffc PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a885ffc401

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +261 to +262
for (const auto& alt : report.alternatives) {
if (!(alt.bag_signature == report.alternatives[report.chosen].bag_signature)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve order when validating ordered alternatives

For the hump-and-bowl and three-trains workloads, which contain ORDER BY ... LIMIT, this comparison validates only the sorted bag checksum. A non-chosen alternative that returns the correct rows in the wrong order therefore passes and is reported as equivalent, even though its SQL result is incorrect. Use the ordered signature when the bound plan's order_permission is deterministic, reserving the bag comparison for genuinely unordered workloads.

Useful? React with 👍 / 👎.

Comment on lines +406 to +407
std::ofstream out(path);
if (!out) throw std::runtime_error("cannot write " + path);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid truncating the benchmark history on export

When --markdown is set to the repository's advertised docs/benchmarks.md destination to refresh the Yard Routes section, this default-mode ofstream truncates the entire existing benchmark document before writing only the new section. Since there is no append or section-replacement logic elsewhere, a normal refresh destroys all preceding benchmark results; write a fragment explicitly or replace only the existing Yard Routes section.

Useful? React with 👍 / 👎.

@Aly700
Aly700 merged commit 478938c into main Sep 1, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant