Skip to content

test: check sql_example tables line up - #24645

Open
advitrocks9 wants to merge 1 commit into
apache:mainfrom
advitrocks9:sql-example-table-check
Open

test: check sql_example tables line up#24645
advitrocks9 wants to merge 1 commit into
apache:mainfrom
advitrocks9:sql-example-table-check

Conversation

@advitrocks9

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

The SQL example tables live inside sql_example literals and nothing measures them. print_functions_docs copies each one onto the page verbatim, so a table that doesn't line up in the source doesn't line up on the website. config-docs-check only proves the page matches the literal, not that either is well formed. Before #24596 there were 32 of these across 24 files.

What changes are included in this PR?

A test that walks the same four registries print_functions_docs builds the pages from, finds the ASCII tables in each sql_example, and checks every line of a table is the same display width and every cell is padded as | value |. It also rejects a markdown separator row inside an arrow table, since regr's four were all the correct width and a width check alone misses them.

A failure names the function and prints the table with its widths:

first_value, table at line 3 of the example:
    49  +-----------------------------------------------+
    49  | first_value(column_name ORDER BY other_column)|    <- cell is not padded as `| value |`
    49  +-----------------------------------------------+

It sits in datafusion/core/tests/ rather than ci/scripts/ so it rides cargo test (amd64), already a required check, and can read Documentation::sql_example directly instead of scraping the source. Width comes from unicode-width, the same crate comfy-table measures with, at the version already in Cargo.lock.

Two limits. A table ends when a line stops starting with + or |, so two tables printed back to back with nothing between them would read as one. Nothing in the tree does that. And the separator rule wants every cell to be three or more of -, : and space, so a value that is literally --- would read as a rule.

Are these changes tested?

It passes on main, so the evidence is the ablation. Reverting the sweep for arrow_field, the example the issue links:

arrow_field, table at line 3 of the example:
    63  +-------------------------------------------------------------+
    63  | arrow_field(Int64(1))                                       |
    63  +-------------------------------------------------------------+
    64  | {name: lit, data_type: Int64, nullable: false, metadata: {}} |
    63  +-------------------------------------------------------------+

Against the tree before #24596 it reports all 32. Four unit cases pin the rules one at a time: a wide-character table that must pass, a short row, an unpadded cell, and a separator row.

Are there any user-facing changes?

No.

@github-actions github-actions Bot added the core Core DataFusion crate label Aug 25, 2026
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.43%. Comparing base (c699996) to head (d560305).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24645      +/-   ##
==========================================
- Coverage   81.43%   81.43%   -0.01%     
==========================================
  Files        1118     1118              
  Lines      399414   399414              
  Branches   399414   399414              
==========================================
- Hits       325281   325278       -3     
+ Misses      55142    55141       -1     
- Partials    18991    18995       +4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Core DataFusion crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants