Skip to content

cypher_with: add ORDER BY to non-deterministic RETURN queries#2436

Open
jrgemignani wants to merge 1 commit into
apache:masterfrom
jrgemignani:stabilize_cypher_with_regression_tests
Open

cypher_with: add ORDER BY to non-deterministic RETURN queries#2436
jrgemignani wants to merge 1 commit into
apache:masterfrom
jrgemignani:stabilize_cypher_with_regression_tests

Conversation

@jrgemignani

Copy link
Copy Markdown
Contributor

Several cypher_with regression queries RETURN multiple rows without an ORDER BY, so their row order depends on heap/scan order and can vary between runs, build types, and platforms. Add ORDER BY ASC to those queries so the expected output is stable. Ordering keys use id() (a single int64 that bypasses the locale-sensitive string comparison path and is reproducible from the test's deterministic setup order), or the projected path/scalar where that is what the query returns. Where the underlying vertex/edge was dropped by a WITH projection, its id is threaded through as an alias rather than reordering the projection.

Full audit of cypher_with: all 23 multi-row result blocks were checked. After this change, every multi-row, non-EXPLAIN RETURN is deterministically ordered. The two remaining unordered multi-row blocks are left as-is:

  • "RETURN lbl" returns two identical "Person" rows, so order cannot drift;
  • the 13 EXPLAIN (VERBOSE, COSTS OFF) plan blocks emit a fixed serial plan (no parallel/gather nodes), so their row order is already deterministic.

This is a test-only change (regress/sql/cypher_with.sql and regress/expected/cypher_with.out); no extension C code or SQL is modified. Row counts are unchanged (pure reordering).

All 37 regression tests pass (installcheck) on PostgreSQL 18.3.

Co-authored-by: GitHub Copilot noreply@github.com

modified: regress/expected/cypher_with.out
modified: regress/sql/cypher_with.sql

Copilot AI 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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI 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.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@jrgemignani jrgemignani force-pushed the stabilize_cypher_with_regression_tests branch from 4e14eae to 9cb8621 Compare June 11, 2026 14:16
Several cypher_with regression queries RETURN multiple rows without an
ORDER BY, so their row order depends on heap/scan order and can vary
between runs, build types, and platforms. Add ORDER BY ASC to those
queries so the expected output is stable. Ordering keys use id() (a
single int64 that bypasses the locale-sensitive string comparison path
and is reproducible from the test's deterministic setup order), or the
projected path/scalar where that is what the query returns. Where the
underlying vertex/edge was dropped by a WITH projection, its id is
threaded through as an alias rather than reordering the projection.

Full audit of cypher_with: all 23 multi-row result blocks were checked.
After this change, every multi-row, non-EXPLAIN RETURN is deterministically
ordered. The two remaining unordered multi-row blocks are left as-is:
- "RETURN lbl" returns two identical "Person" rows, so order cannot drift;
- the 13 EXPLAIN (VERBOSE, COSTS OFF) plan blocks emit a fixed serial plan
  (no parallel/gather nodes), so their row order is already deterministic.

This is a test-only change (regress/sql/cypher_with.sql and
regress/expected/cypher_with.out); no extension C code or SQL is modified.
Row counts are unchanged (pure reordering).

All 37 regression tests pass (installcheck) on PostgreSQL 18.3.

Co-authored-by: GitHub Copilot <noreply@github.com>

modified:   regress/expected/cypher_with.out
modified:   regress/sql/cypher_with.sql
@jrgemignani jrgemignani force-pushed the stabilize_cypher_with_regression_tests branch from 9cb8621 to 05d5c71 Compare June 12, 2026 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants