Skip to content

Commit

Permalink
chore: get rid of sqlparse
Browse files Browse the repository at this point in the history
  • Loading branch information
betodealmeida committed Jan 23, 2024
1 parent 74f953d commit 276ccfb
Show file tree
Hide file tree
Showing 9 changed files with 239 additions and 132 deletions.
1 change: 1 addition & 0 deletions superset-frontend/src/SqlLab/actions/sqlLab.js
Original file line number Diff line number Diff line change
Expand Up @@ -912,6 +912,7 @@ export function formatQuery(queryEditor) {
const { sql } = getUpToDateQuery(getState(), queryEditor);
return SupersetClient.post({
endpoint: `/api/v1/sqllab/format_sql/`,
// TODO (betodealmeida): pass engine as a parameter for better formatting
body: JSON.stringify({ sql }),
headers: { 'Content-Type': 'application/json' },
}).then(({ json }) => {
Expand Down
52 changes: 3 additions & 49 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import numpy as np
import pandas as pd
import sqlalchemy as sa
import sqlparse
from flask import escape, Markup
from flask_appbuilder import Model
from flask_appbuilder.security.sqla.models import User
Expand Down Expand Up @@ -1100,7 +1099,9 @@ def _process_sql_expression(


class SqlaTable(
Model, BaseDatasource, ExploreMixin
Model,
BaseDatasource,
ExploreMixin,
): # pylint: disable=too-many-public-methods
"""An ORM object for SqlAlchemy table references"""

Expand Down Expand Up @@ -1414,26 +1415,6 @@ def mutate_query_from_config(self, sql: str) -> str:
def get_template_processor(self, **kwargs: Any) -> BaseTemplateProcessor:
return get_template_processor(table=self, database=self.database, **kwargs)

def get_query_str_extended(
self,
query_obj: QueryObjectDict,
mutate: bool = True,
) -> QueryStringExtended:
sqlaq = self.get_sqla_query(**query_obj)
sql = self.database.compile_sqla_query(sqlaq.sqla_query)
sql = self._apply_cte(sql, sqlaq.cte)
sql = sqlparse.format(sql, reindent=True)
if mutate:
sql = self.mutate_query_from_config(sql)
return QueryStringExtended(
applied_template_filters=sqlaq.applied_template_filters,
applied_filter_columns=sqlaq.applied_filter_columns,
rejected_filter_columns=sqlaq.rejected_filter_columns,
labels_expected=sqlaq.labels_expected,
prequeries=sqlaq.prequeries,
sql=sql,
)

def get_query_str(self, query_obj: QueryObjectDict) -> str:
query_str_ext = self.get_query_str_extended(query_obj)
all_queries = query_str_ext.prequeries + [query_str_ext.sql]
Expand Down Expand Up @@ -1475,33 +1456,6 @@ def get_from_clause(

return from_clause, cte

def get_rendered_sql(
self, template_processor: BaseTemplateProcessor | None = None
) -> str:
"""
Render sql with template engine (Jinja).
"""

sql = self.sql
if template_processor:
try:
sql = template_processor.process_template(sql)
except TemplateError as ex:
raise QueryObjectValidationError(
_(
"Error while rendering virtual dataset query: %(msg)s",
msg=ex.message,
)
) from ex
sql = sqlparse.format(sql.strip("\t\r\n; "), strip_comments=True)
if not sql:
raise QueryObjectValidationError(_("Virtual dataset query cannot be empty"))
if len(sqlparse.split(sql)) > 1:
raise QueryObjectValidationError(
_("Virtual dataset query cannot consist of multiple statements")
)
return sql

def adhoc_metric_to_sqla(
self,
metric: AdhocMetric,
Expand Down
4 changes: 2 additions & 2 deletions superset/db_engine_specs/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
from superset.constants import TimeGrain as TimeGrainConstants
from superset.databases.utils import make_url_safe
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.sql_parse import ParsedQuery, Table
from superset.sql_parse import ParsedQuery, SQLQuery, Table
from superset.superset_typing import ResultSetColumnType, SQLAColumnType
from superset.utils import core as utils
from superset.utils.core import ColumnSpec, GenericDataType
Expand Down Expand Up @@ -1448,7 +1448,7 @@ def select_star( # pylint: disable=too-many-arguments,too-many-locals
qry = partition_query
sql = database.compile_sqla_query(qry)
if indent:
sql = sqlparse.format(sql, reindent=True)
sql = SQLQuery(sql).format()
return sql

@classmethod
Expand Down
7 changes: 4 additions & 3 deletions superset/db_engine_specs/postgres.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from re import Pattern
from typing import Any, TYPE_CHECKING

import sqlparse
from flask_babel import gettext as __
from sqlalchemy.dialects.postgresql import DOUBLE_PRECISION, ENUM, JSON
from sqlalchemy.dialects.postgresql.base import PGInspector
Expand All @@ -37,6 +36,7 @@
from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
from superset.exceptions import SupersetException, SupersetSecurityException
from superset.models.sql_lab import Query
from superset.sql_parse import SQLQuery
from superset.utils import core as utils
from superset.utils.core import GenericDataType

Expand Down Expand Up @@ -281,8 +281,9 @@ def get_default_schema_for_query(
This method simply uses the parent method after checking that there are no
malicious path setting in the query.
"""
sql = sqlparse.format(query.sql, strip_comments=True)
if re.search(r"set\s+search_path\s*=", sql, re.IGNORECASE):
statement = SQLQuery(query.sql)
settings = statement.get_settings()
if "search_path" in settings:

Check warning on line 286 in superset/db_engine_specs/postgres.py

View check run for this annotation

Codecov / codecov/patch

superset/db_engine_specs/postgres.py#L284-L286

Added lines #L284 - L286 were not covered by tests
raise SupersetSecurityException(
SupersetError(
error_type=SupersetErrorType.QUERY_SECURITY_ACCESS_ERROR,
Expand Down
22 changes: 15 additions & 7 deletions superset/models/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@
insert_rls_in_predicate,
ParsedQuery,
sanitize_clause,
SQLQuery,
SQLStatement,
)
from superset.superset_typing import (
AdhocMetric,
Expand Down Expand Up @@ -901,12 +903,14 @@ def _apply_cte(sql: str, cte: Optional[str]) -> str:
return sql

def get_query_str_extended(
self, query_obj: QueryObjectDict, mutate: bool = True
self,
query_obj: QueryObjectDict,
mutate: bool = True,
) -> QueryStringExtended:
sqlaq = self.get_sqla_query(**query_obj)
sql = self.database.compile_sqla_query(sqlaq.sqla_query)
sql = self._apply_cte(sql, sqlaq.cte)
sql = sqlparse.format(sql, reindent=True)
sql = SQLStatement(sql).format()
if mutate:
sql = self.mutate_query_from_config(sql)
return QueryStringExtended(
Expand Down Expand Up @@ -1054,7 +1058,8 @@ def assign_column_label(df: pd.DataFrame) -> Optional[pd.DataFrame]:
)

def get_rendered_sql(
self, template_processor: Optional[BaseTemplateProcessor] = None
self,
template_processor: Optional[BaseTemplateProcessor] = None,
) -> str:
"""
Render sql with template engine (Jinja).
Expand All @@ -1071,13 +1076,16 @@ def get_rendered_sql(
msg=ex.message,
)
) from ex
sql = sqlparse.format(sql.strip("\t\r\n; "), strip_comments=True)
if not sql:
raise QueryObjectValidationError(_("Virtual dataset query cannot be empty"))
if len(sqlparse.split(sql)) > 1:

query = SQLQuery(sql.strip("\t\r\n; "))
if len(query.statements) > 1:
raise QueryObjectValidationError(
_("Virtual dataset query cannot consist of multiple statements")
)

sql = query.statements[0].format(comments=False)
if not sql:
raise QueryObjectValidationError(_("Virtual dataset query cannot be empty"))

Check warning on line 1088 in superset/models/helpers.py

View check run for this annotation

Codecov / codecov/patch

superset/models/helpers.py#L1088

Added line #L1088 was not covered by tests
return sql

def text(self, clause: str) -> TextClause:
Expand Down
Loading

0 comments on commit 276ccfb

Please sign in to comment.