Repository: andialbrecht/sqlparse

Issue:
/*! MySQL-specific code */ is not a comment
Hi!
According to docs http://dev.mysql.com/doc/refman/5.7/en/comments.html for example `/*!40101 SET NAMES utf8 */;` or `/*! SET NAMES utf8 */;` are an conditional-executed pieces of code. I think this should not be truncated by `sqlparse.format(sql, strip_comments=True)`.
What do You think?

Discussion:
@vmuriart: They all seem to start with `/*!` correct? I made a similar note (in an upstream branch) with respect to Oracle's `hint comments` but didn't press the issue. I think they **shouldn't** be stripped out. 
In `mysql` do they have a particular location they go? in `oracle` they usually are next to the `select` keyword.

@andialbrecht any concerns in keeping them? should it be a secondary flag to strip out "special" comments?
@kamarkiewicz: I'm not an expert, but it seem not to be resolved in strings when I perform sql INSERT... Some example cases are in documentation that I gave a link before.
@vmuriart: not sure i understood what you meant. The example shows CREATE and SELECT examples. 
The changes I have in mind would identify the token and categorize it as a `comment.code` type regarless of what type of statement its in, or if its a valid instruction as long as it starts and ends with `/*!` `*/`
@dlenski: @vmuriart, Oracle's optimizer hints all look like `/*+ … */`. The gory details are here ([Oracle 11g](https://docs.oracle.com/cd/E11882_01/server.112/e41573/hintsref.htm#PFGRF005)).

All the examples and docs I've seen match this regex exactly, including the spaces:

``` python
re.match(r'/\*\+ .+ \*/', token.value)
```

