betodealmeida commented on a change in pull request #16822:
URL: https://github.com/apache/superset/pull/16822#discussion_r717097074
##########
File path: superset/db_engine_specs/base.py
##########
@@ -952,9 +952,11 @@ def select_star( # pylint:
disable=too-many-arguments,too-many-locals
if (show_cols or latest_partition) and not cols:
cols = database.get_columns(table_name, schema)
- if show_cols:
- fields = cls._get_fields(cols)
quote = engine.dialect.identifier_preparer.quote
+ if show_cols:
+ # Explicitly quote all column names, as BigQuery doesn't quote
column
+ # names that are also identifiers (eg, "limit") by default.
+ fields = [text(quote(col["name"])) for col in cols]
Review comment:
Ahh, looks like someone changed the behavior of `_get_fields` in
BigQuery:
https://github.com/apache/superset/blob/master/superset/db_engine_specs/bigquery.py#L284-L296
##########
File path: superset/db_engine_specs/base.py
##########
@@ -952,9 +952,11 @@ def select_star( # pylint:
disable=too-many-arguments,too-many-locals
if (show_cols or latest_partition) and not cols:
cols = database.get_columns(table_name, schema)
- if show_cols:
- fields = cls._get_fields(cols)
quote = engine.dialect.identifier_preparer.quote
+ if show_cols:
+ # Explicitly quote all column names, as BigQuery doesn't quote
column
+ # names that are also identifiers (eg, "limit") by default.
+ fields = [text(quote(col["name"])) for col in cols]
Review comment:
This goes back 3+ years: https://github.com/apache/superset/pull/5655
##########
File path: superset/db_engine_specs/bigquery.py
##########
@@ -281,20 +281,6 @@ def extra_table_metadata(
"clustering": {"cols": cluster_columns},
}
- @classmethod
- def _get_fields(cls, cols: List[Dict[str, Any]]) -> List[ColumnClause]:
- """
- BigQuery dialect requires us to not use backtick in the fieldname
which are
- nested.
- Using literal_column handles that issue.
-
https://docs.sqlalchemy.org/en/latest/core/tutorial.html#using-more-specific-text-with-table-literal-column-and-column
- Also explicility specifying column names so we don't encounter
duplicate
- column names in the result.
- """
- return [
- literal_column(c["name"]).label(c["name"].replace(".", "__")) for
c in cols
- ]
Review comment:
This was implemented in 2018
(https://github.com/apache/superset/pull/5655) and is no longer needed. I
tested loading the preview for a table with nested records and it works fine
when the `_get_fields` method is removed, each part gets quoted separately:
Note that the data preview query quotes the parts correctly, even though it
fails (for an unrelated reason).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
