Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent exceptions when running fix on dialect fixtures #2818

2 changes: 1 addition & 1 deletion src/sqlfluff/core/rules/analysis/select.py
Expand Up @@ -80,7 +80,7 @@ def get_select_statement_info(
for ref in reference_buffer.copy():
ref_path = segment.path_to(ref)
# is it in a subselect? i.e. a select which isn't this one.
if any(
if ref_path and any(
seg.is_type("select_statement") and seg is not segment for seg in ref_path
):
reference_buffer.remove(ref)
Expand Down
48 changes: 26 additions & 22 deletions src/sqlfluff/core/rules/base.py
Expand Up @@ -802,28 +802,32 @@ def _choose_anchor_segment(

anchor: BaseSegment = segment
child: BaseSegment = segment
for seg in context.parent_stack[0].path_to(segment)[1:-1][::-1]:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review this bit hiding whitespace:

image

# Which lists of children to check against.
children_lists: List[List[BaseSegment]] = []
if filter_meta:
# Optionally check against filtered (non-meta only) children.
children_lists.append(
[child for child in seg.segments if not child.is_meta]
)
# Always check against the full set of children.
children_lists.append(seg.segments)
children: List[BaseSegment]
for children in children_lists:
if edit_type == "create_before" and children[0] is child:
anchor = seg
assert anchor.raw.startswith(segment.raw)
child = seg
break
elif edit_type == "create_after" and children[-1] is child:
anchor = seg
assert anchor.raw.endswith(segment.raw)
child = seg
break
parent: BaseSegment = context.parent_stack[0]
path: Optional[List[BaseSegment]] = parent.path_to(segment) if parent else None
inner_path: Optional[List[BaseSegment]] = path[1:-1] if path else None
if inner_path:
for seg in inner_path[::-1]:
# Which lists of children to check against.
children_lists: List[List[BaseSegment]] = []
if filter_meta:
# Optionally check against filtered (non-meta only) children.
children_lists.append(
[child for child in seg.segments if not child.is_meta]
)
# Always check against the full set of children.
children_lists.append(seg.segments)
children: List[BaseSegment]
for children in children_lists:
if edit_type == "create_before" and children[0] is child:
anchor = seg
assert anchor.raw.startswith(segment.raw)
child = seg
break
elif edit_type == "create_after" and children[-1] is child:
anchor = seg
assert anchor.raw.endswith(segment.raw)
child = seg
break
return anchor
WittierDinosaur marked this conversation as resolved.
Show resolved Hide resolved

@staticmethod
Expand Down
5 changes: 4 additions & 1 deletion src/sqlfluff/rules/L026.py
Expand Up @@ -148,7 +148,10 @@ def _should_ignore_reference(reference, selectable):
# - They are the target table, similar to an INSERT or UPDATE
# statement, thus not expected to match a table in the FROM
# clause.
return any(seg.is_type("into_table_clause") for seg in ref_path)
if ref_path:
tunetheweb marked this conversation as resolved.
Show resolved Hide resolved
return any(seg.is_type("into_table_clause") for seg in ref_path)
else:
return False # pragma: no cover

@staticmethod
def _get_table_refs(ref, dialect):
Expand Down
3 changes: 1 addition & 2 deletions src/sqlfluff/rules/L028.py
Expand Up @@ -128,8 +128,7 @@ def _generate_fixes(
) -> Optional[List[LintResult]]:
"""Iterate through references and check consistency."""
# How many aliases are there? If more than one then abort.
assert len(table_aliases) > 0, "Only Selects with tables should be checked"
if len(table_aliases) > 1:
if len(table_aliases) != 1:
return None
Comment on lines -131 to 132
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't see why we abort on more than one, but not on 0? Same thing IMHO and this fixes one of the issues raised.


# A buffer to keep any violations.
Expand Down
10 changes: 10 additions & 0 deletions test/fixtures/rules/std_rule_cases/L020.yml
Expand Up @@ -33,3 +33,13 @@ test_pass_subquery:
from table_1 as a
join table_2 as b on a.pk = b.pk
) AS a

test_pass_bigquery_function:
pass_str: |
SELECT
gcpproject.functions.timestamp_parsing(log_tbl.orderdate) AS orderdate
FROM
`gcp-project.data.year_2021` AS log_tbl
configs:
core:
dialect: bigquery
7 changes: 7 additions & 0 deletions test/fixtures/rules/std_rule_cases/L026.yml
Expand Up @@ -200,3 +200,10 @@ test_tsql_select_system_as_identifier:
configs:
core:
dialect: tsql

test_mysql_select_no_from_should_not_except:
pass_str: |
SELECT DATE_SUB('1992-12-31 23:59:59.000002', INTERVAL '1.999999' SECOND_MICROSECOND);
configs:
core:
dialect: mysql
18 changes: 18 additions & 0 deletions test/fixtures/rules/std_rule_cases/L028.yml
Expand Up @@ -282,3 +282,21 @@ test_pass_tsql_parameter:
configs:
core:
dialect: tsql

test_pass_tsql_pivot:
# This should pass for certain dialects
pass_str: |
SELECT 1
FROM
(
SELECT DaysToManufacture, StandardCost
FROM Production.Product
) AS SourceTable
PIVOT
(
AVG(StandardCost)
FOR DaysToManufacture IN ([0], [1], [2], [3], [4])
) AS PivotTable;
configs:
core:
dialect: tsql
15 changes: 0 additions & 15 deletions test/test-lint-rules.sh

This file was deleted.

21 changes: 21 additions & 0 deletions test/test_rules_against_dialect_fixtures.sh
@@ -0,0 +1,21 @@
#!/bin/bash

echo "This script will run 'sqlfluff fix -f' on all our dialect fixtures"
echo "to look for critical errors."
echo "WARNING this will change the fixtures so do not commit these changes!"

DIALECTS=($( ls -d test/fixtures/dialects/*/ ))
for DIALECT in "${DIALECTS[@]}"
do
echo "Testing $DIALECT SQL files fix without critical errors..."
OUTPUT=$(sqlfluff fix -f ${DIALECT})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interested in adding other .sqlfluff files, e.g. Danny's from #2811? (Feel free to create a ticket for later.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's already the longest running job so would suggest a separate, parallel, job if we do that.

However I did test it with tab_space = 2 and got no CRITICAL errors (which is all it currently looks for) any more since your fix, plus the ones here.

Are you still getting the errors if you do it on this branch? Cause if not, then no need to add. The default config identified the issues and they were fixed.

echo "${OUTPUT}" | grep -i -q critical
# exit error, if match
if [ $? -eq 0 ]; then
echo "Critical errors found:"
echo "${OUTPUT}" | grep -i "critical\|test/fixtures" | grep -i critical -A 1
exit 1
fi
done

echo "WARNING If running locally you should revert the changes."
2 changes: 1 addition & 1 deletion tox.ini
Expand Up @@ -84,7 +84,7 @@ commands = doc8 {toxinidir}/docs/source --file-encoding utf8

[testenv:ruleslinting]
allowlist_externals = sh
commands = {toxinidir}/test/test-lint-rules.sh
commands = {toxinidir}/test/test_rules_against_dialect_fixtures.sh


[testenv:docbuild]
Expand Down