Repository: sqlfluff/sqlfluff
Pull request: Issue 968: Jinja templater throwing UnboundLocalError with "for" loop
Resolves #968, #630

Problem statement:
[BigQuery] Jinja for loop failing to parse
A simple Jinja loop with BigQuery fails to parse using sqlfluff 0.5.2, last working for this is version 0.3.6

## Expected Behaviour
No errors

## Observed Behaviour

```
[stijnzanders@stijn]$ sqlfluff parse test.sql
Traceback (most recent call last):
  File "/home/stijnzanders/.local/bin/sqlfluff", line 8, in <module>
    sys.exit(cli())
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/click/core.py", line 829, in __call__
    return self.main(*args, **kwargs)
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/click/core.py", line 782, in main
    rv = self.invoke(ctx)
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/click/core.py", line 1259, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/click/core.py", line 1066, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/click/core.py", line 610, in invoke
    return callback(*args, **kwargs)
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/sqlfluff/cli/commands.py", line 582, in parse
    for parsed_string in result:
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/sqlfluff/core/linter.py", line 1469, in parse_path
    yield self.parse_string(
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/sqlfluff/core/linter.py", line 874, in parse_string
    templated_file, templater_violations = self.templater.process(
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/sqlfluff/core/templaters/jinja.py", line 299, in process
    raw_sliced, sliced_file, out_str = self.slice_file(
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/sqlfluff/core/templaters/python.py", line 287, in slice_file
    sliced_file = list(
  File "/home/stijnzanders/.local/lib/python3.8/site-packages/sqlfluff/core/templaters/python.py", line 950, in _split_uniques_coalesce_rest
    "        Attempting Subsplit [post]: %s", sub_section
UnboundLocalError: local variable 'sub_section' referenced before assignment
```

## Steps to Reproduce

SQL File:
```
{% set event_columns = ['campaign', 'click_item'] %}

SELECT
    event_id
    {% for event_column in event_columns %}
    , {{ event_column }}
    {% endfor %}
FROM events
```

## Version
```
==== sqlfluff ====
sqlfluff:           0.5.2 python:             3.8.7
dialect:         bigquery verbosity:              2
templater:          jinja
```
## Configuration
```
[sqlfluff]
dialect = bigquery

[sqlfluff:indentation]
indented_joins = False

[sqlfluff:rules:L019]
comma_style = leading
```

Hints from issue discussion:
This looks like it may be the same underlying issue as #630. If so, that could be really helpful, as @alanmcruickshank mentioned he was having trouble reproducing that issue himself.

I'll take a quick look at this -- if I can reproduce it, it _may_ be easy to fix. 🤞🏽
Reproduced the error myself. Here are the local variables for the call that crashes:
```
(Pdb) pp locals()
{'cls': <class 'sqlfluff.core.templaters.jinja.JinjaTemplater'>,
 'pdb': <module 'pdb' from '/Users/bhart/.pyenv/versions/3.7.3/lib/python3.7/pdb.py'>,
 'raw_occurances': {'\n\nSELECT\n    event_id\n    ': [52],
                    '\n    ': [60, 73, 117, 142],
                    '\n    , ': [117],
                    '\nFROM events\n': [159]},
 'split_file': [IntermediateFileSlice(intermediate_type='compound', source_slice=slice(0, 52, None), templated_slice=slice(0, 0, None), slice_buffer=[RawFileSlice(raw="{% set event_columns = ['campaign', 'click_item'] %}", slice_type='block_start', source_idx=0)]),
                IntermediateFileSlice(intermediate_type='invariant', source_slice=slice(52, 78, None), templated_slice=slice(0, 26, None), slice_buffer=[RawFileSlice(raw='\n\nSELECT\n    event_id\n    ', slice_type='literal', source_idx=0)]),
                IntermediateFileSlice(intermediate_type='compound', source_slice=slice(78, 159, None), templated_slice=slice(26, 68, None), slice_buffer=[RawFileSlice(raw='{% for event_column in event_columns %}', slice_type='block_start', source_idx=78), RawFileSlice(raw='\n    , ', slice_type='literal', source_idx=117), RawFileSlice(raw='{{ event_column }}', slice_type='templated', source_idx=124), RawFileSlice(raw='\n    ', slice_type='literal', source_idx=142), RawFileSlice(raw='{% endfor %}', slice_type='block_end', source_idx=147)]),
                IntermediateFileSlice(intermediate_type='invariant', source_slice=slice(159, 172, None), templated_slice=slice(68, 81, None), slice_buffer=[RawFileSlice(raw='\nFROM events\n', slice_type='literal', source_idx=68)])],
 'templ_occurances': {'\n\nSELECT\n    event_id\n    ': [0],
                      '\n    ': [8, 21, 26, 41, 46, 63],
                      '\n    , ': [26, 46],
                      '\nFROM events\n': [68]},
 'templated_str': '\n'
                  '\n'
                  'SELECT\n'
                  '    event_id\n'
                  '    \n'
                  '    , campaign\n'
                  '    \n'
                  '    , click_item\n'
                  '    \n'
                  'FROM events\n'}
```
The function that's failing, `_split_uniques_coalesce_rest()`, is pretty complex and unfamiliar to me. I think with this easy-to-reproduce test case, it should be possible to make progress on it. @alanmcruickshank, think you'd be up for taking a look at this? My name is all over the `git blame` for the file `src/sqlfluff/core/templaters/python.py`,, but I suspect that was a DOS/Unix line ending change or something cosmetic like that -- pretty sure I haven't worked on this code in a meaningful way. 😅

