Skip to content

fix(ENGKNOW-2803): Fix case insensitivity in calcifmissing#70

Merged
gmagnu merged 2 commits into
mainfrom
ENGKNOW-2803-calfifmissing-is-case-sensitive
Sep 9, 2025
Merged

fix(ENGKNOW-2803): Fix case insensitivity in calcifmissing#70
gmagnu merged 2 commits into
mainfrom
ENGKNOW-2803-calfifmissing-is-case-sensitive

Conversation

@gmagnu
Copy link
Copy Markdown
Contributor

@gmagnu gmagnu commented Sep 5, 2025

  • Fix case insensitivity in calcifmissing
  • Add warning if duplicate columns.

@gmagnu gmagnu changed the title fix(ENGKNOW-2803): Fix case insensitivity in calcifmissing, add warn… fix(ENGKNOW-2803): Fix case insensitivity in calcifmissing Sep 5, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Sep 5, 2025

Junit Tests - Summary

4 316 tests  +2   4 150 ✅ ±0   11m 26s ⏱️ + 1m 26s
  456 suites +1     166 💤 +2 
  456 files   +1       0 ❌ ±0 

Results for commit e9c6bb1. ± Comparison against base commit 24e9375.

This pull request skips 1 test.
gorsat.UTestSignature ‑ testSignature10Seconds

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown

@andrimar1 andrimar1 left a comment

Choose a reason for hiding this comment

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

Had one question

if (ALLOW_DUPLICATE_COLUMNS || allowDuplicates) {
column = column + "x"
colToUp = column.toUpperCase
if (!allowDuplicates) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if !allowDuplicates - just logs warning? Am I misinterpreting ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, we allowDuplicates is false but we are overwriting at a global level with ALLOW_DUPLICATE_COLUMNS we print out a warning about the duplicate labels.
This is basically the case when we are switching over, we have many commands with allowDublicates = false but we overwrite for backward compatibiltity with ALLOW_DUPLICATE_COLUMNS=true, we print out the warning for those columns that have allowDublicates = false. When all the queries are fixed we will set ALLOW_DUPLICATE_COLUMNS=false.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

okay I see this is with the prior that ALLOW_DUPLICATE_COLUMNS is set to TRUE

Copy link
Copy Markdown

@andrimar1 andrimar1 left a comment

Choose a reason for hiding this comment

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

LGTM

@gmagnu gmagnu merged commit c18b082 into main Sep 9, 2025
11 checks passed
@gmagnu gmagnu deleted the ENGKNOW-2803-calfifmissing-is-case-sensitive branch September 9, 2025 17:45
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.

2 participants