Add ASOF JOIN documentation with syntax, examples, and matching rules#3462
Add ASOF JOIN documentation with syntax, examples, and matching rules#3462zclllyybb merged 1 commit intoapache:masterfrom
Conversation
|
/review |
There was a problem hiding this comment.
Code Review Summary — PR #3462: Add ASOF JOIN Documentation
Goal and Implementation
Goal: Add ASOF JOIN documentation for Apache Doris.
Implementation: The PR adds new documentation pages for ASOF JOIN in English (docs/query-data/asof-join.md, versioned_docs/version-4.x/query-data/asof-join.md) and Chinese (i18n/zh-CN/docusaurus-plugin-content-docs/current/query-data/asof-join.md, i18n/zh-CN/docusaurus-plugin-content-docs/version-4.x/query-data/asof-join.md), plus sidebar entries in both sidebars.ts and versioned_sidebars/version-4.x-sidebars.json.
The implementation largely satisfies the goal. The content is comprehensive, well-structured, and covers all required sections (Overview, Syntax, Parameters, Matching Rules, Examples, Equivalent Rewrite, Usage Notes, Best Practices).
Critical Checkpoint Review
1. Does the implementation satisfy the goal?
Yes. All required documentation sections for a new feature are present. The content is accurate and covers the feature thoroughly.
2. Is the change focused?
Yes. Only ASOF JOIN documentation files and sidebar entries are touched.
3. Documentation information architecture:
Partially correct. There is one structural issue (see Issue #1 below): the PR checklist marks only 4.x as targeted, but the PR also adds docs/query-data/asof-join.md and its Chinese counterpart — which correspond to the current (dev) version, not 4.x. Modifying docs/ and sidebars.ts is appropriate only when targeting the dev version. The PR either needs to also check the dev box, or remove the docs/ and i18n/.../current/ files from scope (but this is likely intentional since future dev builds should also include the feature).
4. Link, redirect, and navigation consistency:
Sidebar entries are correctly added after query-data/join in both sidebars.ts (current) and version-4.x-sidebars.json. No existing links point to this new page. No redirects are needed since this is an entirely new page. Navigation is consistent.
5. Docusaurus config, React, or styling changes:
None. Not applicable.
6. Validation:
No automated tests or builds have been run (the checklist items are unchecked). The SQL examples appear self-consistent, though there is one output formatting anomaly (see Issue #2).
7. Other correctness, usability, SEO, or maintainability issues:
See Issues #2–#4 below.
Issues Found
Issue #1 (Medium): PR checklist declares 4.x only, but docs/ (current/dev) and i18n/.../current/ are also modified
The PR body checks only 4.x under Versions, yet the change also adds:
docs/query-data/asof-join.md(current/dev version EN)i18n/zh-CN/docusaurus-plugin-content-docs/current/query-data/asof-join.md(current/dev version ZH)- Entry in
sidebars.ts(current sidebar)
These files serve the current (dev) documentation, not 4.x. This is likely intentional (the feature should also appear in the dev docs), but the checklist is inconsistent. Please either check the dev box in the PR description, or clarify whether the docs/ and current i18n/ files were added intentionally.
Additionally, the docs/query-data/asof-join.md (current/dev) and i18n/zh-CN/docusaurus-plugin-content-docs/current/query-data/asof-join.md are missing the :::info version callout that states when this feature was introduced ("This feature is supported since Apache Doris versions 4.0.5 and 4.1.0"). Users browsing the current docs would not know the feature is unavailable in older releases. The 4.x variants correctly include this callout.
Issue #2 (Minor): Inconsistent decimal formatting in example output
In the result sets of Examples 1, 2, and 3, the price column for GOOG rows shows 2800.0 and 2805.0 (one decimal place), while all other values in the same column show two decimal places (e.g., 150.50, 150.75). Since the column is defined as DECIMAL(10, 2), the expected output should be 2800.00 and 2805.00. This inconsistency may confuse users.
Affected files: docs/query-data/asof-join.md (lines 133–134, 160–161, 187–188) and all mirrored files.
Issue #3 (Minor): DATETIME used in examples but only DATEV2/DATETIMEV2/TIMESTAMPTZ documented as supported
The DDL and CTE examples use the DATETIME type (e.g., trade_time DATETIME, CAST(... AS DATETIME)), but the Syntax section, Parameters table, Key Rules section, and Usage Notes only list DATEV2, DATETIMEV2, and TIMESTAMPTZ as supported types for MATCH_CONDITION. DATETIME is an alias for DATETIMEV2 in Doris, so this works in practice, but it creates a user-facing inconsistency. Users might wonder if DATETIME is supported. Either clarify that DATETIME is an alias and is supported, or use DATETIMEV2 consistently in the examples.
Issue #4 (Minor): Equivalent Rewrite uses undefined table names and columns
The Equivalent Rewrite section uses table names left_table, right_table, column names id, rid, val, ts, grp that are not defined in this document and differ from the trades/quotes example used throughout. While this is a schematic illustration, it would be clearer to either:
- Use the same
trades/quotestables from the earlier examples, or - Add a brief note explaining that
left_table/right_tableare illustrative placeholders.
This is a minor usability issue that could cause confusion if a user tries to copy-paste the query.
Summary
The documentation is comprehensive and well-written. The main structural concern is the inconsistency between the PR's declared scope (4.x only) and the files actually modified (which include the current/dev version). The version callout (:::info) is missing from the current/dev EN and ZH files. Example output has a minor decimal formatting inaccuracy. All other content — sections, examples, Chinese/English alignment, sidebar placement, and navigation — is correct.
i18n/zh-CN/docusaurus-plugin-content-docs/current/query-data/asof-join.md
Show resolved
Hide resolved
i18n/zh-CN/docusaurus-plugin-content-docs/current/query-data/asof-join.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Adds documentation for the ASOF JOIN feature and exposes it in the Docusaurus navigation for both current and versioned (4.x) docs, including a zh-CN translation.
Changes:
- Add new ASOF JOIN documentation pages (EN current + 4.x, zh-CN current + 4.x).
- Add ASOF JOIN to the “Data Queries” section in both current and versioned sidebars.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
docs/query-data/asof-join.md |
New English “current” ASOF JOIN doc page. |
versioned_docs/version-4.x/query-data/asof-join.md |
New English ASOF JOIN doc page for version 4.x (includes version support note). |
i18n/zh-CN/docusaurus-plugin-content-docs/current/query-data/asof-join.md |
New zh-CN “current” ASOF JOIN doc page. |
i18n/zh-CN/docusaurus-plugin-content-docs/version-4.x/query-data/asof-join.md |
New zh-CN ASOF JOIN doc page for version 4.x (includes version support note). |
sidebars.ts |
Adds query-data/asof-join to current docs sidebar. |
versioned_sidebars/version-4.x-sidebars.json |
Adds query-data/asof-join to 4.x versioned docs sidebar. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
doc of apache/doris#59591
cases maintained in apache/doris#61351
Versions
Languages
Docs Checklist