legend-engine
legend-engine copied to clipboard
integrate concat_ws for memsql and snowflake
What type of PR is this?
- Bug Fix
What does this PR do / why is it needed ?
The current translation of joinStrings from PURE is to the CONCAT method in SQL. In some of the SQL implementations, including MemSQL and Snowflake that are addressed in this MR, the CONCAT method concatenates the variables without a delimiter, and produces null when one the values is null.
Example1: ['A', 'B']->joinStrings('|') in PURE, produces SQL CONCAT('A', 'B', '|') which evaluates to 'AB|'. Example2: ['A', null]->joinStrings('|') in PURE, produces SQL CONCAT('A', null, '|') which evaluates to NULL.
Fix: This MR maps the joinStrings for MemSQL and Snowflake to CONCAT_WS instead. CONCAT_WS concatenates the variables using a delimiter Example1: ['A', 'B']->joinStrings('|') in PURE, produces SQL CONCAT_WS('|', 'A', 'B') which evaluates to 'A|B'. Example2: ['A', 'B']->joinStrings('|') in PURE, produces SQL CONCAT_WS('|', 'A', NULL) which evaluates to 'A'.
Which issue(s) this PR fixes:
Fixes #
Other notes for reviewers:
Does this PR introduce a user-facing change?
Test Results
767 files ±0 767 suites ±0 1h 7m 8s :stopwatch: +53s 12 631 tests +3 12 398 :heavy_check_mark: +3 233 :zzz: ±0 0 :x: ±0 15 742 runs +3 15 491 :heavy_check_mark: +3 251 :zzz: ±0 0 :x: ±0
Results for commit 5a538d91. ± Comparison against base commit e55e1f10.
:recycle: This comment has been updated with latest results.
This PR is stale because it has been open for 30 days with no activity. Please remove stale label or add any comment to keep this open. Otherwise this will be closed in 5 days.
This PR is stale because it has been open for 30 days with no activity. Please remove stale label or add any comment to keep this open. Otherwise this will be closed in 5 days.
This PR is stale because it has been open for 30 days with no activity. Please remove stale label or add any comment to keep this open. Otherwise this will be closed in 5 days.
This PR was closed because it has been inactive for 35 days. Please re-open if this PR is still relevant.