Skip to content

Force common filters and group bys to be lowercase#2012

Open
flanakin wants to merge 1 commit intofeatures/hubs-recsfrom
flanakin/hubs-recs-filters
Open

Force common filters and group bys to be lowercase#2012
flanakin wants to merge 1 commit intofeatures/hubs-recsfrom
flanakin/hubs-recs-filters

Conversation

@flanakin
Copy link
Collaborator

🛠️ Description

Force common filter and group-by columns to lowercase in the IngestionSetup KQL script to ensure consistent casing for joins and comparisons across datasets.

Fixes # N/A

📋 Checklist

🔬 How did you test this change?

  • 🤏 Lint tests
  • 🤞 PS -WhatIf / az validate
  • 👍 Manually deployed + verified
  • 💪 Unit tests
  • 🙌 Integration tests

🙋‍♀️ Do any of the following that apply?

  • 🚨 This is a breaking change.
  • 🤏 The change is less than 20 lines of code.

📑 Did you update docs/changelog.md?

  • ✅ Updated changelog (required for dev PRs)
  • ➡️ Will add log in a future PR (feature branch PRs only)
  • ❎ Log not needed (small/internal change)

📖 Did you update documentation?

  • ✅ Public docs in docs (required for dev)
  • ✅ Public docs in docs-mslearn (required for dev)
  • ✅ Internal dev docs in docs-wiki (required for dev)
  • ✅ Internal dev docs in src (required for dev)
  • ➡️ Will add docs in a future PR (feature branch PRs only)
  • ❎ Docs not needed (small/internal change)

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 FinOps hubs ADX ingestion setup (FOCUS 1.2) to normalize casing for key recommendation columns so joins and comparisons behave consistently across datasets.

Changes:

  • Lowercase ResourceId and ResourceName in Recommendations_transform_v1_2().
  • Lowercase x_ResourceGroupName in Recommendations_transform_v1_2().
  • Update x_RecommendationId projection (now uses coalesce(...) with a ResourceId-based fallback).

x_RecommendationDetails,
x_RecommendationId, // TODO: Set for reservation recommendations
x_ResourceGroupName,
x_RecommendationId = coalesce(x_RecommendationId, tolower(ResourceId)), // Force lowercase for consistent grouping/filtering
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

x_RecommendationId is only lowercased when it is null/empty (because the tolower is applied to the ResourceId fallback). If x_RecommendationId is populated with mixed casing, this change won’t normalize it, which defeats the stated goal of consistent casing for joins/comparisons. Consider lowercasing x_RecommendationId itself (and then applying any fallback logic after normalization, if still needed).

Suggested change
x_RecommendationId = coalesce(x_RecommendationId, tolower(ResourceId)), // Force lowercase for consistent grouping/filtering
x_RecommendationId = tolower(coalesce(x_RecommendationId, ResourceId)), // Force lowercase for consistent grouping/filtering

Copilot uses AI. Check for mistakes.
Comment on lines +1737 to +1738
x_RecommendationId = coalesce(x_RecommendationId, tolower(ResourceId)), // Force lowercase for consistent grouping/filtering
x_ResourceGroupName = tolower(x_ResourceGroupName), // Force lowercase for consistent grouping/filtering
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

This PR is described as a casing-only normalization, but x_RecommendationId now also changes behavior by filling missing values from ResourceId via coalesce(...). If that fallback is intentional, the PR description should mention the semantic change (and why it’s safe); otherwise, consider keeping the existing null behavior and only normalize casing.

Copilot uses AI. Check for mistakes.
Comment on lines +1724 to +1725
ResourceId = tolower(ResourceId), // Force lowercase for consistent grouping/filtering
ResourceName = tolower(ResourceName), // Force lowercase for consistent grouping/filtering
Copy link

Copilot AI Feb 18, 2026

Choose a reason for hiding this comment

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

If this change is intended to be promoted/merged into dev, the repo’s documented process requires customer-facing changes to be reflected in the changelog (and applicable docs) before promotion (see docs-wiki/Release-process.md and docs-wiki/Branching-strategy.md). Please add the corresponding entry in docs-mslearn/toolkit/changelog.md as part of this PR (or before merging to dev).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Micro PR 🔬 Very small PR that should be especially easy for newcomers Needs: Review 👀 PR that is ready to be reviewed Skill: KQL KQL queries and Data Explorer integration Tool: FinOps hubs Data pipeline solution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

Comments