Skip to content

Medallion Demo SQL Formatting - #6768

Merged
anandbraman merged 1 commit into
mainfrom
medallion-demo-comment
Jul 30, 2026
Merged

Medallion Demo SQL Formatting#6768
anandbraman merged 1 commit into
mainfrom
medallion-demo-comment

Conversation

@anandbraman

Copy link
Copy Markdown
Contributor

Formatted medallion demo sql and removed a lengthy but unncessary comment. Added .env to gitignore

Describe Manual Test Plan

Checklist

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

Breaking Changes?

Mark if you think the answer is yes for any of these components:

Describe Incompatible Changes

@anandbraman
anandbraman requested review from mihaibudiu and ryzhyk July 30, 2026 15:04
Formatted medallion demo sql and removed a lengthy but unncessary comment. Added .env to gitignore

Signed-off-by: Anand Raman <anand.raman@feldera.com>
@anandbraman
anandbraman force-pushed the medallion-demo-comment branch from c40a9e3 to 5ded806 Compare July 30, 2026 15:06
@anandbraman
anandbraman enabled auto-merge July 30, 2026 15:06
@anandbraman
anandbraman added this pull request to the merge queue Jul 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 30, 2026
@mihaibudiu
mihaibudiu added this pull request to the merge queue Jul 30, 2026

@mythical-fred mythical-fred 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.

LGTM.

One soft nit, take it or leave it: the removed comment in sql/01-medallion-architecture.sql around avg_daily_units was doing real work — it explained why the denominator is COUNT(DISTINCT DATE_TRUNC(order_created_at, DAY)) rather than a hard-coded 30, and why the more obvious WHERE order_created_at > NOW() - INTERVAL '30' DAY alternative was rejected for this demo. That reasoning is not recoverable from the SQL itself. If it feels too verbose inline, a two-line "// per-day rate over the observed span, not a fixed 30-day window" version keeps the intent without the wall of text. Non-blocking.

@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 30, 2026
@anandbraman
anandbraman added this pull request to the merge queue Jul 30, 2026
Merged via the queue into main with commit cf6aa82 Jul 30, 2026
1 check passed
@anandbraman
anandbraman deleted the medallion-demo-comment branch July 30, 2026 17:14
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.

3 participants