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

Fixing get_query_str #2478

Merged
merged 1 commit into from
Mar 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 24 additions & 19 deletions superset/connectors/sqla/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@

from superset import db, utils, import_util
from superset.connectors.base import BaseDatasource, BaseColumn, BaseMetric
from superset.utils import (
wrap_clause_in_parens,
DTTM_ALIAS, QueryStatus
)
from superset.utils import DTTM_ALIAS, QueryStatus
from superset.models.helpers import QueryResult
from superset.models.core import Database
from superset.jinja_context import get_template_processor
Expand Down Expand Up @@ -293,7 +290,7 @@ def values_for_column(self, column_name, limit=10000):
cols = {col.column_name: col for col in self.columns}
target_col = cols[column_name]

tbl = table(self.table_name)
tbl = self.get_sqla_table()
qry = (
select([target_col.sqla_col])
.select_from(tbl)
Expand All @@ -312,24 +309,33 @@ def values_for_column(self, column_name, limit=10000):
engine, compile_kwargs={"literal_binds": True}, ),
)

df = pd.read_sql_query(
sql=sql,
con=engine
)
df = pd.read_sql_query(sql=sql, con=engine)
return [row[0] for row in df.to_records(index=False)]

def get_template_processor(self, **kwargs):
return get_template_processor(
table=self, database=self.database, **kwargs)

def get_query_str(self, **kwargs):
engine = self.database.get_sqla_engine()
qry = self.get_sqla_query(**kwargs)
sql = str(qry.compile(kwargs['engine']))
sql = str(
qry.compile(
engine,
compile_kwargs={"literal_binds": True}
)
)
logging.info(sql)
sql = sqlparse.format(sql, reindent=True)
sql = self.database.db_engine_spec.sql_preprocessor(sql)
return sql

def get_sqla_table(self):
tbl = table(self.table_name)
if self.schema:
tbl.schema = self.schema
return tbl

def get_sqla_query( # sqla
self,
groupby, metrics,
Expand Down Expand Up @@ -437,14 +443,12 @@ def visit_column(element, compiler, **kw):
select_exprs += metrics_exprs
qry = sa.select(select_exprs)

tbl = table(self.table_name)
if self.schema:
tbl.schema = self.schema

# Supporting arbitrary SQL statements in place of tables
if self.sql:
from_sql = template_processor.process_template(self.sql)
tbl = TextAsFrom(sa.text(from_sql), []).alias('expr_qry')
else:
tbl = self.get_sqla_table()

if not columns:
qry = qry.group_by(*groupby_exprs)
Expand Down Expand Up @@ -484,12 +488,12 @@ def visit_column(element, compiler, **kw):
if extras:
where = extras.get('where')
if where:
where_clause_and += [wrap_clause_in_parens(
template_processor.process_template(where))]
where = template_processor.process_template(where)
where_clause_and += [sa.text('({})'.format(where))]
having = extras.get('having')
if having:
having_clause_and += [wrap_clause_in_parens(
template_processor.process_template(having))]
having = template_processor.process_template(having)
having_clause_and += [sa.text('({})'.format(having))]
if granularity:
qry = qry.where(and_(*([time_filter] + where_clause_and)))
else:
Expand Down Expand Up @@ -536,14 +540,15 @@ def query(self, query_obj):
qry_start_dttm = datetime.now()
engine = self.database.get_sqla_engine()
qry = self.get_sqla_query(**query_obj)
sql = str(qry)
sql = self.get_query_str(**query_obj)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the actual fix, the rest of the PR is mostly syntactic improvements I touched up while debugging.

status = QueryStatus.SUCCESS
error_message = None
df = None
try:
df = pd.read_sql_query(qry, con=engine)
except Exception as e:
status = QueryStatus.FAILED
logging.exception(e)
error_message = str(e)

return QueryResult(
Expand Down
8 changes: 0 additions & 8 deletions superset/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -415,14 +415,6 @@ def __exit__(self, type, value, traceback):
logging.warning("timeout can't be used in the current context")
logging.exception(e)


def wrap_clause_in_parens(sql):
"""Wrap where/having clause with parenthesis if necessary"""
if sql.strip():
sql = '({})'.format(sql)
return sa.text(sql)


def pessimistic_connection_handling(target):
@event.listens_for(target, "checkout")
def ping_connection(dbapi_connection, connection_record, connection_proxy):
Expand Down
7 changes: 5 additions & 2 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1126,8 +1126,11 @@ def filter(self, datasource_type, datasource_id, column):
"""
# TODO: Cache endpoint by user, datasource and column
datasource_class = ConnectorRegistry.sources[datasource_type]
datasource = db.session.query(
datasource_class).filter_by(id=datasource_id).first()
datasource = (
db.session.query(datasource_class)
.filter_by(id=datasource_id)
.first()
)

if not datasource:
return json_error_response(DATASOURCE_MISSING_ERR)
Expand Down
2 changes: 0 additions & 2 deletions superset/viz.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
from markdown import markdown
import simplejson as json
from six import string_types, PY3
from werkzeug.datastructures import ImmutableMultiDict, MultiDict
from werkzeug.urls import Href
from dateutil import relativedelta as rdelta

from superset import app, utils, cache
Expand Down