You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/08/05 04:14:08 UTC
[GitHub] [superset] ktmud commented on a diff in pull request #20729: fix: Ensure Presto database engine spec correctly handles Trino
ktmud commented on code in PR #20729:
URL: https://github.com/apache/superset/pull/20729#discussion_r938351037
##########
superset/db_engine_specs/presto.py:
##########
@@ -906,21 +907,23 @@ def extra_table_metadata(
indexes = database.get_indexes(table_name, schema_name)
if indexes:
cols = indexes[0].get("column_names", [])
- full_table_name = table_name
- if schema_name and "." not in table_name:
- full_table_name = "{}.{}".format(schema_name, table_name)
- pql = cls._partition_query(full_table_name, database)
- col_names, latest_parts = cls.latest_partition(
- table_name, schema_name, database, show_first=True
- )
- if not latest_parts:
- latest_parts = tuple([None] * len(col_names))
- metadata["partitions"] = {
- "cols": cols,
- "latest": dict(zip(col_names, latest_parts)),
- "partitionQuery": pql,
- }
+ if cols:
+ full_table_name = table_name
+ if schema_name and "." not in table_name:
+ full_table_name = "{}.{}".format(schema_name, table_name)
Review Comment:
f-string?
##########
superset/db_engine_specs/presto.py:
##########
@@ -906,21 +907,23 @@ def extra_table_metadata(
indexes = database.get_indexes(table_name, schema_name)
if indexes:
cols = indexes[0].get("column_names", [])
- full_table_name = table_name
- if schema_name and "." not in table_name:
- full_table_name = "{}.{}".format(schema_name, table_name)
- pql = cls._partition_query(full_table_name, database)
- col_names, latest_parts = cls.latest_partition(
- table_name, schema_name, database, show_first=True
- )
- if not latest_parts:
- latest_parts = tuple([None] * len(col_names))
- metadata["partitions"] = {
- "cols": cols,
- "latest": dict(zip(col_names, latest_parts)),
- "partitionQuery": pql,
- }
+ if cols:
+ full_table_name = table_name
+ if schema_name and "." not in table_name:
+ full_table_name = "{}.{}".format(schema_name, table_name)
+ pql = cls._partition_query(full_table_name, database)
+ col_names, latest_parts = cls.latest_partition(
+ table_name, schema_name, database, show_first=True
+ )
+
+ if not latest_parts:
+ latest_parts = tuple([None] * len(col_names))
+ metadata["partitions"] = {
+ "cols": cols,
+ "latest": dict(zip(col_names, latest_parts)),
Review Comment:
I feel we should probably change the signature of `latest_partition()` itself to return this dict---would worth another PR.
For this code, it seems it can be simplified as:
```suggestion
"latest": {
col: latest_parts[i] if latest_parts else None
for i, col in enumerate(col_names)
}
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org