Repository: sqlfluff/sqlfluff

Issue:
fix for L034 reorder of columns can break query
The fix for L034 reorders columns. However, when two select queries are being unioned the fix may change the result of the query and/or cause the query to break. 

The following is a case where there is no re-ordering of the columns that will work without altering the result of the query. 

## Expected Behaviour
Don't fix this. I'm not sure if L034 should apply to this situtation, but I don't think there is an automated fix. 

## Observed Behaviour
sqlfluff fixes as follows which significantly changes the query:
```
select
    4 as first_col,
    case
        when 4 = 5
            then 'x'
        else 'y'
    end as second_col
union all
select
    'l' as second_col,
    case
        when 4 = 5
            then 9
        else 10
    end as first_col
```
## Steps to Reproduce
Run sqlfluff fix on the following SQL:
```
select
   4 as first_col,
   case
       when 4 = 5
           then 'x'
       else 'y'
   end as second_col
union all
select
   case
       when 4 = 5
           then 9
       else 10
   end as first_col,
   'l' as second_col
```
## Version
sqlfluff 0.5.6




Discussion:
barrywhart: Ah, I had wondered if `SelectCrawler` might be useful here, but its main value comes when recursing over a tree of `SELECT` statements. In this case, since you're looking for a `set_expression` parent, I wonder if it'd be more appropriate just check the `parent_stack`, as some existing rules do. 
NiallRees: > Ah, I had wondered if `SelectCrawler` might be useful here, but its main value comes when recursing over a tree of `SELECT` statements. In this case, since you're looking for a `set_expression` parent, I wonder if it'd be more appropriate just check the `parent_stack`, as some existing rules do.

That's exactly what I needed!
codecov[bot]: # [Codecov](https://codecov.io/gh/sqlfluff/sqlfluff/pull/1119?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) Report
> Merging [#1119](https://codecov.io/gh/sqlfluff/sqlfluff/pull/1119?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) (558e009) into [master](https://codecov.io/gh/sqlfluff/sqlfluff/commit/f8c268126efdea196ae622e557f3bcd34a79eb6f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) (f8c2681) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/sqlfluff/sqlfluff/pull/1119/graphs/tree.svg?width=650&height=150&src=pr&token=maM8kLgCmn&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff)](https://codecov.io/gh/sqlfluff/sqlfluff/pull/1119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff)

```diff
@@           Coverage Diff           @@
##           master    #1119   +/-   ##
=======================================
  Coverage   95.96%   95.96%           
=======================================
  Files         123      123           
  Lines        8180     8180           
=======================================
  Hits         7850     7850           
  Misses        330      330           
```

| Flag | Coverage Δ | |
|---|---|---|
| dbt018-py38 | `62.16% <100.00%> (ø)` | |
| dbt019-py38 | `62.16% <100.00%> (ø)` | |
| py36 | `94.91% <100.00%> (ø)` | |
| py37 | `94.94% <100.00%> (ø)` | |
| py38 | `94.92% <100.00%> (ø)` | |
| py39 | `94.92% <100.00%> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/sqlfluff/sqlfluff/pull/1119?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff) | Coverage Δ | |
|---|---|---|
| [src/sqlfluff/rules/L034.py](https://codecov.io/gh/sqlfluff/sqlfluff/pull/1119/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff#diff-c3JjL3NxbGZsdWZmL3J1bGVzL0wwMzQucHk=) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/sqlfluff/sqlfluff/pull/1119?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/sqlfluff/sqlfluff/pull/1119?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff). Last update [f8c2681...558e009](https://codecov.io/gh/sqlfluff/sqlfluff/pull/1119?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=sqlfluff).


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 44be11fbc..07c8b0749 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 - L034 now ignores select statements which contain macros.
+- L034 now ignores select statements part of a set expression, most commonly a union.
 
 Contributors:
 - [@GitHub-Username](Link to GitHub profile) ([#PR-Number](Link to PR))
diff --git a/src/sqlfluff/rules/L034.py b/src/sqlfluff/rules/L034.py
index 41e3a3c9b..e66d3acde 100644
--- a/src/sqlfluff/rules/L034.py
+++ b/src/sqlfluff/rules/L034.py
@@ -45,7 +45,7 @@ class Rule_L034(BaseRule):
         self.current_element_band = i
         self.seen_band_elements[i].append(segment)
 
-    def _eval(self, segment, **kwargs):
+    def _eval(self, segment, parent_stack, **kwargs):
         self.violation_buff = []
         self.violation_exists = False
         # Bands of select targets in order to be enforced
@@ -64,7 +64,10 @@ class Rule_L034(BaseRule):
         # If we find a matching target element, we append the element to the corresponding index
         self.seen_band_elements = [[] for i in select_element_order_preference] + [[]]
 
-        if segment.is_type("select_clause"):
+        # Ignore select clauses which belong to a set expression, which are most commonly a union.
+        if segment.is_type("select_clause") and not parent_stack[-2].is_type(
+            "set_expression"
+        ):
             select_clause_segment = segment
             select_target_elements = segment.get_children("select_clause_element")
             if not select_target_elements:


Test patch:
diff --git a/test/fixtures/rules/std_rule_cases/L034.yml b/test/fixtures/rules/std_rule_cases/L034.yml
index d7bdfca75..3f3c30f62 100644
--- a/test/fixtures/rules/std_rule_cases/L034.yml
+++ b/test/fixtures/rules/std_rule_cases/L034.yml
@@ -89,3 +89,15 @@ test_6:
       row_number() over (partition by id order by date) as y,
       2::int + 4 as sum
     from x
+
+test_union_statements_ignored:
+  pass_str: |
+    select
+      a + b as c,
+      d
+    from table_a
+    union all
+    select
+      c,
+      d
+    from table_b