Skip to content

feat: implement CONCAT_WS function for E6 dialect#137

Merged
ranjanankur314 merged 4 commits intoe6data:mainfrom
tkaunlaky-e6:Mapping-CONCAT_SW
Aug 5, 2025
Merged

feat: implement CONCAT_WS function for E6 dialect#137
ranjanankur314 merged 4 commits intoe6data:mainfrom
tkaunlaky-e6:Mapping-CONCAT_SW

Conversation

@tkaunlaky-e6
Copy link

Summary

• Implemented CONCAT_WS function for E6 dialect with full Databricks compatibility
• Added comprehensive test coverage for all documented behaviors

Implementation Details

NULL handling: NULL separator returns NULL, NULL arguments are filtered out
Array support: Arrays are flattened and NULL elements within arrays are removed
Edge cases: Returns empty string when only separator provided or all args are NULL
Performance: Uses efficient AST manipulation instead of complex CASE statements

Test Coverage

Added tests covering all Databricks examples:
concat_ws(' ', 'Spark', 'SQL')'Spark SQL'
concat_ws('s')''
concat_ws(',', 'Spark', array('S', 'Q', NULL, 'L'), NULL)'Spark,S,Q,L'
• Multiple additional edge cases

Test Results

✅ All 857 tests pass - no regressions introduced

ranjanankur314
ranjanankur314 previously approved these changes Aug 4, 2025
Copy link

@ranjanankur314 ranjanankur314 left a comment

Choose a reason for hiding this comment

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

Looks good

@ranjanankur314 ranjanankur314 dismissed their stale review August 4, 2025 13:44

The merge-base changed after approval.

ranjanankur314
ranjanankur314 previously approved these changes Aug 4, 2025
Copy link

@ranjanankur314 ranjanankur314 left a comment

Choose a reason for hiding this comment

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

Looks good

@tkaunlaky-e6 tkaunlaky-e6 dismissed ranjanankur314’s stale review August 4, 2025 13:45

The merge-base changed after approval.

ranjanankur314
ranjanankur314 previously approved these changes Aug 4, 2025
@tkaunlaky-e6 tkaunlaky-e6 dismissed ranjanankur314’s stale review August 4, 2025 13:53

The merge-base changed after approval.

gauravdawar-e6
gauravdawar-e6 previously approved these changes Aug 4, 2025
Copy link
Collaborator

@gauravdawar-e6 gauravdawar-e6 left a comment

Choose a reason for hiding this comment

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

Looks good

shreyas2201
shreyas2201 previously approved these changes Aug 4, 2025
ranjanankur314
ranjanankur314 previously approved these changes Aug 4, 2025
aravindh-e6x
aravindh-e6x previously approved these changes Aug 4, 2025
@tkaunlaky-e6 tkaunlaky-e6 dismissed stale reviews from aravindh-e6x, ranjanankur314, shreyas2201, and gauravdawar-e6 August 4, 2025 13:59

The merge-base changed after approval.

@tkaunlaky-e6
Copy link
Author

Hi @aravindh-e6x , @ranjanankur314 ,@gauravdawar-e6
GitHub auto-dismissed your approvals because I merged the TYPEOF bug fix from PR #152.

Tanay Kulkarni added 4 commits August 4, 2025 20:37
…ONCAT_WS

- Replace hardcoded 'ARRAY' string with proper exp.Array expression node
- Use self.func('ARRAY_TO_STRING', ...) directly to avoid ARRAY_JOIN mapping
- Maintain all Databricks CONCAT_WS behaviors (NULL filtering, array flattening)
@tkaunlaky-e6
Copy link
Author

@ranjanankur314 @gauravdawar-e6 @aravindh-e6x
I rebased on latest main to clean up the commit history - same CONCAT_WS code, just without the merge commits now.
I think GitHub dismissed the approvals because of the rebase.

@ranjanankur314 ranjanankur314 merged commit 21b7285 into e6data:main Aug 5, 2025
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.

6 participants