sqlfluff icon indicating copy to clipboard operation
sqlfluff copied to clipboard

Macro to call UDF breaking L012, L029

Open davidsr2r opened this issue 2 years ago • 5 comments

Search before asking

  • [X] I searched the issues and found no similar issues.

What Happened

The following code has two exceptions raised at the REPLACE, while there is no issue if the UDF call is replaced with a regular column name:

SELECT
  *,
  `{{ expand_udf_name("exampleUDF") }}`(ColumnName1).* REPLACE (
    COALESCE(
      `{{ expand_udf_name("exampleUDF") }}`(ColumnName1).Element,
      NULL
    ) AS Element
  )
FROM {{ ref('exampleDataset') }}

The exceptions raised are:

  • L012 (implicit column naming); and
  • L029 (keywords should not be used as identifiers)

Expected Behaviour

No exception should be raised.

Observed Behaviour

Exceptions L012 and L029 were raised

How to reproduce

Run sqlfluff lint filename.sql with the above code in that file, using the below config.

Dialect

bigquery

Version

Version 0.13.1

Configuration

[sqlfluff.rules]
tab_space_size = 2
max_line_length = 200
indent_unit = space

[sqlfluff:rules:L003]  # Consistent Indentation
indent_unit = space
tab_space_size = 2

[sqlfluff:rules:L010]  # Keywords
capitalisation_policy = upper

[sqlfluff:rules:L016]  # Line length
max_line_length = 200
indent_unit = space
tab_space_size = 2

[sqlfluff:rules:L030]  # Function names
capitalisation_policy = upper

[sqlfluff:rules:L040]  # Null & Boolean Literals
capitalisation_policy = upper

[sqlfluff:rules:L042]
forbid_subquery_in = both

[sqlfluff.indentation]  
template_blocks_indent = true

[sqlfluff]
exclude_rules = L014,L032,L036,L057
dialect = bigquery
templater = jinja

[sqlfluff:templater:jinja]
apply_dbt_builtins = True

[sqlfluff:templater:jinja:macros]
full_name = {% macro full_name(fn_ref) %}{{fn_ref}}{% endmacro %}

Are you willing to work on and submit a PR to address the issue?

  • [ ] Yes I am willing to submit a PR!

Code of Conduct

davidsr2r avatar May 20 '22 00:05 davidsr2r

Note: The reason we use this UDF expander is to allow dbt-managed UDFs to be created and called in dev environments

davidsr2r avatar May 20 '22 00:05 davidsr2r

Should this:

[sqlfluff:templater:jinja:macros]
full_name = {% macro full_name(fn_ref) %}{{fn_ref}}{% endmacro %}

Actually be this?:

[sqlfluff:templater:jinja:macros]
expand_udf_name = {% macro expand_udf_name(fn_ref) %}{{fn_ref}}{% endmacro %}

I get templating errors without that.

Even then I'm still getting a parse error:

% sqlfluff lint test.sql 
== [test.sql] FAIL                                                                                                                                                    
L:   3 | P:  56 | L012 | Implicit/explicit aliasing of columns.
L:   3 | P:  56 | L029 | Keywords should not be used as identifiers.
L:   3 | P:  64 |  PRS | Line 3, Position 39: Found unparsable section: '(\n
                       | COALESCE(\n `exampleUDF`(Colum...'
All Finished 📜 🎉!

tunetheweb avatar May 20 '22 05:05 tunetheweb

Here's the expanded sql:

SELECT
  *,
  `exampleUDF`(ColumnName1).* REPLACE (
    COALESCE(
      `exampleUDF`(ColumnName1).Element,
      NULL
    ) AS Element
  )
FROM exampleDataset

To be honest I don't understand this SQL. Are you missing a comma before the replace?

SELECT
  *,
  `exampleUDF`(ColumnName1).*,
  REPLACE (
    COALESCE(
      `exampleUDF`(ColumnName1).Element,
      NULL
    ) AS Element
  )
FROM exampleDataset

That no longer produces L012 nor L029 errors. Without that comma it thinks you're running REPLACE as an implicit alias - which breaks both those rules.

tunetheweb avatar May 20 '22 05:05 tunetheweb

Based on discussion so far, this sounds like a bug in the user's macro.

barrywhart avatar Jun 05 '22 20:06 barrywhart

Actually this needs to be reopened @barrywhart, (a) the UDF mismatch is only because I changed names around in order to obfuscate our code, they do match in our code, and (b) @tunetheweb the expanded SQL should work, this is a SELECT * REPLACE, and not a string replace call, so adding that comma breaks the SQL. This is still a bug that needs to be fixed.

davidsr2r avatar Sep 01 '22 03:09 davidsr2r