Repository: sqlfluff/sqlfluff
Pull request: Adds new rule L062 to allow blocking of certain words
<!--Thanks for adding this feature!-->

<!--Please give the Pull Request a meaningful title for the release notes-->

### Brief summary of the change made
<!--Please include `fixes #XXXX` to automatically close any corresponding issue when the pull request is merged. Alternatively if not fully closed you can say `makes progress on #XXXX`.-->
Fixes #361
Fixes #1800 
Follow on from #2507

This adds a very configurable new rule to allow blocking of certain terms. To quote from the rule docstring:

    This generic rule can be useful to prevent certain keywords, functions, or objects
    from being used. Only whole words can be blocked, not phrases.

    This block list is case insensitive.

    Example use cases:

    * We prefer ``BOOL`` over ``BOOLEAN`` and there is no existing rule to enforce
      this. We can add ``BOOLEAN`` to the deny list until such a rule is written to
      cause a linting error.
    * We have deprecated a schema/table/function and want to prevent it being used
      in future. We can add that to the denylist and then add a ``-- noqa: L062`` for
      the few exceptions that still need to be in the code base.


### Are there any other side effects of this change that we should be aware of?
Can't think of any. Obviously people shouldn't block certain words (e.g. `SELECT`) but that's up to them.

### Pull Request checklist
- [x] Please confirm you have completed any of the necessary steps below.

- Included test cases to demonstrate any code changes, which may be one or more of the following:
  - `.yml` rule test cases in `test/fixtures/rules/std_rule_cases`.
  - `.sql`/`.yml` parser test cases in `test/fixtures/dialects` (note YML files can be auto generated with `tox -e generate-fixture-yml`).
  - Full autofix test cases in `test/fixtures/linter/autofix`.
  - Other.
- Added appropriate documentation for the change.
- Created GitHub issues for any relevant followup/future enhancements if appropriate.


Problem statement:
Blocklisting SQL vendor specific data types
Following Simon Holywell's SQL Style Guide, it would be nice to have a lint rule that you can config to check for SQL data types used in the query.

I.e. In BigQuery you can use the keyword `BOOL` or `BOOLEAN` to represent a boolean type, the lint rule could blacklist one of those and enforce only a single keyword.

New rule: database & schema restrictions
_Note: This is for general enhancements to the project. Please use the Bug report template instead to raise parsing/linting/syntax issues for existing supported dialects_
At some companies different databases & schemas serve different purposes. Some are secure. Others aren’t. Some are sandboxes not fit for production while others are for production. In order not to misuse data source it is a good idea to allow projects to prohibit usage of (or limit usage to) certain databases & schemas in their `.sqlfluff` file.

Hints from issue discussion:
I like this rule. I think it would be best if sqlfluff could be opinionated about *which* of the available options is preferred too. From my perspective I would find this really useful with snowflake string types where `STRING`, `TEXT`, `CHAR` and `VARCHAR` are all equivalent.

@ericxiao251 do you expect there is enough consensus for us to come up with the preferred options for the main dialects? If you're able to suggest the preferred versions for bigquery that would be a great.

As a sniff test, if you and @barrywhart can come to an agreement without too much argument for what the preferred set for bigquery is then I'm happy to roll this out in the next release.
BigQuery may be different in this respect -- it has `STRING` and `BYTES` types, but none of the other types you mention. https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#string_type

