addign review table data trasnformation query#69
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThe reviews SQL script is rewritten from a simple ChangesReviews data profiling and cleaning
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Ritik574-coder
left a comment
Ritik574-coder
left a comment
There was a problem hiding this comment.
Changes made
- Added
review_idvalidation and duplicate-detection workflows - Implemented
txn_idstructural validation and formatting checks - Added customer and product consistency verification against master tables
- Implemented customer and product attribute profiling workflows
- Added rating validation and rating-distribution analysis
- Implemented
rating_textprofiling and standardization checks - Added structural review-date pattern analysis
- Implemented ambiguity-aware review-date parsing and standardization logic
- Added future-date validation for malformed temporal records
- Implemented
verified_purchasecategorical normalization - Added review-channel standardization and semantic consolidation
- Implemented review-title cleaning and newline-character removal
- Added analytical frequency-distribution profiling queries
- Implemented defensive validation logic using
TRY_CONVERT() - Added final clean-review transformation query for downstream consumption
|
Failed to generate code suggestions for PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 303f89191f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| WHEN review_date LIKE '__/__/____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 103) | ||
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, SUBSTRING(review_date, 4, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 110) | ||
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 105) | ||
| ELSE TRY_CONVERT(DATE, review_date) |
There was a problem hiding this comment.
The fallback ELSE TRY_CONVERT(DATE, review_date) makes ambiguous values like 04/05/2024 depend on the session DATEFORMAT/language, so the same input can be interpreted as April 5 or May 4 across environments, which can silently corrupt downstream time-based analysis. Because none of the prior __ / __ / ____ branches handle the both-<=12 case, those rows always hit this nondeterministic path in the final clean output.
Useful? React with 👍 / 👎.
| FROM bronze.reviews as r | ||
| INNER JOIN bronze.products as p | ||
| ON p.product_id = r.product_id | ||
| WHERE p.product_id != r.product_id ; |
There was a problem hiding this comment.
This check is currently unsatisfiable because the join already enforces p.product_id = r.product_id, then the WHERE p.product_id != r.product_id predicate filters everything out. As written, it can never surface mismatched product names, so the profiling step gives a false clean signal instead of detecting inconsistencies.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
explore_database/reviews/reviews.sql (1)
466-499: 💤 Low valueConsider extracting transformation logic to reduce duplication.
The date parsing,
verified_purchase,review_channel, andreview_titletransformations are duplicated between the profiling sections and the final clean SELECT. If this script evolves into a production transformation, consider consolidating the logic into a single CTE or view to ensure consistency and reduce maintenance overhead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@explore_database/reviews/reviews.sql` around lines 466 - 499, The transformations for review_date (the long CASE expression), verified_purchase (the CASE mapping), review_channel (the CASE mapping), and review_title (the REPLACE/TRIM/dbo.TitleCase logic) are duplicated; extract them into a single reusable expression set (e.g., a CTE or view named something like normalized_reviews) that defines normalized_review_date, normalized_verified_purchase, normalized_review_channel, and normalized_review_title and then reference those columns in both profiling and final SELECTs so the logic in the CASE for review_date, the CASE for verified_purchase, the CASE for review_channel, and the review_title expression (using dbo.TitleCase) is implemented once and kept consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@explore_database/reviews/reviews.sql`:
- Around line 185-192: The WHERE clause compares p.product_id and r.product_id
but those are enforced equal by the INNER JOIN (bronze.reviews r JOIN
bronze.products p ON p.product_id = r.product_id), so the condition p.product_id
!= r.product_id always fails; change the filter to compare the product name
columns instead (e.g., r.product_name != p.product_name), and consider
normalizing NULLs/whitespace/case (COALESCE/LOWER/TRIM) on r.product_name and
p.product_name to catch semantic mismatches.
- Around line 440-501: The final SELECT currently returns all rows from the
derived table t including rows where review_date conversion failed or is in the
future; add a WHERE on the outer query referencing t.review_date to exclude
invalid/future dates (e.g., WHERE t.review_date IS NOT NULL AND t.review_date <=
GETDATE()) so only successfully parsed, non-future review_date rows are
returned.
- Line 214: The percentages expression uses integer arithmetic:
CAST(ROUND(COUNT(*)*100/SUM(COUNT(*)) OVER(), 2) as nvarchar) + '%' AS
percentages — change the calculation to force floating-point division (e.g.,
replace 100 with 100.0 or cast COUNT(*) or SUM(COUNT(*)) to FLOAT/NUMERIC) so
ROUND receives a non-integer value; apply the same fix to the other occurrences
mentioned (lines with the same percentages expression).
- Around line 283-287: The CASE that converts review_date leaves ambiguous dates
(both day and month ≤12) to the locale-dependent ELSE TRY_CONVERT(DATE,
review_date); update this to use an explicit style (e.g., TRY_CONVERT(DATE,
review_date, 101) to default to MM/DD/YYYY) or add a clear disambiguation branch
for ambiguous patterns so parsing is deterministic; locate the WHEN/ELSE block
handling the review_date conversions (the CASE using TRY_CONVERT on review_date
and the four existing WHEN lines) and replace the ELSE branch with an explicit
TRY_CONVERT using the chosen style or add an extra WHEN to handle ambiguous
"__/__/____" and "__-__-____" cases before the ELSE.
---
Nitpick comments:
In `@explore_database/reviews/reviews.sql`:
- Around line 466-499: The transformations for review_date (the long CASE
expression), verified_purchase (the CASE mapping), review_channel (the CASE
mapping), and review_title (the REPLACE/TRIM/dbo.TitleCase logic) are
duplicated; extract them into a single reusable expression set (e.g., a CTE or
view named something like normalized_reviews) that defines
normalized_review_date, normalized_verified_purchase, normalized_review_channel,
and normalized_review_title and then reference those columns in both profiling
and final SELECTs so the logic in the CASE for review_date, the CASE for
verified_purchase, the CASE for review_channel, and the review_title expression
(using dbo.TitleCase) is implemented once and kept consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6a4d619f-cf6e-4cc8-8f5b-9bbffcf6c795
📒 Files selected for processing (1)
explore_database/reviews/reviews.sql
| -- Product Name Consistency Check | ||
| SELECT | ||
| r.product_name, | ||
| p.product_name | ||
| FROM bronze.reviews as r | ||
| INNER JOIN bronze.products as p | ||
| ON p.product_id = r.product_id | ||
| WHERE p.product_id != r.product_id ; |
There was a problem hiding this comment.
Logic error: WHERE clause always returns zero rows.
The INNER JOIN guarantees p.product_id = r.product_id, so the WHERE condition p.product_id != r.product_id can never be true. This query will never return any results.
Based on the analogous customer name consistency check (lines 100-107), the intent is likely to compare product names.
🐛 Proposed fix
-- Product Name Consistency Check
SELECT
r.product_name,
p.product_name
FROM bronze.reviews as r
INNER JOIN bronze.products as p
ON p.product_id = r.product_id
-WHERE p.product_id != r.product_id ;
+WHERE p.product_name != r.product_name ;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| -- Product Name Consistency Check | |
| SELECT | |
| r.product_name, | |
| p.product_name | |
| FROM bronze.reviews as r | |
| INNER JOIN bronze.products as p | |
| ON p.product_id = r.product_id | |
| WHERE p.product_id != r.product_id ; | |
| -- Product Name Consistency Check | |
| SELECT | |
| r.product_name, | |
| p.product_name | |
| FROM bronze.reviews as r | |
| INNER JOIN bronze.products as p | |
| ON p.product_id = r.product_id | |
| WHERE p.product_name != r.product_name ; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@explore_database/reviews/reviews.sql` around lines 185 - 192, The WHERE
clause compares p.product_id and r.product_id but those are enforced equal by
the INNER JOIN (bronze.reviews r JOIN bronze.products p ON p.product_id =
r.product_id), so the condition p.product_id != r.product_id always fails;
change the filter to compare the product name columns instead (e.g.,
r.product_name != p.product_name), and consider normalizing
NULLs/whitespace/case (COALESCE/LOWER/TRIM) on r.product_name and p.product_name
to catch semantic mismatches.
| SELECT | ||
| rating, | ||
| COUNT(*) as rating_count , | ||
| CAST(ROUND(COUNT(*)*100/SUM(COUNT(*)) OVER(), 2) as nvarchar) + '%' AS percentages |
There was a problem hiding this comment.
Integer division truncates percentage values.
COUNT(*)*100/SUM(COUNT(*)) performs integer division, truncating decimal places before ROUND is applied. This affects lines 214, 241, 317, 336, 376, 398, and 424.
Line 268 correctly uses 100.0 for floating-point division.
🔧 Proposed fix (apply to all affected lines)
- CAST(ROUND(COUNT(*)*100/SUM(COUNT(*)) OVER(), 2) as nvarchar) + '%' AS percentages
+ CAST(ROUND(COUNT(*)*100.0/SUM(COUNT(*)) OVER(), 2) as nvarchar) + '%' AS percentages 📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| CAST(ROUND(COUNT(*)*100/SUM(COUNT(*)) OVER(), 2) as nvarchar) + '%' AS percentages | |
| CAST(ROUND(COUNT(*)*100.0/SUM(COUNT(*)) OVER(), 2) as nvarchar) + '%' AS percentages |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@explore_database/reviews/reviews.sql` at line 214, The percentages expression
uses integer arithmetic: CAST(ROUND(COUNT(*)*100/SUM(COUNT(*)) OVER(), 2) as
nvarchar) + '%' AS percentages — change the calculation to force floating-point
division (e.g., replace 100 with 100.0 or cast COUNT(*) or SUM(COUNT(*)) to
FLOAT/NUMERIC) so ROUND receives a non-integer value; apply the same fix to the
other occurrences mentioned (lines with the same percentages expression).
| WHEN review_date LIKE '__/__/____' AND TRY_CONVERT(INT, SUBSTRING(review_date, 4, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 101) | ||
| WHEN review_date LIKE '__/__/____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 103) | ||
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, SUBSTRING(review_date, 4, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 110) | ||
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 105) | ||
| ELSE TRY_CONVERT(DATE, review_date) |
There was a problem hiding this comment.
Ambiguous dates fall through to locale-dependent parsing.
When both segments are ≤12 (e.g., 05/06/2024), neither condition matches and the ELSE branch uses default TRY_CONVERT, which is locale-dependent. This could produce inconsistent results across environments or sessions with different language settings.
Consider documenting the assumed default format or adding explicit handling (e.g., defaulting to MM/DD/YYYY with style 101).
🔧 Suggested fix - explicit default style
WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 105)
- ELSE TRY_CONVERT(DATE, review_date)
+ -- Ambiguous dates default to MM/DD/YYYY (US format)
+ WHEN review_date LIKE '__/__/____' THEN TRY_CONVERT(DATE, review_date, 101)
+ WHEN review_date LIKE '__-__-____' THEN TRY_CONVERT(DATE, review_date, 110)
+ ELSE TRY_CONVERT(DATE, review_date)
END as review_date📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| WHEN review_date LIKE '__/__/____' AND TRY_CONVERT(INT, SUBSTRING(review_date, 4, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 101) | |
| WHEN review_date LIKE '__/__/____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 103) | |
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, SUBSTRING(review_date, 4, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 110) | |
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 105) | |
| ELSE TRY_CONVERT(DATE, review_date) | |
| WHEN review_date LIKE '__/__/____' AND TRY_CONVERT(INT, SUBSTRING(review_date, 4, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 101) | |
| WHEN review_date LIKE '__/__/____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 103) | |
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, SUBSTRING(review_date, 4, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 110) | |
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 105) | |
| -- Ambiguous dates default to MM/DD/YYYY (US format) | |
| WHEN review_date LIKE '__/__/____' THEN TRY_CONVERT(DATE, review_date, 101) | |
| WHEN review_date LIKE '__-__-____' THEN TRY_CONVERT(DATE, review_date, 110) | |
| ELSE TRY_CONVERT(DATE, review_date) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@explore_database/reviews/reviews.sql` around lines 283 - 287, The CASE that
converts review_date leaves ambiguous dates (both day and month ≤12) to the
locale-dependent ELSE TRY_CONVERT(DATE, review_date); update this to use an
explicit style (e.g., TRY_CONVERT(DATE, review_date, 101) to default to
MM/DD/YYYY) or add a clear disambiguation branch for ambiguous patterns so
parsing is deterministic; locate the WHEN/ELSE block handling the review_date
conversions (the CASE using TRY_CONVERT on review_date and the four existing
WHEN lines) and replace the ELSE branch with an explicit TRY_CONVERT using the
chosen style or add an extra WHEN to handle ambiguous "__/__/____" and
"__-__-____" cases before the ELSE.
| SELECT | ||
| review_id | ||
| ,txn_id | ||
| ,customer_id | ||
| ,customer_name | ||
| ,product_id | ||
| ,product_name | ||
| ,rating | ||
| ,rating_text | ||
| ,review_date | ||
| ,verified_purchase | ||
| ,helpful_votes | ||
| ,review_channel | ||
| ,review_title | ||
| FROM | ||
| ( | ||
| SELECT | ||
| review_id | ||
| ,txn_id | ||
| ,customer_id | ||
| ,customer_name | ||
| ,product_id | ||
| ,product_name | ||
| ,rating | ||
| ,rating_text | ||
|
|
||
| ,CASE | ||
| WHEN review_date LIKE '[A-Z][a-z][a-z][a-z] __, ____' THEN TRY_CONVERT(DATE ,review_date) | ||
| WHEN review_date LIKE '[A-Z][a-z][a-z] __, ____' THEN TRY_CONVERT(DATE ,review_date) | ||
| WHEN review_date LIKE '____/__/__' THEN TRY_CONVERT(DATE ,review_date) | ||
| WHEN review_date LIKE '____-__-__' THEN TRY_CONVERT(DATE ,review_date) | ||
|
|
||
| WHEN review_date LIKE '__/__/____' AND TRY_CONVERT(INT, SUBSTRING(review_date, 4, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 101) | ||
| WHEN review_date LIKE '__/__/____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 103) | ||
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, SUBSTRING(review_date, 4, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 110) | ||
| WHEN review_date LIKE '__-__-____' AND TRY_CONVERT(INT, LEFT(review_date, 2)) > 12 THEN TRY_CONVERT(DATE, review_date, 105) | ||
| ELSE TRY_CONVERT(DATE, review_date) | ||
| END as review_date | ||
|
|
||
| ,CASE | ||
| WHEN TRIM(LOWER(verified_purchase)) IN ('1', 'y', 'yes', 'true', 'verified') THEN 'Verified' | ||
| WHEN TRIM(LOWER(verified_purchase)) IN ('0', 'n', 'no', 'false') THEN 'Not Verified' | ||
| ELSE 'Unknown' | ||
| END AS verified_purchase | ||
|
|
||
| ,[helpful_votes] | ||
|
|
||
| ,CASE | ||
| WHEN TRIM(LOWER(review_channel)) IN ('app', 'mobile app', 'mobile') THEN 'Mobile App' | ||
| WHEN TRIM(LOWER(review_channel)) IN ('in store', 'in-store', 'store') THEN 'In Store' | ||
| WHEN TRIM(LOWER(review_channel)) IN ('online', 'web') THEN 'Online' | ||
| WHEN TRIM(LOWER(review_channel)) = 'phone' THEN 'Phone Call' | ||
| WHEN TRIM(LOWER(review_channel)) = 'catalog' THEN 'Catalog' | ||
| ELSE 'Unknown' | ||
| END AS review_channel | ||
|
|
||
| ,CASE | ||
| WHEN REPLACE(REPLACE(TRIM(dbo.TitleCase(review_title)), CHAR(13), ''), CHAR(10), '') = '' THEN 'Unknown' | ||
| ELSE REPLACE(REPLACE(TRIM(dbo.TitleCase(review_title)), CHAR(13), ''), CHAR(10), '') | ||
| END as review_title | ||
| FROM [bronze].[reviews] | ||
| )t ; |
There was a problem hiding this comment.
Final clean output does not filter out invalid/future dates.
The profiling section (lines 291-295) identifies records with NULL or future review_date, but the final "clean data" SELECT includes all rows without filtering. This means malformed records will appear in the output.
Add a WHERE clause to exclude records that failed date conversion or have future dates.
🔧 Proposed fix
,CASE
WHEN REPLACE(REPLACE(TRIM(dbo.TitleCase(review_title)), CHAR(13), ''), CHAR(10), '') = '' THEN 'Unknown'
ELSE REPLACE(REPLACE(TRIM(dbo.TitleCase(review_title)), CHAR(13), ''), CHAR(10), '')
END as review_title
FROM [bronze].[reviews]
-)t ;
+)t
+WHERE review_date IS NOT NULL
+ AND review_date <= CAST(GETDATE() AS DATE) ;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@explore_database/reviews/reviews.sql` around lines 440 - 501, The final
SELECT currently returns all rows from the derived table t including rows where
review_date conversion failed or is in the future; add a WHERE on the outer
query referencing t.review_date to exclude invalid/future dates (e.g., WHERE
t.review_date IS NOT NULL AND t.review_date <= GETDATE()) so only successfully
parsed, non-future review_date rows are returned.
1 participant
Summary
Implemented comprehensive profiling, validation, cleaning, and standardization workflows for the
bronze.reviewsdataset to improve analytical consistency, detect malformed operational records, normalize heterogeneous categorical values, and establish a defensible review-data transformation pipeline.The implementation focuses on:
Changes made
review_idvalidation and duplicate-detection workflowstxn_idstructural validation and formatting checksrating_textprofiling and standardization checksverified_purchasecategorical normalizationTRY_CONVERT()Validation
Related issues
N/A
Notes for reviewers
The transformation workflow intentionally prioritizes defensive parsing, deterministic standardization, malformed-record isolation, and ambiguity-aware handling to reduce silent data corruption risk in downstream analytical systems.
Special attention was given to:
The current implementation follows a profiling-first transformation approach to preserve auditability and improve downstream analytical transparency.
Summary by CodeRabbit