Force common filters and group bys to be lowercase#2012
Force common filters and group bys to be lowercase#2012flanakin wants to merge 1 commit intofeatures/hubs-recsfrom
Conversation
There was a problem hiding this comment.
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
ResourceIdandResourceNameinRecommendations_transform_v1_2(). - Lowercase
x_ResourceGroupNameinRecommendations_transform_v1_2(). - Update
x_RecommendationIdprojection (now usescoalesce(...)with aResourceId-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 |
There was a problem hiding this comment.
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).
| 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 |
| x_RecommendationId = coalesce(x_RecommendationId, tolower(ResourceId)), // Force lowercase for consistent grouping/filtering | ||
| x_ResourceGroupName = tolower(x_ResourceGroupName), // Force lowercase for consistent grouping/filtering |
There was a problem hiding this comment.
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.
| ResourceId = tolower(ResourceId), // Force lowercase for consistent grouping/filtering | ||
| ResourceName = tolower(ResourceName), // Force lowercase for consistent grouping/filtering |
There was a problem hiding this comment.
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).
🛠️ 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?
🙋♀️ Do any of the following that apply?
📑 Did you update
docs/changelog.md?📖 Did you update documentation?