Commit message: Issue 968: Jinja templater throwing UnboundLocalError with "for" loop

Patch to review:
diff --git a/src/sqlfluff/core/templaters/python.py b/src/sqlfluff/core/templaters/python.py
index 814c427e93f..b56c29ef0e8 100644
--- a/src/sqlfluff/core/templaters/python.py
+++ b/src/sqlfluff/core/templaters/python.py
@@ -544,6 +544,8 @@ def _split_uniques_coalesce_rest(
         # A buffer to capture tail segments
         tail_buffer: List[TemplatedFileSlice] = []
 
+        sub_section = None
+
         templater_logger.debug("    _split_uniques_coalesce_rest: %s", split_file)
 
         for int_file_slice in split_file:
@@ -947,7 +949,11 @@ def _split_uniques_coalesce_rest(
                     template_idx + raw_len,
                 )
 
-            if starts[1] < stops[1] and last_owu_idx is not None:
+            if (
+                starts[1] < stops[1]
+                and last_owu_idx is not None
+                and sub_section is not None
+            ):
                 # Yield the end bit
                 templater_logger.debug(
                     "        Attempting Subsplit [post]: %s", sub_section
diff --git a/test/core/templaters/jinja_test.py b/test/core/templaters/jinja_test.py
index 17aa8918481..c6ccf2cc5af 100644
--- a/test/core/templaters/jinja_test.py
+++ b/test/core/templaters/jinja_test.py
@@ -10,12 +10,24 @@
 JINJA_STRING = "SELECT * FROM {% for c in blah %}{{c}}{% if not loop.last %}, {% endif %}{% endfor %} WHERE {{condition}}\n\n"
 
 
-def test__templater_jinja():
+@pytest.mark.parametrize(
+    "instr, expected_outstr",
+    [
+        (
+            JINJA_STRING,
+            "SELECT * FROM f, o, o WHERE a < 10\n\n",
+        ),
+        (
+            "{% set event_columns = ['campaign', 'click_item'] %}\n\nSELECT\n    event_id\n    {% for event_column in event_columns %}\n    , {{ event_column }}\n    {% endfor %}\nFROM events\n",
+            "\n\nSELECT\n    event_id\n    \n    , campaign\n    \n    , click_item\n    \nFROM events\n",
+        ),
+    ],
+)
+def test__templater_jinja(instr, expected_outstr):
     """Test jinja templating and the treatment of whitespace."""
     t = JinjaTemplater(override_context=dict(blah="foo", condition="a < 10"))
-    instr = JINJA_STRING
     outstr, _ = t.process(in_str=instr, config=FluffConfig())
-    assert str(outstr) == "SELECT * FROM f, o, o WHERE a < 10\n\n"
+    assert str(outstr) == expected_outstr
 
 
 def test__templater_jinja_error_variable():
@@ -52,6 +64,10 @@ def test__templater_jinja_error_catatrophic():
     assert len(vs) > 0
 
 
+# def test__templater_jinja_fix_issue_968():
+#     """Tests a bug that was throwing an "UnboundLocalError"."""
+
+
 def assert_structure(yaml_loader, path, code_only=True):
     """Check that a parsed sql file matches the yaml file with the same name."""
     lntr = Linter()


Code review comments:
diff_hunk: @@ -947,7 +949,11 @@ def _split_uniques_coalesce_rest(
                     template_idx + raw_len,
                 )
 
-            if starts[1] < stops[1] and last_owu_idx is not None:
+            if (
+                starts[1] < stops[1]
+                and last_owu_idx is not None
+                and sub_section is not None
path: src/sqlfluff/core/templaters/python.py
text: @author:
I am not familiar with the `_split_uniques_coalesce_rest()` function, but just as a simple experiment, I made this change to the code simply to avoid the crash, and it works correctly (!!!), at least for the test case provided with #968. I did not try to reproduce the issue with the test case in #630. I am optimistically assuming this fix addresses that issue as well. It should definitely avoid the crash, at least.

@user1:
Reviewing this code now, it seems risky to be referring to `sub_section` here _at all_, which means I wonder whether this is a patch fix for something which could be more effectively fixed. I'm going to try running this code locally now the test case is in to see what results we get locally and will report back...

@author:
Agreed.