Skip to content

docs(subscription): document multi-row cursor fetch and non-blocking short-read behavior#1071

Open
Copilot wants to merge 3 commits intomainfrom
copilot/docs-update-multi-row-cursor-fetch
Open

docs(subscription): document multi-row cursor fetch and non-blocking short-read behavior#1071
Copilot wants to merge 3 commits intomainfrom
copilot/docs-update-multi-row-cursor-fetch

Conversation

Copy link
Contributor

Copilot AI commented Mar 19, 2026

Description

The subscription cursor docs only showed FETCH NEXT and vaguely referenced "N rows" in the blocking section without a matching SQL example — leaving users unsure whether FETCH n FROM <cursor> is valid syntax.

Non-blocking fetch section:

  • Introduces FETCH n FROM cursor_name; alongside FETCH NEXT
  • Notes that FETCH NEXTFETCH 1
  • Explicitly states short-read semantics: returns however many rows are available (up to n), or empty immediately if none
  • Adds a concrete FETCH 10 FROM cur; example with sample output

Blocking fetch section:

  • Adds FETCH n FROM cursor_name WITH (timeout = '1s'); to align syntax with prose
  • Replaces ambiguous N with backtick-wrapped n consistent with the syntax block

Examples section:

  • Shows FETCH 10 FROM cur; retrieving both UpdateDelete + UpdateInsert rows from a single UPDATE in one call
  • Retains FETCH NEXT variant as an alternative
-- Non-blocking: returns up to 10 rows immediately (or empty if none available)
FETCH 10 FROM cur;

-- Blocking: waits up to 1s for up to 10 rows
FETCH 10 FROM cur WITH (timeout = '1s');

Related code PR

N/A

Related doc issue

https://github.com/risingwavelabs/risingwave-docs/issues/2239

Checklist

  • I have run the documentation build locally to verify the updates are applied correctly.
  • For new pages, I have updated mint.json to include the page in the table of contents.
  • All links and references have been checked and are not broken.
Original prompt

This section details on the original issue you should resolve

<issue_title>docs(subscription): document multi-row cursor fetch and non-blocking short-read behavior</issue_title>
<issue_description>## Context

Problem

The subscription docs are ambiguous/inconsistent about multi-row fetch syntax:

  • The page only shows FETCH NEXT FROM cursor_name; and says to fetch multiple rows by repeatedly calling FETCH NEXT.
  • The blocking section says "Fetch up to N rows" but does not show the corresponding SQL syntax using n.
  • This leads to confusion about whether FETCH n FROM <cursor> is supported, even though runtime behavior supports it.

Suggested Fix

  1. In serve/subscription.mdx, explicitly document multi-row syntax:
    • FETCH n FROM cursor_name;
  2. Add a concrete example (for example FETCH 10 FROM cur;) in the fetch section.
  3. Clarify non-blocking semantics for FETCH n:
    • If fewer than n rows are available, return available rows immediately.
    • If no rows are available, return an empty result immediately.
  4. Clarify blocking semantics with timeout + row count in matching syntax examples, so wording and SQL examples are aligned.
  5. (Optional) Add a brief note that FETCH NEXT is equivalent to fetching one row.
    </issue_description>

Comments on the Issue (you are @copilot in this section)


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

…short-read behavior

Co-authored-by: kwannoel <47273164+kwannoel@users.noreply.github.com>
Copilot AI changed the title [WIP] Update subscription documentation for multi-row cursor fetch docs(subscription): document multi-row cursor fetch and non-blocking short-read behavior Mar 19, 2026
Copilot AI requested a review from kwannoel March 19, 2026 21:59
@cyberchen98 cyberchen98 requested a review from hzxa21 March 24, 2026 17:14
update t1 set v3 = 10 where v1 = 1;

-- Fetch both change rows in one call
FETCH 10 FROM cur;
Copy link
Contributor

@cyberchen98 cyberchen98 Mar 24, 2026

Choose a reason for hiding this comment

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

Verified with v2.7.2, can you help confirm the use case @hzxa21

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. @xxhZs can double confirm.

@lmatz
Copy link
Collaborator

lmatz commented Mar 25, 2026

Docs PR Audit Report — PR #1071

Title: docs(subscription): document multi-row cursor fetch and non-blocking short-read behavior
Author: copilot-swe-agent
Risk Level: 🟢 Low

