GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [python] Turn the commented-out window RANK tests into positive tests by 555aaditya · Pull Request #7678 · feldera/feldera

Conversation

@555aaditya

@555aaditya 555aaditya commented Oct 28, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Fixes #6226

test_window_agg.py had three negative tests commented out with "TODO: fix this test, it now should pass": RANK, DENSE_RANK and ROW_NUMBER over a constant ORDER BY, which the compiler used to reject outside a TopK pattern. All three are positive tests now.

  • RANK() OVER (ORDER BY NULL) and DENSE_RANK() OVER (ORDER BY 1): every row ties, so every row gets rank 1. These only compile since [SQL] Compile RANK when ORDER BY is on a constant #7694, the fix for [SQL] RANK/DENSE_RANK over a constant ORDER BY generates Rust that doesn't compile #7677.
  • ROW_NUMBER() OVER (ORDER BY NULL): which row gets which number is unspecified, so the test checks just the numbers 1, 2, 3.

Describe Manual Test Plan

Ran tests/runtime_aggtest/illarg_tests2/main.py against the pipeline-manager:0.372.0 image, which includes #7694, and it passes. To make sure each new view is actually checked, I broke the expected rows of the RANK and DENSE_RANK views one at a time. Each run failed on that view (failed view: illarg_window_rank_order_by_null, ... dense_rank_order_by_const). On the earlier run I did the same for the ROW_NUMBER view. pre-commit run on the file is clean.

Checklist

  • Unit tests added/updated
  • Integration tests added/updated
  • Documentation updated
  • Changelog updated

Breaking Changes?

  • OpenAPI / REST HTTP API / feldera-types / manager
  • Feldera SQL (Syntax, Semantics)
  • feldera-sqllib (incl. dependencies fxp, etc.)
  • Python SDK
  • fda (CLI arguments)
  • Adapters (including configuration)
  • Storage Format / Checkpoints
  • Others (specify)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@mihaibudiu
mihaibudiu marked this pull request as draft October 28, 2026 18:05
@mihaibudiu

mihaibudiu commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

Let's consider this after #7677 is fixed.
I marked it as draft until then; please request a review again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@555aaditya
[python] Turn the commented-out window RANK tests into positive tests
c42c803
test_window_agg.py kept three negative tests commented out with "TODO:
fix this test, it now should pass": RANK, DENSE_RANK and ROW_NUMBER over
a constant ORDER BY, which the compiler used to reject outside a TopK
pattern and now accepts.

Turn all three into positive tests. With a constant ORDER BY every row
ties, so RANK and DENSE_RANK give every row 1. For ROW_NUMBER, which row
gets which number is unspecified, so that test checks only the numbers.

RANK and DENSE_RANK only compile since feldera#7694 (the fix for feldera#7677).

Fixes feldera#6226

Signed-off-by: 555aaditya <[email protected]>
@555aaditya
555aaditya force-pushed the test/6226-window-rank-positive branch from 6bbb48c to c42c803 Compare October 30, 2026 22:02
@555aaditya
555aaditya marked this pull request as ready for review October 30, 2026 22:03
@555aaditya

555aaditya commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

#7677 is fixed by #7694, so all three tests are enabled now. They pass against v0.372.0. @mihaibudiu ready for another look.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mihaibudiu
mihaibudiu approved these changes Oct 30, 2026
View reviewed changes
@mihaibudiu
mihaibudiu added this pull request to the merge queue Oct 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 31, 2026

Uh oh!

There was an error while loading. Please reload this page.

@mihaibudiu
mihaibudiu added this pull request to the merge queue Oct 31, 2026
Merged via the queue into feldera:main with commit 41454c6 Oct 31, 2026
3 checks passed

Uh oh!

There was an error while loading. Please reload this page.

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

Reviewers

@mihaibudiu mihaibudiu mihaibudiu approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[python] Change negative tests for window RANK to positive tests

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@555aaditya @mihaibudiu
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.