So I think your solution of adding a non-skipped, non-stripped `Token.Comment.Code` makes a lot of sense. :+1:
@vmuriart: Agreed!. I placed the [`hint` tokens](https://github.com/andialbrecht/sqlparse/blob/master/sqlparse/keywords.py#L22-L23) in an earlier commit is I'm familiar with its existance. Turns out there is a more obscure hint type; [`--+ ... \n` is valid hint syntax](https://docs.oracle.com/cd/E11882_01/server.112/e41084/sql_elements006.htm#SQLRF00219).

On a side note, [ISO-SQL92 defines](https://github.com/vmuriart/sqldef/blob/cc3d69efa1fa8b039866644b9ddd0565d5282a81/src/sql92.ebnf#L3010-L3036) another way to embed code (from other languages) within SQL. I've yet to find query that use it or database that allows it.
@vmuriart: Just remembered why I haven't closed issue. I have [upstream work](https://github.com/vmuriart/sqlparse/commit/77784c5c12bbaba96b17e53706a6e120e798cc47) that changes the behavior of the comment filter and handles it much earlier in the parsing process to speed-up the parsing grouping process.  
I was waiting for `0.2.0` release (hint: @andialbrecht :innocent: ) before considering merging that work in. 
@ahirve: Hi all,
Just checking in on this...I am on sqlparse version 0.2.3 (python 3.5.3) and am having issues stripping comments from Vertica SQL containing hints in the format /*+ <hint> */. Specifically, when hints are contained within a larger comment.

Actually upon further investigation, this may be a separate issue with parsing comments inside comments?

>> stmt = "/* insert /*+ direct */ into table1 select * from table2; */"
>> sqlparse.format(stmt, strip_comments=True, strip_whitespace=True)
'into table1 select * from table2;*/'

It looks like embedding /* <comment> */ within a larger /* <comment */ results in some unexpected behavior. This does parse fine when I run the actual statement in Vertica.

Thanks!@rafaelreuber: Any workaround for this issue?@skryzh: Hi everyone!
Same problem w/ cutting SQL-Hints from scripts but for Vertica.

We are using this library not directly, but as a part of [Airflow](https://github.com/apache/airflow): 
- sqlparse 0.5.1
- Python 3.10.10
- Airflow 2.7.3

## Examples
Code in Airflow project looks something like this:
```python
sqlparse.split(sqlparse.format(sql, strip_comments=True))
```

Code for tests:
```python
import sqlparse

sql = """..."""

if __name__ == "__main__":
    res = sqlparse.split(sqlparse.format(sql, strip_comments=True))
    for i in res:
        print(i)
```

Input:
```sql
/* multiline comment one line */

/* multiline comment
couple line */

-- single line comment standard

/* Vertica */
INSERT /*+ DIRECT */ INTO table_schema.table_name
select * from work.table1; -- select * from trst

insert /*+ DIRECT */ into schema.table_name as select * from foo;
with /*+ENABLE_WITH_CLAUSE_MATERIALIZATION*/
 prt_date as (
    select distinct partition_date as prt_d
    from work.table_name
)
select *
from prt_date;

/* Oracle hints
https://hellodba.com/Download/OracleSQLHints.pdf
*/
select --+full(u);
SELECT /*+cluster(T)*/* FROM T_EEE T where A >:1;
SELECT /*+ hash(a) full(d) */* FROM T_AAA A, T_DDD D WHERE a.c=d.c;
SELECT /*+ ORDERED INDEX (b, jl_br_balances_n1) USE_NL (j b)
           USE_NL (glcc glf) USE_MERGE (gp gsb) */
 b.application_id ,
 b.set_of_books_id ,
 b.personnel_id,
 p.vendor_id Personnel,
 p.segment1 PersonnelNumber,
 p.vendor_name Name
FROM  jl_br_journals j,  -- some comment
      jl_br_balances b,
      gl_code_combinations glcc,
      fnd_flex_values_vl glf,
      gl_periods gp,
      gl_sets_of_books gsb,
      po_vendors p;

/* MySQL comment */
# comment
#+ mysql hint
select * from foo;
```


Output w/ problem:
```sql
INSERT   INTO table_schema.table_name
select * from work.table1;
insert   into schema.table_name as select * from foo;
with
 prt_date as (
    select distinct partition_date as prt_d
    from work.table_name
)
select *
from prt_date;
select
SELECT  * FROM T_EEE T where A >:1;
SELECT  * FROM T_AAA A, T_DDD D WHERE a.c=d.c;
SELECT
 b.application_id ,
 b.set_of_books_id ,
 b.personnel_id,
 p.vendor_id Personnel,
 p.segment1 PersonnelNumber,
 p.vendor_name Name
FROM  jl_br_journals j,
      jl_br_balances b,
      gl_code_combinations glcc,
      fnd_flex_values_vl glf,
      gl_periods gp,
      gl_sets_of_books gsb,
      po_vendors p;
#+ mysql hint
select * from foo;
```

I made a fix for this problem in my fork https://github.com/skryzh/sqlparse and created PR, hope that will help (@rafaelreuber) https://github.com/andialbrecht/sqlparse/pull/792

Fixed output:
```sql
INSERT /*+ DIRECT */ INTO table_schema.table_name
select * from work.table1;
insert /*+ DIRECT */ into schema.table_name as select * from foo;
with /*+ENABLE_WITH_CLAUSE_MATERIALIZATION*/
 prt_date as (
    select distinct partition_date as prt_d
    from work.table_name
)
select *
from prt_date;
select --+full(u);
SELECT /*+cluster(T)*/* FROM T_EEE T where A >:1;
SELECT /*+ hash(a) full(d) */* FROM T_AAA A, T_DDD D WHERE a.c=d.c;
SELECT /*+ ORDERED INDEX (b, jl_br_balances_n1) USE_NL (j b)
           USE_NL (glcc glf) USE_MERGE (gp gsb) */
 b.application_id ,
 b.set_of_books_id ,
 b.personnel_id,
 p.vendor_id Personnel,
 p.segment1 PersonnelNumber,
 p.vendor_name Name
FROM  jl_br_journals j,
      jl_br_balances b,
      gl_code_combinations glcc,
      fnd_flex_values_vl glf,
      gl_periods gp,
      gl_sets_of_books gsb,
      po_vendors p;
#+ mysql hint
select * from foo;
```

I open to discussions and suggestions.@andialbrecht: Fixed in #792 

Solution patch:
diff --git a/sqlparse/filters/others.py b/sqlparse/filters/others.py
index 3388a782..6c1680ec 100644
--- a/sqlparse/filters/others.py
+++ b/sqlparse/filters/others.py
@@ -15,9 +15,9 @@ class StripCommentsFilter:
 
     @staticmethod
     def _process(tlist):
-        def get_next_comment():
+        def get_next_comment(idx=-1):
             # TODO(andi) Comment types should be unified, see related issue38
-            return tlist.token_next_by(i=sql.Comment, t=T.Comment)
+            return tlist.token_next_by(i=sql.Comment, t=T.Comment, idx=idx)
 
         def _get_insert_token(token):
             """Returns either a whitespace or the line breaks from token."""
@@ -31,15 +31,35 @@ def _get_insert_token(token):
             else:
                 return sql.Token(T.Whitespace, ' ')
 
+        sql_hints = (T.Comment.Multiline.Hint, T.Comment.Single.Hint)
         tidx, token = get_next_comment()
         while token:
+            # skipping token remove if token is a SQL-Hint. issue262
+            is_sql_hint = False
+            if token.ttype in sql_hints:
+                is_sql_hint = True
+            elif isinstance(token, sql.Comment):
+                comment_tokens = token.tokens
+                if len(comment_tokens) > 0:
+                    if comment_tokens[0].ttype in sql_hints:
+                        is_sql_hint = True
+
+            if is_sql_hint:
+                # using current index as start index to search next token for
+                # preventing infinite loop in cases when token type is a
+                # "SQL-Hint" and has to be skipped
+                tidx, token = get_next_comment(idx=tidx)
+                continue
+
             pidx, prev_ = tlist.token_prev(tidx, skip_ws=False)
             nidx, next_ = tlist.token_next(tidx, skip_ws=False)
             # Replace by whitespace if prev and next exist and if they're not
             # whitespaces. This doesn't apply if prev or next is a parenthesis.
-            if (prev_ is None or next_ is None
-                    or prev_.is_whitespace or prev_.match(T.Punctuation, '(')
-                    or next_.is_whitespace or next_.match(T.Punctuation, ')')):
+            if (
+                prev_ is None or next_ is None
+                or prev_.is_whitespace or prev_.match(T.Punctuation, '(')
+                or next_.is_whitespace or next_.match(T.Punctuation, ')')
+            ):
                 # Insert a whitespace to ensure the following SQL produces
                 # a valid SQL (see #425).
                 if prev_ is not None and not prev_.match(T.Punctuation, '('):
@@ -48,7 +68,10 @@ def _get_insert_token(token):
             else:
                 tlist.tokens[tidx] = _get_insert_token(token)
 
-            tidx, token = get_next_comment()
+            # using current index as start index to search next token for
+            # preventing infinite loop in cases when token type is a
+            # "SQL-Hint" and has to be skipped
+            tidx, token = get_next_comment(idx=tidx)
 
     def process(self, stmt):
         [self.process(sgroup) for sgroup in stmt.get_sublists()]


Test patch:
diff --git a/tests/test_format.py b/tests/test_format.py
index df94630c..4dae299d 100644
--- a/tests/test_format.py
+++ b/tests/test_format.py
@@ -106,6 +106,26 @@ def test_strip_comments_preserves_whitespace(self):
         res = sqlparse.format(sql, strip_comments=True)
         assert res == 'SELECT 1 AS foo'
 
+    def test_strip_comments_preserves_hint(self):
+        sql = 'select --+full(u)'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == sql
+        sql = '#+ hint\nselect * from foo'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == sql
+        sql = 'select --+full(u)\n--comment simple'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == 'select --+full(u)\n'
+        sql = '#+ hint\nselect * from foo\n# comment simple'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == '#+ hint\nselect * from foo\n'
+        sql = 'SELECT /*+cluster(T)*/* FROM T_EEE T where A >:1'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == sql
+        sql = 'insert /*+ DIRECT */ into sch.table_name as select * from foo'
+        res = sqlparse.format(sql, strip_comments=True)
+        assert res == sql
+
     def test_strip_ws(self):
         f = lambda sql: sqlparse.format(sql, strip_whitespace=True)
         s = 'select\n* from      foo\n\twhere  ( 1 = 2 )\n'