Findings

  • ✅ [Accuracy] FETCH n FROM cursor_name syntax is valid

    • Claim: The PR documents FETCH n FROM cursor_name; as valid syntax alongside FETCH NEXT FROM cursor_name;
    • Evidence: Confirmed in RisingWave source at src/sqlparser/src/ast/statement.rs lines 746-762. The parser accepts either the NEXT keyword (mapped to count=1) or a literal_u32 for the count, followed by FROM. The count field is passed through to cursor_manager.get_rows_with_cursor() in src/frontend/src/handler/fetch_cursor.rs.
    • Verdict: OK
  • ✅ [Accuracy] FETCH NEXT is equivalent to FETCH 1

    • Claim: The PR states "FETCH NEXT is equivalent to FETCH 1."
    • Evidence: Source code at line 748-749 shows if p.parse_keyword(Keyword::NEXT) { 1 }, and the Display impl at line 768 shows if self.count == 1 { "NEXT " }. The parser maps NEXT to count=1 directly.
    • Verdict: OK
  • ✅ [Accuracy] Non-blocking short-read semantics description

    • Claim: "Both forms are non-blocking: if fewer than n rows are available, only the available rows are returned immediately. If no rows are available, an empty result is returned immediately."
    • Evidence: This matches the existing documentation's blocking-section description ("whatever has been read up to that point will be returned") and is consistent with the behavior described in issue docs(subscription): document multi-row cursor fetch and non-blocking short-read behavior #1070, where a maintainer validated with psql that FETCH 10 returns available rows immediately.
    • Verdict: OK
  • ✅ [Accuracy] Blocking fetch with count syntax

    • Claim: FETCH n FROM cursor_name WITH (timeout = '1s');
    • Evidence: The parser accepts WithProperties after the cursor name (line 755), and the handler processes the timeout option (fetch_cursor.rs). The count is passed as stmt.count regardless of whether timeout is specified. The syntax is correct.
    • Verdict: OK
  • ✅ [Internal Consistency] Examples are consistent with described semantics

    • Claim: The FETCH 10 FROM cur; example returns 2 rows (UpdateDelete + UpdateInsert) from a single UPDATE.
    • Evidence: This correctly demonstrates the short-read behavior (asked for 10, got 2) and is consistent with the existing documentation that a single UPDATE produces two rows. The timestamps match the existing examples (1715669376304).
    • Verdict: OK
  • ⚠️ [Cross-page Consistency] Example output column naming differs from existing example

    • Claim: The new FETCH 10 FROM cur; example in the non-blocking section uses prefixed column names (t1.v1, t1.v2, t1.v3, t1.op, rw_timestamp).
    • Evidence: The existing snapshot fetch example earlier on the same page (line ~180 in the original file) shows unprefixed columns: v1 | v2 | v3 | op | rw_timestamp. This is likely due to different cursor declaration contexts (FULL vs incremental), but the PR does not explain the difference. Both the existing and new examples in the "Examples" section at the bottom of the page already use the t1. prefix, so this is consistent within that section. The inconsistency is pre-existing.
    • Verdict: Needs human verification — the pre-existing column naming inconsistency between snapshot and incremental results may warrant a separate documentation clarification, but it is not introduced by this PR.
  • ⚠️ [Completeness] PR body references non-existent issue #2239

  • ✅ [Terminology] Consistent use of lowercase n for row count parameter

    • Claim: The PR replaces uppercase N with backtick-wrapped lowercase n throughout.
    • Evidence: The original docs used uppercase N in prose but had no matching syntax example. The PR normalizes to n matching the SQL syntax blocks. This is a clear improvement.
    • Verdict: OK
  • ✅ [Completeness] Checklist items unchecked

    • Claim: The PR checklist shows all items unchecked (local build, mint.json, links).
    • Evidence: This is a DRAFT PR, so unchecked items are expected. No new pages are added, so mint.json update is not needed. No new external links are introduced.
    • Verdict: OK — checklist should be completed before merging, but acceptable for draft state.

Verdict

Recommendation: ✅ OK to merge (after draft finalization)
Reason: All technical claims are accurate and verified against the RisingWave SQL parser source code. The FETCH n FROM cursor_name syntax, FETCH NEXT equivalence to FETCH 1, short-read semantics, and blocking timeout behavior are all correctly documented. The only issue is the dead issue link (#2239) in the PR description body, which does not affect shipped documentation. The changes are a clear improvement over the existing docs, which were ambiguous about multi-row fetch support. The PR should complete its checklist (local build verification) before leaving draft state.


Audited by @desginer-and-reviewer using the docs review process

@hzxa21
Copy link
Contributor

hzxa21 commented Mar 25, 2026

The doc audit report is amazing. Is it a automated process?

@lmatz lmatz marked this pull request as ready for review March 25, 2026 17:17
Copilot AI review requested due to automatic review settings March 25, 2026 17:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the subscription cursor documentation to explicitly describe multi-row FETCH n behavior and to better align the blocking/non-blocking semantics with concrete SQL examples, addressing ambiguity raised in issue #2239.

Changes:

  • Documented FETCH n FROM <cursor> (and clarified FETCH NEXT vs multi-row fetch) in the non-blocking section.
  • Added a concrete multi-row fetch example with sample output to demonstrate short-read semantics.
  • Updated blocking fetch documentation to include FETCH n ... WITH (timeout = ...) syntax and consistent n wording.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Golem <44337247+cyberchen98@users.noreply.github.com>
@lmatz
Copy link
Collaborator

lmatz commented Mar 26, 2026

Cross-review (Round 2) by @implementor:

I re-reviewed this PR independently (after @desginer-and-reviewer) and checked current diff scope + consistency with related docs/code context.

  • Decision: Direct merge OK
  • Why: Doc change is scoped and consistent with current subscription behavior description; no risky cross-page semantic change found.

If useful, I can help prepare a consolidation patch for overlapping PR groups to reduce maintainer load.

@lmatz
Copy link
Collaborator

lmatz commented Mar 26, 2026

The doc audit report is amazing. Is it a automated process?

Yes, I will make the prompt into agents.md

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.

docs(subscription): document multi-row cursor fetch and non-blocking short-read behavior

6 participants