You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by el...@apache.org on 2021/12/06 21:50:11 UTC
[superset] branch 1.4 updated (361e510 -> 9468bdf)
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a change to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git.
from 361e510 update changelog and updating.md for 1.4.0
new 733fc74 Handle undefined (#17183)
new 9818bc5 Revert "fix(native-filters): Fix update ownState (#17181)" (#17311)
new 46343a9 fix: avoid escaping bind-like params containing colons (#17419)
new c42ff79 use full resultType with csv download on chart in dashboard (#17431)
new 9468bdf fix(sqllab): Have table name tooltip only show when name is truncated (#17386)
The 5 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../components/gridComponents/Chart_spec.jsx | 2 +-
.../src/SqlLab/components/TableElement/index.tsx | 72 +++++++++++--------
.../src/dashboard/components/Dashboard.jsx | 81 +++++++---------------
.../dashboard/components/gridComponents/Chart.jsx | 2 +-
.../dashboard/util/getLeafComponentIdFromPath.js | 2 +-
superset/connectors/sqla/models.py | 20 ++++--
superset/utils/core.py | 29 --------
tests/integration_tests/charts/api_tests.py | 59 +++++++++++++++-
8 files changed, 141 insertions(+), 126 deletions(-)
[superset] 01/05: Handle undefined (#17183)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 733fc7494cf24a93eaf6472ad05a677ed727e15e
Author: Geido <60...@users.noreply.github.com>
AuthorDate: Thu Oct 21 19:34:32 2021 +0300
Handle undefined (#17183)
(cherry picked from commit 91199c30d8ccbc9d6fef86de30a9e3450f6f170c)
---
superset-frontend/src/dashboard/util/getLeafComponentIdFromPath.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/superset-frontend/src/dashboard/util/getLeafComponentIdFromPath.js b/superset-frontend/src/dashboard/util/getLeafComponentIdFromPath.js
index 9f8f991..37bed77 100644
--- a/superset-frontend/src/dashboard/util/getLeafComponentIdFromPath.js
+++ b/superset-frontend/src/dashboard/util/getLeafComponentIdFromPath.js
@@ -24,7 +24,7 @@ export default function getLeafComponentIdFromPath(directPathToChild = []) {
while (currentPath.length) {
const componentId = currentPath.pop();
- const componentType = componentId.split('-')[0];
+ const componentType = componentId && componentId.split('-')[0];
if (!IN_COMPONENT_ELEMENT_TYPES.includes(componentType)) {
return componentId;
[superset] 02/05: Revert "fix(native-filters): Fix update ownState (#17181)" (#17311)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 9818bc5e7a37f3883c4e9581cdeec8561605eea5
Author: Erik Ritter <er...@airbnb.com>
AuthorDate: Tue Nov 2 13:17:29 2021 -0700
Revert "fix(native-filters): Fix update ownState (#17181)" (#17311)
This reverts commit cf284ba3c72550f64ddb19aeed44de2c5cf0b677.
(cherry picked from commit 7c6d6f47bf71dce15e049f37fe82076bf7cb9c63)
---
.../src/dashboard/components/Dashboard.jsx | 81 +++++++---------------
1 file changed, 24 insertions(+), 57 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx
index 1377789..7e67288 100644
--- a/superset-frontend/src/dashboard/components/Dashboard.jsx
+++ b/superset-frontend/src/dashboard/components/Dashboard.jsx
@@ -220,55 +220,6 @@ class Dashboard extends React.PureComponent {
return Object.values(this.props.charts);
}
- isFilterKeyRemoved(filterKey) {
- const { appliedFilters } = this;
- const { activeFilters } = this.props;
-
- // refresh charts if a filter was removed, added, or changed
- const currFilterKeys = Object.keys(activeFilters);
- const appliedFilterKeys = Object.keys(appliedFilters);
-
- return (
- !currFilterKeys.includes(filterKey) &&
- appliedFilterKeys.includes(filterKey)
- );
- }
-
- isFilterKeyNewlyAdded(filterKey) {
- const { appliedFilters } = this;
- const appliedFilterKeys = Object.keys(appliedFilters);
-
- return !appliedFilterKeys.includes(filterKey);
- }
-
- isFilterKeyChangedValue(filterKey) {
- const { appliedFilters } = this;
- const { activeFilters } = this.props;
-
- return !areObjectsEqual(
- appliedFilters[filterKey].values,
- activeFilters[filterKey].values,
- {
- ignoreUndefined: true,
- },
- );
- }
-
- isFilterKeyChangedScope(filterKey) {
- const { appliedFilters } = this;
- const { activeFilters } = this.props;
-
- return !areObjectsEqual(
- appliedFilters[filterKey].scope,
- activeFilters[filterKey].scope,
- );
- }
-
- hasFilterKeyValues(filterKey) {
- const { appliedFilters } = this;
- return Object.keys(appliedFilters[filterKey]?.values ?? []).length;
- }
-
applyFilters() {
const { appliedFilters } = this;
const { activeFilters, ownDataCharts } = this.props;
@@ -284,21 +235,37 @@ class Dashboard extends React.PureComponent {
);
[...allKeys].forEach(filterKey => {
if (
- this.isFilterKeyRemoved(filterKey) ||
- this.isFilterKeyNewlyAdded(filterKey)
+ !currFilterKeys.includes(filterKey) &&
+ appliedFilterKeys.includes(filterKey)
) {
- // check if there are values in filter, if no, there is was added only ownState, so no need reload other charts
- if (this.hasFilterKeyValues(filterKey)) {
- affectedChartIds.push(...appliedFilters[filterKey].scope);
- }
+ // filterKey is removed?
+ affectedChartIds.push(...appliedFilters[filterKey].scope);
+ } else if (!appliedFilterKeys.includes(filterKey)) {
+ // filterKey is newly added?
+ affectedChartIds.push(...activeFilters[filterKey].scope);
} else {
+ // if filterKey changes value,
// update charts in its scope
- if (this.isFilterKeyChangedValue(filterKey)) {
+ if (
+ !areObjectsEqual(
+ appliedFilters[filterKey].values,
+ activeFilters[filterKey].values,
+ {
+ ignoreUndefined: true,
+ },
+ )
+ ) {
affectedChartIds.push(...activeFilters[filterKey].scope);
}
+ // if filterKey changes scope,
// update all charts in its scope
- if (this.isFilterKeyChangedScope(filterKey)) {
+ if (
+ !areObjectsEqual(
+ appliedFilters[filterKey].scope,
+ activeFilters[filterKey].scope,
+ )
+ ) {
const chartsInScope = (activeFilters[filterKey].scope || []).concat(
appliedFilters[filterKey].scope || [],
);
[superset] 03/05: fix: avoid escaping bind-like params containing colons (#17419)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 46343a9dca29bbadd17f34c58dc0adda5d4ff6f5
Author: Ville Brofeldt <33...@users.noreply.github.com>
AuthorDate: Sat Nov 13 09:01:49 2021 +0200
fix: avoid escaping bind-like params containing colons (#17419)
* fix: avoid escaping bind-like params containing colons
* fix query for mysql
* address comments
(cherry picked from commit ad8a7c42f9da8ce6092b368d7081c3e06b797f8d)
---
superset/connectors/sqla/models.py | 20 +++++++---
superset/utils/core.py | 29 --------------
tests/integration_tests/charts/api_tests.py | 59 ++++++++++++++++++++++++++++-
3 files changed, 72 insertions(+), 36 deletions(-)
diff --git a/superset/connectors/sqla/models.py b/superset/connectors/sqla/models.py
index fcb40f2..b1f1e7c 100644
--- a/superset/connectors/sqla/models.py
+++ b/superset/connectors/sqla/models.py
@@ -64,7 +64,7 @@ from sqlalchemy.engine.base import Connection
from sqlalchemy.orm import backref, Query, relationship, RelationshipProperty, Session
from sqlalchemy.orm.mapper import Mapper
from sqlalchemy.schema import UniqueConstraint
-from sqlalchemy.sql import column, ColumnElement, literal_column, table, text
+from sqlalchemy.sql import column, ColumnElement, literal_column, table
from sqlalchemy.sql.elements import ColumnClause, TextClause
from sqlalchemy.sql.expression import Label, Select, TextAsFrom
from sqlalchemy.sql.selectable import Alias, TableClause
@@ -102,6 +102,16 @@ logger = logging.getLogger(__name__)
VIRTUAL_TABLE_ALIAS = "virtual_table"
+def text(clause: str) -> TextClause:
+ """
+ SQLALchemy wrapper to ensure text clauses are escaped properly
+
+ :param clause: clause potentially containing colons
+ :return: text clause with escaped colons
+ """
+ return sa.text(clause.replace(":", "\\:"))
+
+
class SqlaQuery(NamedTuple):
applied_template_filters: List[str]
extra_cache_keys: List[Any]
@@ -793,7 +803,7 @@ class SqlaTable(Model, BaseDatasource): # pylint: disable=too-many-public-metho
raise QueryObjectValidationError(
_("Virtual dataset query must be read-only")
)
- return TextAsFrom(sa.text(from_sql), []).alias(VIRTUAL_TABLE_ALIAS)
+ return TextAsFrom(text(from_sql), []).alias(VIRTUAL_TABLE_ALIAS)
def get_rendered_sql(
self, template_processor: Optional[BaseTemplateProcessor] = None
@@ -814,8 +824,6 @@ class SqlaTable(Model, BaseDatasource): # pylint: disable=too-many-public-metho
)
) from ex
sql = sqlparse.format(sql.strip("\t\r\n; "), strip_comments=True)
- # we need to escape strings that SQLAlchemy interprets as bind parameters
- sql = utils.escape_sqla_query_binds(sql)
if not sql:
raise QueryObjectValidationError(_("Virtual dataset query cannot be empty"))
if len(sqlparse.split(sql)) > 1:
@@ -1265,7 +1273,7 @@ class SqlaTable(Model, BaseDatasource): # pylint: disable=too-many-public-metho
msg=ex.message,
)
) from ex
- where_clause_and += [sa.text("({})".format(where))]
+ where_clause_and += [text(f"({where})")]
having = extras.get("having")
if having:
try:
@@ -1277,7 +1285,7 @@ class SqlaTable(Model, BaseDatasource): # pylint: disable=too-many-public-metho
msg=ex.message,
)
) from ex
- having_clause_and += [sa.text("({})".format(having))]
+ having_clause_and += [text(f"({having})")]
if apply_fetch_values_predicate and self.fetch_values_predicate:
qry = qry.where(self.get_fetch_values_predicate())
if granularity:
diff --git a/superset/utils/core.py b/superset/utils/core.py
index 9f30c64..71d5934 100644
--- a/superset/utils/core.py
+++ b/superset/utils/core.py
@@ -80,7 +80,6 @@ from sqlalchemy import event, exc, select, Text
from sqlalchemy.dialects.mysql import MEDIUMTEXT
from sqlalchemy.engine import Connection, Engine
from sqlalchemy.engine.reflection import Inspector
-from sqlalchemy.sql.elements import TextClause
from sqlalchemy.sql.type_api import Variant
from sqlalchemy.types import TEXT, TypeDecorator, TypeEngine
from typing_extensions import TypedDict, TypeGuard
@@ -132,8 +131,6 @@ JS_MAX_INTEGER = 9007199254740991 # Largest int Java Script can handle 2^53-1
InputType = TypeVar("InputType")
-BIND_PARAM_REGEX = TextClause._bind_params_regex # pylint: disable=protected-access
-
class LenientEnum(Enum):
"""Enums with a `get` method that convert a enum value to `Enum` if it is a
@@ -1787,29 +1784,3 @@ def apply_max_row_limit(limit: int, max_limit: Optional[int] = None,) -> int:
if limit != 0:
return min(max_limit, limit)
return max_limit
-
-
-def escape_sqla_query_binds(sql: str) -> str:
- """
- Replace strings in a query that SQLAlchemy would otherwise interpret as
- bind parameters.
-
- :param sql: unescaped query string
- :return: escaped query string
- >>> escape_sqla_query_binds("select ':foo'")
- "select '\\\\:foo'"
- >>> escape_sqla_query_binds("select 'foo'::TIMESTAMP")
- "select 'foo'::TIMESTAMP"
- >>> escape_sqla_query_binds("select ':foo :bar'::TIMESTAMP")
- "select '\\\\:foo \\\\:bar'::TIMESTAMP"
- >>> escape_sqla_query_binds("select ':foo :foo :bar'::TIMESTAMP")
- "select '\\\\:foo \\\\:foo \\\\:bar'::TIMESTAMP"
- """
- matches = BIND_PARAM_REGEX.finditer(sql)
- processed_binds = set()
- for match in matches:
- bind = match.group(0)
- if bind not in processed_binds:
- sql = sql.replace(bind, bind.replace(":", "\\:"))
- processed_binds.add(bind)
- return sql
diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py
index 3647442..fd228b6 100644
--- a/tests/integration_tests/charts/api_tests.py
+++ b/tests/integration_tests/charts/api_tests.py
@@ -20,7 +20,7 @@ import json
import unittest
from datetime import datetime
from io import BytesIO
-from typing import Optional
+from typing import Optional, List
from unittest import mock
from zipfile import is_zipfile, ZipFile
@@ -42,6 +42,7 @@ from tests.integration_tests.fixtures.world_bank_dashboard import (
load_world_bank_dashboard_with_slices,
)
from tests.integration_tests.test_app import app
+from superset import security_manager
from superset.charts.commands.data import ChartDataCommand
from superset.connectors.sqla.models import SqlaTable, TableColumn
from superset.errors import SupersetErrorType
@@ -57,6 +58,7 @@ from superset.utils.core import (
ChartDataResultFormat,
get_example_database,
get_main_database,
+ AdhocMetricExpressionType,
)
@@ -2031,3 +2033,58 @@ class TestChartApi(SupersetTestCase, ApiOwnersTestCaseMixin, InsertChartMixin):
self.assertEqual(
set(column for column in data[0].keys()), {"state", "name", "sum__num"}
)
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_chart_data_virtual_table_with_colons(self):
+ """
+ Chart data API: test query with literal colon characters in query, metrics,
+ where clause and filters
+ """
+ self.login(username="admin")
+ owner = self.get_user("admin").id
+ user = db.session.query(security_manager.user_model).get(owner)
+
+ table = SqlaTable(
+ table_name="virtual_table_1",
+ schema=get_example_default_schema(),
+ owners=[user],
+ database=get_example_database(),
+ sql="select ':foo' as foo, ':bar:' as bar, state, num from birth_names",
+ )
+ db.session.add(table)
+ db.session.commit()
+ table.fetch_metadata()
+
+ request_payload = get_query_context("birth_names")
+ request_payload["datasource"] = {
+ "type": "table",
+ "id": table.id,
+ }
+ request_payload["queries"][0]["columns"] = ["foo", "bar", "state"]
+ request_payload["queries"][0]["where"] = "':abc' != ':xyz:qwerty'"
+ request_payload["queries"][0]["orderby"] = None
+ request_payload["queries"][0]["metrics"] = [
+ {
+ "expressionType": AdhocMetricExpressionType.SQL,
+ "sqlExpression": "sum(case when state = ':asdf' then 0 else 1 end)",
+ "label": "count",
+ }
+ ]
+ request_payload["queries"][0]["filters"] = [
+ {"col": "foo", "op": "!=", "val": ":qwerty:",}
+ ]
+
+ rv = self.post_assert_metric(CHART_DATA_URI, request_payload, "data")
+ db.session.delete(table)
+ db.session.commit()
+ assert rv.status_code == 200
+ response_payload = json.loads(rv.data.decode("utf-8"))
+ result = response_payload["result"][0]
+ data = result["data"]
+ assert {col for col in data[0].keys()} == {"foo", "bar", "state", "count"}
+ # make sure results and query parameters are unescaped
+ assert {row["foo"] for row in data} == {":foo"}
+ assert {row["bar"] for row in data} == {":bar:"}
+ assert "':asdf'" in result["query"]
+ assert "':xyz:qwerty'" in result["query"]
+ assert "':qwerty:'" in result["query"]
[superset] 05/05: fix(sqllab): Have table name tooltip only show when name is truncated (#17386)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 9468bdf0c16444861e12d2b636c04a87e78636ca
Author: Corbin Robb <31...@users.noreply.github.com>
AuthorDate: Fri Nov 19 10:34:28 2021 -0700
fix(sqllab): Have table name tooltip only show when name is truncated (#17386)
* Add conditional to table name tooltip to only show when overflowing
* Remove uneccessary state and useEffect, a little clean up and slight refactoring
Co-authored-by: Corbin Robb <co...@Corbins-MacBook-Pro.local>
(cherry picked from commit 8e1619b1055e50c9ee7cbab650f777ec6a64ef3e)
---
.../src/SqlLab/components/TableElement/index.tsx | 72 +++++++++++++---------
1 file changed, 42 insertions(+), 30 deletions(-)
diff --git a/superset-frontend/src/SqlLab/components/TableElement/index.tsx b/superset-frontend/src/SqlLab/components/TableElement/index.tsx
index 4be3935..0d1624d 100644
--- a/superset-frontend/src/SqlLab/components/TableElement/index.tsx
+++ b/superset-frontend/src/SqlLab/components/TableElement/index.tsx
@@ -77,6 +77,7 @@ const Fade = styled.div`
const TableElement = ({ table, actions, ...props }: TableElementProps) => {
const [sortColumns, setSortColumns] = useState(false);
const [hovered, setHovered] = useState(false);
+ const tableNameRef = React.useRef<HTMLInputElement>(null);
const setHover = (hovered: boolean) => {
debounce(() => setHovered(hovered), 100)();
@@ -213,39 +214,50 @@ const TableElement = ({ table, actions, ...props }: TableElementProps) => {
);
};
- const renderHeader = () => (
- <div
- className="clearfix header-container"
- onMouseEnter={() => setHover(true)}
- onMouseLeave={() => setHover(false)}
- >
- <Tooltip
- id="copy-to-clipboard-tooltip"
- placement="topLeft"
- style={{ cursor: 'pointer' }}
- title={table.name}
- trigger={['hover']}
- >
- <StyledSpan data-test="collapse" className="table-name">
- <strong>{table.name}</strong>
- </StyledSpan>
- </Tooltip>
+ const renderHeader = () => {
+ const element: HTMLInputElement | null = tableNameRef.current;
+ let trigger: string[] = [];
+ if (element && element.offsetWidth < element.scrollWidth) {
+ trigger = ['hover'];
+ }
- <div className="pull-right header-right-side">
- {table.isMetadataLoading || table.isExtraMetadataLoading ? (
- <Loading position="inline" />
- ) : (
- <Fade
- data-test="fade"
- hovered={hovered}
- onClick={e => e.stopPropagation()}
+ return (
+ <div
+ className="clearfix header-container"
+ onMouseEnter={() => setHover(true)}
+ onMouseLeave={() => setHover(false)}
+ >
+ <Tooltip
+ id="copy-to-clipboard-tooltip"
+ style={{ cursor: 'pointer' }}
+ title={table.name}
+ trigger={trigger}
+ >
+ <StyledSpan
+ data-test="collapse"
+ ref={tableNameRef}
+ className="table-name"
>
- {renderControls()}
- </Fade>
- )}
+ <strong>{table.name}</strong>
+ </StyledSpan>
+ </Tooltip>
+
+ <div className="pull-right header-right-side">
+ {table.isMetadataLoading || table.isExtraMetadataLoading ? (
+ <Loading position="inline" />
+ ) : (
+ <Fade
+ data-test="fade"
+ hovered={hovered}
+ onClick={e => e.stopPropagation()}
+ >
+ {renderControls()}
+ </Fade>
+ )}
+ </div>
</div>
- </div>
- );
+ );
+ };
const renderBody = () => {
let cols;
[superset] 04/05: use full resultType with csv download on chart in dashboard (#17431)
Posted by el...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
elizabeth pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/superset.git
commit c42ff7972f8879f83de807a411b3e5f84bab3d75
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Mon Nov 15 09:57:05 2021 -0800
use full resultType with csv download on chart in dashboard (#17431)
(cherry picked from commit 71e3fa1bf350045da9a1c1e89443549c204a3516)
---
.../spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx | 2 +-
superset-frontend/src/dashboard/components/gridComponents/Chart.jsx | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
index fdbd766..b2acc42 100644
--- a/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
+++ b/superset-frontend/spec/javascripts/dashboard/components/gridComponents/Chart_spec.jsx
@@ -116,7 +116,7 @@ describe('Chart', () => {
expect(stubbedExportCSV.lastCall.args[0]).toEqual(
expect.objectContaining({
formData: expect.anything(),
- resultType: 'results',
+ resultType: 'full',
resultFormat: 'csv',
}),
);
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
index 4a86d89..3447c98 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/Chart.jsx
@@ -242,7 +242,7 @@ export default class Chart extends React.Component {
formData: isFullCSV
? { ...this.props.formData, row_limit: this.props.maxRows }
: this.props.formData,
- resultType: 'results',
+ resultType: 'full',
resultFormat: 'csv',
});
}