fix for org= always in first place #27
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MT255026
requested review from
sc250072 and
vs255034
and removed request for
vs255034
sc250072
requested changes
Jan 21, 2026
Collaborator
sc250072
left a comment
sc250072
left a comment
There was a problem hiding this comment.
Couple of review comments addressed
| if (!queryBandMap.containsKey("org")) { | ||
| queryBandMap.put("org", "teradata-internal-telem;"); | ||
| } | ||
| // ✅ Always put org FIRST |
Collaborator
There was a problem hiding this comment.
Remove ✅ symbols in comments
|
|
||
| // Check if "appname" key exists | ||
| if (queryBandMap.containsKey("appname")) { | ||
| // Check if "airflow" exists in the value of "appname" key |
Collaborator
Author
There was a problem hiding this comment.
Got this code from the airflow. Anyway, removed now.
| if (queryBandMap.containsKey("appname")) { | ||
| // Check if "airflow" exists in the value of "appname" key | ||
| String appnameValue = queryBandMap.get("appname").toLowerCase(); | ||
| if (!appnameValue.contains("fivetran")) { |
Collaborator
There was a problem hiding this comment.
contains is case sensitive. this method will provide false for Fivetran or FIVETRAN in appname. Use toLowercase to handle all scenarios
| } | ||
|
|
||
| // Reconstruct the queryBand string from the map | ||
| // ✅ Add remaining keys (if any), preserving original behavior |
Collaborator
There was a problem hiding this comment.
Remove symbols in all comments
| } | ||
|
|
||
| @Test | ||
| void orgAtEnd_shouldBeMovedToFirst() { |
Collaborator
There was a problem hiding this comment.
Add tests for null, empty, duplicate keys and special values like contains "=" cases.
| if (!appnameValue.contains("fivetran")) { | ||
| queryBandMap.put("appname", queryBandMap.get("appname") + "_fivetran;"); | ||
| // ✅ Handle appname SECOND | ||
| if (parsed.containsKey("appname")) { |
Collaborator
There was a problem hiding this comment.
containsKey is also case sensitive. It wont be true for words "APPNAME", "Appname"
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
QueryBand |
-------------------------------------------------+
=S> org=teradata-internal-telem;appname=fivetran;|
=S> org=teradata-internal-telem;appname=fivetran;|
|
=S> org=teradata-internal-telem;appname=fivetran;|
=S> org=teradata-internal-telem;appname=fivetran;|