I think Mailchimp data scientists would be open to either option for Boolean columns. I do see that the officlal BigQuery [docs](https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#boolean_type) list it as `BOOL` without mentioning `BOOLEAN`, so I'd suggest going with `BOOL`.
@alanmcruickshank, my initial thoughts are no, and my answer is based on a preliminary look at the data types in:
* BigQuery [data types link](https://cloud.google.com/bigquery/docs/reference/standard-sql/data-types#integer_type)
* Snowflake [data types link](https://docs.snowflake.com/en/sql-reference/data-types-datetime.html)
* Redshift [data types link](https://docs.aws.amazon.com/redshift/latest/dg/c_Supported_data_types.html)
* Presto [data types link](https://prestodb.io/docs/current/language/types.html)
* PostgresSQL [data types link](https://www.postgresql.org/docs/9.5/datatype.html)

I think there is a big difference in naming between `numerical` and `character` types in each SQL dialect.

For example:
* For `numerical` types in BigQuery some of them are suffixed with `64` (i.e. `INT64`, `FLOAT64`).
* For `character` types in BigQuery, there is only a `STRING` type which is equivalent to a `VARCHAR` in other languages.

For `datetime` types I think we might be able to come to a consensus as they are more aligned. They all seem to support at least 2 of 4:
* `DATE`
* `DATETIME`
* `TIME`
* `TIMESTAMP`

Where some support varieties of `TIMESTAMP` such as `TIMESTAMP_ABC` where `ABC` is signifies that there is a timezone component as well. But `TIMESTAMP` is an alias for varieties. So we could make a rule to only use `TIMESTAMP` and not the variants.
@barrywhart that is a fair point, but from looking at other SQL dialects (Presto, Snowflake, Redshift) `BOOLEAN` is more commonly used, so I would argue for using `BOOLEAN` actually.


I wonder if there's much value in having recommended types across databases. Would it be reasonable to have a list per dialect?
That's what I am leaning towards as well. If we do come up with a list, it would be a very short list and I don't think there's much value in that.

This would also require some sort of CLI flag to dictate what dialect we are testing against and I am not aware that sqlfluff has that as an option currently? 

Would this also deviate from sqlfluff being a "generic sql linter", I am not sure if that was the intentions or not (still new to sqlfluff).
There are necessarily parts of the grammar which are specific to certain databases.

My mental model is that it's a generic core that supports db-specific specializations.
Oh yes -- there is a --dialect command-line option, and you can also set it in the .sqlfluff configuration file.
I'm late to this discussion, but I agree strongly with the premise that support for a single explicit preference is generally preferable to ambiguity, and it's preferrable to not have both BOOL and BOOLEAN used in the same codebase (or worse, in the same file).

I actually want to propose another possible guiding principle here, which could be:

* `Prefer standard/ANSI data types and formulas when those are functionally identical to the platform-specific data type or function.`

I've been similarly considering in my team to disallow the legacy and platform-specific `ISNULL()` and `IFNULL()` functions in preference of the ubiquitous and multi-arg `COALESCE()`. I've just reviewed code today where `IFNULL()` and `COALESCE()` are both used in the same file. My preference would be to follow shared standards and be ANSI compliant (and thereby portable to a broader range of developers), unless there's some significant reason not to do so.

Also would prefer the standard `CAST(myval as int)` over `myval::int` which is less portable and (imho) not any more readable.
For the purposes of this issue I think we should focus just on the _datatype_ issue, but follow the thinking of what @aaronsteers suggests. 

...however...

I'm trying to enumerate the actual changes we would enforce, so far I've got:
- `BOOL` -> `BOOLEAN`
- On snowflake, `REAL`, `DOUBLE`, `DOUBLE PRECISION` -> `FLOAT`
- On snowflake, `INT`, `INTEGER`, `BIGINT`, `SMALLINT`, `DECIMAL`, `NUMERIC` -> `NUMBER`
- On snowflake, `CHAR`, `STRING`, `TEXT`, `CHARACTER` -> `VARCHAR`

I realise in this case snowflake is a bit of an oddity in having so few underlying datatypes, but actually a lot of aliases.

I'm scratching my head of other examples though. Any ideas?
@iajoiner you can remove the template text to make the issues more readable. I've edited them out of your current issues and raised #1792 to avoid this issue in future.
@alanmcruickshank here's another example as discussed in https://github.com/sqlfluff/sqlfluff/issues/361#issuecomment-955778171

Commit message: Review feedback

Patch to review:
diff --git a/src/sqlfluff/core/default_config.cfg b/src/sqlfluff/core/default_config.cfg
index 2775cba87c9..576a3872d29 100644
--- a/src/sqlfluff/core/default_config.cfg
+++ b/src/sqlfluff/core/default_config.cfg
@@ -143,3 +143,7 @@ unquoted_identifiers_policy = all
 quoted_identifiers_policy = all
 allow_space_in_identifier = False
 additional_allowed_characters = ""
+
+[sqlfluff:rules:L062]
+# Blocked phrases
+blocked_words = None
diff --git a/src/sqlfluff/core/rules/config_info.py b/src/sqlfluff/core/rules/config_info.py
index dc1a33da948..5ae079e7268 100644
--- a/src/sqlfluff/core/rules/config_info.py
+++ b/src/sqlfluff/core/rules/config_info.py
@@ -137,6 +137,11 @@
             "in addition to alphanumerics (A-Z, a-z, 0-9) and underscores."
         ),
     },
+    "blocked_words": {
+        "definition": (
+            "Optional list of blocked words which should not be used in statements."
+        ),
+    },
 }
 
 
diff --git a/src/sqlfluff/rules/L062.py b/src/sqlfluff/rules/L062.py
new file mode 100644
index 00000000000..96781e91f02
--- /dev/null
+++ b/src/sqlfluff/rules/L062.py
@@ -0,0 +1,92 @@
+"""Implementation of Rule L062."""
+
+from typing import Optional
+
+from sqlfluff.core.rules.base import BaseRule, LintResult, RuleContext
+from sqlfluff.core.rules.doc_decorators import document_configuration
+
+
+@document_configuration
+class Rule_L062(BaseRule):
+    """Block a list of configurable phrases from being used.
+
+    This generic rule can be useful to prevent certain keywords, functions, or objects
+    from being used. Only whole words can be blocked, not phrases, nor parts of words.
+
+    This block list is case insensitive.
+
+    Example use cases:
+
+    * We prefer ``BOOL`` over ``BOOLEAN`` and there is no existing rule to enforce
+      this. We can add ``BOOLEAN`` to the deny list until such a rule is written to
+      cause a linting error.
+    * We have deprecated a schema/table/function and want to prevent it being used
+      in future. We can add that to the denylist and then add a ``-- noqa: L062`` for
+      the few exceptions that still need to be in the code base.
+
+    | **Anti-pattern**
+    | If the ``blocked_words`` config is set to ``deprecated_table,bool`` then the
+    | following will flag:
+
+    .. code-block:: sql
+
+        SELECT * FROM deprecated_table WHERE 1 = 1;
+        CREATE TABLE myschema.t1 (a BOOL);
+
+    | **Best practice**
+    | Do not used any blocked words
+
+    .. code-block:: sql
+
+        SELECT * FROM another_table WHERE 1 = 1;
+        CREATE TABLE myschema.t1 (a BOOLEAN);
+
+    """
+
+    config_keywords = [
+        "blocked_words",
+    ]
+
+    def _eval(self, context: RuleContext) -> Optional[LintResult]:
+        # Config type hints
+        self.blocked_words: Optional[str]
+
+        # Exit early if no block list set
+        if not self.blocked_words:
+            return None
+
+        # Get the ignore list configuration and cache it
+        try:
+            blocked_words_list = self.blocked_words_list
+        except AttributeError:
+            # First-time only, read the settings from configuration.
+            # So we can cache them for next time for speed.
+            blocked_words_list = self._init_blocked_words()
+
+        # Only look at child elements
+        # Note: we do not need to ignore comments or meta types
+        # or the like as they will not have single word raws
+        if context.segment.segments:
+            return None
+
+        if str(context.segment.raw_upper) in blocked_words_list:
+            return LintResult(
+                anchor=context.segment,
+                description=f"Use of blocked word '{context.segment.raw}'.",
+            )
+
+        return None
+
+    def _init_blocked_words(self):
+        """Called first time rule is evaluated to fetch & cache the blocked_words."""
+        # Use str() in case bools are passed which might otherwise be read as bool
+        blocked_words_config = str(getattr(self, "blocked_words"))
+        if blocked_words_config and blocked_words_config != "None":
+            self.blocked_words_list = self.split_comma_separated_string(
+                blocked_words_config.upper()
+            )
+        else:
+            self.blocked_words_list = []
+
+        blocked_words_list = self.blocked_words_list
+        return blocked_words_list
diff --git a/test/fixtures/rules/std_rule_cases/L062.yml b/test/fixtures/rules/std_rule_cases/L062.yml
new file mode 100644
index 00000000000..0a24e6996c6
--- /dev/null
+++ b/test/fixtures/rules/std_rule_cases/L062.yml
@@ -0,0 +1,91 @@
+rule: L062
+
+test_pass_default:
+  pass_str: |
+    SELECT col1 FROM deprecated_table
+
+test_fail_deny_word:
+  fail_str: |
+    SELECT col1 FROM deprecated_table
+  configs:
+    rules:
+      L062:
+        blocked_words: deprecated_table
+
+test_fail_deny_word_case_difference1:
+  fail_str: |
+    SELECT col1 FROM deprecated_table
+  configs:
+    rules:
+      L062:
+        blocked_words: Deprecated_Table
+
+test_fail_deny_word_case_difference2:
+  fail_str: |
+    SELECT col1 FROM Deprecated_Table
+  configs:
+    rules:
+      L062:
+        blocked_words: deprecated_table
+
+test_fail_multiple_deny_words1:
+  fail_str: |
+    SELECT myOldFunction(col1) FROM table1
+  configs:
+    rules:
+      L062:
+        blocked_words: deprecated_table,myoldFunction
+
+test_fail_multiple_deny_words2:
+  fail_str: |
+    SELECT col1 FROM deprecated_table
+  configs:
+    rules:
+      L062:
+        blocked_words: deprecated_table,myoldFunction
+
+test_pass_not_complete_match:
+  pass_str: |
+    SELECT col1 FROM deprecated_table1
+  configs:
+    rules:
+      L062:
+        blocked_words: deprecated_table
+
+test_pass_is_comment:
+  pass_str: |
+    -- deprecated_table
+    SELECT col1 FROM new_table
+  configs:
+    rules:
+      L062:
+        blocked_words: deprecated_table
+
+test_pass_in_comment:
+  pass_str: |
+    -- This used to use the deprecated_table
+    SELECT col1 FROM new_table
+  configs:
+    rules:
+      L062:
+        blocked_words: deprecated_table
+
+test_fail_bool:
+  fail_str: |
+    CREATE TABLE myschema.t1 (a BOOL);
+  configs:
+    core:
+      dialect: exasol
+    rules:
+      L062:
+        blocked_words: bool
+
+test_pass_bool:
+  pass_str: |
+    CREATE TABLE myschema.t1 (a BOOLEAN);
+  configs:
+    core:
+      dialect: exasol
+    rules:
+      L062:
+        blocked_words: bool


Code review comments:
diff_hunk: @@ -0,0 +1,92 @@
+"""Implementation of Rule L062."""
+
+from typing import Optional
+
+from sqlfluff.core.rules.base import BaseRule, LintResult, RuleContext
+from sqlfluff.core.rules.doc_decorators import document_configuration
+
+
+@document_configuration
+class Rule_L062(BaseRule):
+    """Block a list of configurable phrases from being used.
+
+    This generic rule can be useful to prevent certain keywords, functions, or objects
+    from being used. Only whole words can be blocked, not phrases, nor parts of words.
+
+    This block list is case insensitive.
+
+    Example use cases:
+
+    * We prefer ``BOOL`` over ``BOOLEAN`` and there is no existing rule to enforce
+      this. We can add ``BOOLEAN`` to the deny list until such a rule is written to
+      cause a linting error.
+    * We have deprecated a schema/table/function and want to prevent it being used
+      in future. We can add that to the denylist and then add a ``-- noqa: L062`` for
+      the few exceptions that still need to be in the code base.
+
+    | **Anti-pattern**
+    | If the ``blocked_words`` config is set to ``deprecated_table,bool`` then the
+    | following will flag:
+
+    .. code-block:: sql
+
+        SELECT * FROM deprecated_table WHERE 1 = 1;
+        CREATE TABLE myschema.t1 (a BOOL);
+
+    | **Best practice**
+    | Do not used any blocked words
+
+    .. code-block:: sql
+
+        SELECT * FROM another_table WHERE 1 = 1;
+        CREATE TABLE myschema.t1 (a BOOLEAN);
+
+    """
+
+    config_keywords = [
+        "blocked_words",
+    ]
+
+    def _eval(self, context: RuleContext) -> Optional[LintResult]:
+        # Config type hints
+        self.blocked_words: Optional[str]
+
+        # Exit early if no block list set
+        if not self.blocked_words:
+            return None
+
+        # Get the ignore list configuration and cache it
+        try:
+            blocked_words_list = self.blocked_words_list
+        except AttributeError:
+            # First-time only, read the settings from configuration.
+            # So we can cache them for next time for speed.
+            blocked_words_list = self._init_blocked_words()
+
+        # Only look at child elements
+        # Note: we do not need to ignore comments or meta types
+        # or the like as they will not have single word raws
+        if context.segment.segments:
+            return None
+
+        if str(context.segment.raw_upper) in blocked_words_list:
+            return LintResult(
+                anchor=context.segment,
+                description=f"Use of blocked word '{context.segment.raw}'.",
+            )
+
+        return None
+
+    def _init_blocked_words(self):
+        """Called first time rule is evaluated to fetch & cache the blocked_words."""
+        # Use str() in case bools are passed which might otherwise be read as bool
+        blocked_words_config = str(getattr(self, "blocked_words"))
+        if blocked_words_config and blocked_words_config != "None":
+            self.blocked_words_list = self.split_comma_separated_string(
+                blocked_words_config.upper()
+            )
+        else:
+            self.blocked_words_list = []
+
+        blocked_words_list = self.blocked_words_list
+        return blocked_words_list
path: src/sqlfluff/rules/L062.py
text: @user1:
Nitpick: I'd replace these two lines with the single line:
```
return self.blocked_words_list
```

@author:
Fixed. Not sure why L029 did it that way. Well I added that, but it was copied from another rule (L010) that did it like that.