chore(sqla): Add explicit bidirectional performant relationships for SQLA model

[john-bodley]

SUMMARY

This PR partially addresses recommendation #⁠1 in #14383 by defining the explicit bidirectional relationship mapping between the SqlaTable model (parent) and the corresponding SqlMetric and TableColumn models (children) to improve the performance of the GET /api/v1/dataset/{pk} endpoint—in conjunction with dpgaspar/Flask-AppBuilder#1961. This PR is a rehash of #22332 after it was discovered there were numerous shortcomings.

The motivation was driven by a specific (and somewhat atypical) virtual dataset at Airbnb which is comprised of over 7,000 columns and 30,000 metrics where previously the /api/v1/dataset/{pk} endpoint would timeout due to:

1. FAB was using Joined Eager Loading which "eagerly" loads the children alongside the parent, however this results in a slew of LEFT OUTER JOINs which will rapidly result into a expansive interim result set—from a multiplicative combinatorial sense—given that the SqlaTable model has potentially multiple high cardinality relationships (owners, columns, metrics). Furthermore the result set—which is repetitive in nature—requires further overhead by SQLAlchemy to de-duplicate the rows against the actual model record.
2. Superset was using lazy loading (see this Slack thread) when back referencing the TableColumn.table field when evaluating the columns.type_generic field which resulted in a SELECT statement per column, i.e., for each column it would execute a SQL statement to fetch the corresponding table ID.

The first issue is partially addressed in dpgaspar/Flask-AppBuilder#1961 in conjunction with this change—as FAB defers the relationship loading logic to Superset—which meant that the response time (excluding downloading of the payload) went from timing out to ~ 20 seconds. The second issue was addressed by defining eager loading of the back (parent) references which are pre-defined whilst fetching the SqlaTable model. This further reduced the response time to ~ 5 seconds. Note this PR is safe to merge prior to the FAB change as in the context of FAB the selectin—a more extensible form of select which can be used when fetching multiple SqlaTable columns—relationship loading technique is ignored given that FAB is using an explicit joinedload which is akin to lazy="joined".

Note to further improve the performance of the GET /api/v1/dataset/{pk} endpoint all requests should likely provide a filter which includes only the subset of the columns required. This should both reduce the backend complexity as well as reduce the size of the response payload.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

CI. Tested locally.

ADDITIONAL INFORMATION

• Has associated issue: [SIP-64] Migrate filter_box to Dashboard Native Filter Component #14383
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[john-bodley]

The default is lazy="select" which results in lazy loading and a SELECT statement per column.

[ktmud]

👏 👏 I swear I've dealt with these join magic somewhere but I can't remember where lol

[villebro]

@john-bodley this is super deep stuff, so I really need to dig into some docs before being able to review this properly. I'll do my best to have this reviewed before Friday morning PST.

[dpgaspar]

I do like the non explicit change on FAB, but it may have some consequences. I want to check some get list performance on dashboards, charts, datasets, currently CI is failling also

[john-bodley]

backrefs, per here, is deemed legacy though is used in some instances—such as the database <> table relationship given how/where the code is defined.

[dpgaspar]

posting some test findings performed locally with examples data:

Without this PR and without the FAB change:

/api/v1/dashboard/: Response time avg: ~27ms

• Query:

SELECT anon_1.created_by_id,
       anon_1.created_by_first_name,
       anon_1.created_by_last_name,
       anon_1.changed_by_id,
       anon_1.changed_by_first_name,
       anon_1.changed_by_last_name,
       anon_1.changed_by_username,
       anon_1.dashboards_created_on,
       anon_1.dashboards_changed_on,
       anon_1.dashboards_id,
       anon_1.dashboards_dashboard_title,
       anon_1.dashboards_position_json,
       anon_1.dashboards_css,
       anon_1.dashboards_certified_by,
       anon_1.dashboards_certification_details,
       anon_1.dashboards_json_metadata,
       anon_1.dashboards_slug,
       anon_1.dashboards_published,
       anon_1.dashboards_is_managed_externally,
       anon_1.dashboards_created_by_fk,
       anon_1.dashboards_changed_by_fk,
       ab_user_1.id,
       ab_user_1.first_name,
       ab_user_1.last_name,
       ab_user_1.username,
       ab_user_1.email,
       ab_role_1.id AS id_1,
       ab_role_1.name
FROM   (SELECT dashboards.uuid                  AS dashboards_uuid,
               dashboards.created_on            AS dashboards_created_on,
               dashboards.changed_on            AS dashboards_changed_on,
               dashboards.id                    AS dashboards_id,
               dashboards.dashboard_title       AS dashboards_dashboard_title,
               dashboards.position_json         AS dashboards_position_json,
               dashboards.description           AS dashboards_description,
               dashboards.css                   AS dashboards_css,
               dashboards.certified_by          AS dashboards_certified_by,
               dashboards.certification_details AS
               dashboards_certification_details,
               dashboards.json_metadata         AS dashboards_json_metadata,
               dashboards.slug                  AS dashboards_slug,
               dashboards.published             AS dashboards_published,
               dashboards.is_managed_externally AS
               dashboards_is_managed_externally,
               dashboards.external_url          AS dashboards_external_url,
               dashboards.created_by_fk         AS dashboards_created_by_fk,
               dashboards.changed_by_fk         AS dashboards_changed_by_fk,
               changed_by.id                    AS changed_by_id,
               changed_by.first_name            AS changed_by_first_name,
               changed_by.last_name             AS changed_by_last_name,
               changed_by.username              AS changed_by_username,
               changed_by.password              AS changed_by_password,
               changed_by.active                AS changed_by_active,
               changed_by.email                 AS changed_by_email,
               changed_by.last_login            AS changed_by_last_login,
               changed_by.login_count           AS changed_by_login_count,
               changed_by.fail_login_count      AS changed_by_fail_login_count,
               changed_by.created_on            AS changed_by_created_on,
               changed_by.changed_on            AS changed_by_changed_on,
               changed_by.created_by_fk         AS changed_by_created_by_fk,
               changed_by.changed_by_fk         AS changed_by_changed_by_fk,
               created_by.id                    AS created_by_id,
               created_by.first_name            AS created_by_first_name,
               created_by.last_name             AS created_by_last_name,
               created_by.username              AS created_by_username,
               created_by.password              AS created_by_password,
               created_by.active                AS created_by_active,
               created_by.email                 AS created_by_email,
               created_by.last_login            AS created_by_last_login,
               created_by.login_count           AS created_by_login_count,
               created_by.fail_login_count      AS created_by_fail_login_count,
               created_by.created_on            AS created_by_created_on,
               created_by.changed_on            AS created_by_changed_on,
               created_by.created_by_fk         AS created_by_created_by_fk,
               created_by.changed_by_fk         AS created_by_changed_by_fk
        FROM   dashboards
               LEFT OUTER JOIN ab_user AS changed_by
                            ON dashboards.changed_by_fk = changed_by.id
               LEFT OUTER JOIN ab_user AS created_by
                            ON dashboards.created_by_fk = created_by.id
        ORDER  BY dashboards.changed_on DESC
        LIMIT  :param_1) AS anon_1
       LEFT OUTER JOIN (dashboard_user AS dashboard_user_1
                        JOIN ab_user AS ab_user_1
                          ON ab_user_1.id = dashboard_user_1.user_id)
                    ON anon_1.dashboards_id = dashboard_user_1.dashboard_id
       LEFT OUTER JOIN (dashboard_roles AS dashboard_roles_1
                        JOIN ab_role AS ab_role_1
                          ON ab_role_1.id = dashboard_roles_1.role_id)
                    ON anon_1.dashboards_id = dashboard_roles_1.dashboard_id
ORDER  BY anon_1.dashboards_changed_on DESC 

/api/v1/dataset/1: Response time avg: ~2300ms

• Query:

SELECT anon_1.database_id,
       anon_1.database_database_name,
       anon_1.created_by_id,
       anon_1.created_by_first_name,
       anon_1.created_by_last_name,
       anon_1.changed_by_id,
       anon_1.changed_by_first_name,
       anon_1.changed_by_last_name,
       anon_1.tables_created_on,
       anon_1.tables_changed_on,
       anon_1.tables_id,
       anon_1.tables_description,
       anon_1.tables_default_endpoint,
       anon_1.tables_filter_select_enabled,
       anon_1.tables_offset,
       anon_1.tables_cache_timeout,
       anon_1.tables_table_name,
       anon_1.tables_main_dttm_col,
       anon_1.tables_database_id,
       anon_1.tables_fetch_values_predicate,
       anon_1.tables_schema,
       anon_1.tables_sql,
       anon_1.tables_is_sqllab_view,
       anon_1.tables_template_params,
       anon_1.tables_extra,
       anon_1.tables_created_by_fk,
       anon_1.tables_changed_by_fk,
       ab_user_1.id,
       ab_user_1.first_name,
       ab_user_1.last_name,
       ab_user_1.username,
       table_columns_1.uuid,
       table_columns_1.created_on,
       table_columns_1.changed_on,
       table_columns_1.id         AS id_1,
       table_columns_1.column_name,
       table_columns_1.verbose_name,
       table_columns_1.is_active,
       table_columns_1.type,
       table_columns_1.advanced_data_type,
       table_columns_1.groupby,
       table_columns_1.filterable,
       table_columns_1.description,
       table_columns_1.is_dttm,
       table_columns_1.expression,
       table_columns_1.python_date_format,
       table_columns_1.extra,
       sql_metrics_1.created_on   AS created_on_1,
       sql_metrics_1.changed_on   AS changed_on_1,
       sql_metrics_1.id           AS id_2,
       sql_metrics_1.metric_name,
       sql_metrics_1.verbose_name AS verbose_name_1,
       sql_metrics_1.metric_type,
       sql_metrics_1.description  AS description_1,
       sql_metrics_1.d3format,
       sql_metrics_1.warning_text,
       sql_metrics_1.expression   AS expression_1,
       sql_metrics_1.extra        AS extra_1
FROM   (SELECT tables.uuid                        AS tables_uuid,
               tables.created_on                  AS tables_created_on,
               tables.changed_on                  AS tables_changed_on,
               tables.id                          AS tables_id,
               tables.description                 AS tables_description,
               tables.default_endpoint            AS tables_default_endpoint,
               tables.is_featured                 AS tables_is_featured,
               tables.filter_select_enabled       AS
               tables_filter_select_enabled,
               tables."offset"                    AS tables_offset,
               tables.cache_timeout               AS tables_cache_timeout,
               tables.params                      AS tables_params,
               tables.perm                        AS tables_perm,
               tables.schema_perm                 AS tables_schema_perm,
               tables.is_managed_externally       AS
               tables_is_managed_externally,
               tables.external_url                AS tables_external_url,
               tables.table_name                  AS tables_table_name,
               tables.main_dttm_col               AS tables_main_dttm_col,
               tables.database_id                 AS tables_database_id,
               tables.fetch_values_predicate      AS
               tables_fetch_values_predicate,
               tables.SCHEMA                      AS tables_schema,
               tables.sql                         AS tables_sql,
               tables.is_sqllab_view              AS tables_is_sqllab_view,
               tables.template_params             AS tables_template_params,
               tables.extra                       AS tables_extra,
               tables.created_by_fk               AS tables_created_by_fk,
               tables.changed_by_fk               AS tables_changed_by_fk,
               database.uuid                      AS database_uuid,
               database.created_on                AS database_created_on,
               database.changed_on                AS database_changed_on,
               database.id                        AS database_id,
               database.verbose_name              AS database_verbose_name,
               database.database_name             AS database_database_name,
               database.sqlalchemy_uri            AS database_sqlalchemy_uri,
               database.password                  AS database_password,
               database.cache_timeout             AS database_cache_timeout,
               database.select_as_create_table_as AS
               database_select_as_create_table_as
                      ,
               database.expose_in_sqllab          AS
                      database_expose_in_sqllab,
               database.configuration_method      AS
               database_configuration_method,
               database.allow_run_async           AS database_allow_run_async,
               database.allow_file_upload         AS database_allow_file_upload,
               database.allow_ctas                AS database_allow_ctas,
               database.allow_cvas                AS database_allow_cvas,
               database.allow_dml                 AS database_allow_dml,
               database.force_ctas_schema         AS database_force_ctas_schema,
               database.extra                     AS database_extra,
               database.encrypted_extra           AS database_encrypted_extra,
               database.impersonate_user          AS database_impersonate_user,
               database.server_cert               AS database_server_cert,
               database.is_managed_externally     AS
               database_is_managed_externally,
               database.external_url              AS database_external_url,
               database.created_by_fk             AS database_created_by_fk,
               database.changed_by_fk             AS database_changed_by_fk,
               created_by.id                      AS created_by_id,
               created_by.first_name              AS created_by_first_name,
               created_by.last_name               AS created_by_last_name,
               created_by.username                AS created_by_username,
               created_by.password                AS created_by_password,
               created_by.active                  AS created_by_active,
               created_by.email                   AS created_by_email,
               created_by.last_login              AS created_by_last_login,
               created_by.login_count             AS created_by_login_count,
               created_by.fail_login_count        AS created_by_fail_login_count
               ,
               created_by.created_on              AS
               created_by_created_on,
               created_by.changed_on              AS created_by_changed_on,
               created_by.created_by_fk           AS created_by_created_by_fk,
               created_by.changed_by_fk           AS created_by_changed_by_fk,
               changed_by.id                      AS changed_by_id,
               changed_by.first_name              AS changed_by_first_name,
               changed_by.last_name               AS changed_by_last_name,
               changed_by.username                AS changed_by_username,
               changed_by.password                AS changed_by_password,
               changed_by.active                  AS changed_by_active,
               changed_by.email                   AS changed_by_email,
               changed_by.last_login              AS changed_by_last_login,
               changed_by.login_count             AS changed_by_login_count,
               changed_by.fail_login_count        AS changed_by_fail_login_count
               ,
               changed_by.created_on              AS
               changed_by_created_on,
               changed_by.changed_on              AS changed_by_changed_on,
               changed_by.created_by_fk           AS changed_by_created_by_fk,
               changed_by.changed_by_fk           AS changed_by_changed_by_fk
        FROM   tables
               LEFT OUTER JOIN dbs AS database
                            ON tables.database_id = database.id
               LEFT OUTER JOIN ab_user AS created_by
                            ON tables.created_by_fk = created_by.id
               LEFT OUTER JOIN ab_user AS changed_by
                            ON tables.changed_by_fk = changed_by.id
        WHERE  tables.id = :id_3) AS anon_1
       LEFT OUTER JOIN (sqlatable_user AS sqlatable_user_1
                        JOIN ab_user AS ab_user_1
                          ON ab_user_1.id = sqlatable_user_1.user_id)
                    ON anon_1.tables_id = sqlatable_user_1.table_id
       LEFT OUTER JOIN table_columns AS table_columns_1
                    ON anon_1.tables_id = table_columns_1.table_id
       LEFT OUTER JOIN sql_metrics AS sql_metrics_1
                    ON anon_1.tables_id = sql_metrics_1.table_id 

Without this PR and with the FAB change:

/api/v1/dashboard/: Response time avg: ~61ms

• Query:

SELECT anon_1.created_by_id,
       anon_1.created_by_first_name,
       anon_1.created_by_last_name,
       anon_1.changed_by_id,
       anon_1.changed_by_first_name,
       anon_1.changed_by_last_name,
       anon_1.changed_by_username,
       anon_1.dashboards_created_on,
       anon_1.dashboards_changed_on,
       anon_1.dashboards_id,
       anon_1.dashboards_dashboard_title,
       anon_1.dashboards_position_json,
       anon_1.dashboards_css,
       anon_1.dashboards_certified_by,
       anon_1.dashboards_certification_details,
       anon_1.dashboards_json_metadata,
       anon_1.dashboards_slug,
       anon_1.dashboards_published,
       anon_1.dashboards_is_managed_externally,
       anon_1.dashboards_created_by_fk,
       anon_1.dashboards_changed_by_fk
FROM   (SELECT dashboards.uuid                  AS dashboards_uuid,
               dashboards.created_on            AS dashboards_created_on,
               dashboards.changed_on            AS dashboards_changed_on,
               dashboards.id                    AS dashboards_id,
               dashboards.dashboard_title       AS dashboards_dashboard_title,
               dashboards.position_json         AS dashboards_position_json,
               dashboards.description           AS dashboards_description,
               dashboards.css                   AS dashboards_css,
               dashboards.certified_by          AS dashboards_certified_by,
               dashboards.certification_details AS
               dashboards_certification_details,
               dashboards.json_metadata         AS dashboards_json_metadata,
               dashboards.slug                  AS dashboards_slug,
               dashboards.published             AS dashboards_published,
               dashboards.is_managed_externally AS
               dashboards_is_managed_externally,
               dashboards.external_url          AS dashboards_external_url,
               dashboards.created_by_fk         AS dashboards_created_by_fk,
               dashboards.changed_by_fk         AS dashboards_changed_by_fk,
               changed_by.id                    AS changed_by_id,
               changed_by.first_name            AS changed_by_first_name,
               changed_by.last_name             AS changed_by_last_name,
               changed_by.username              AS changed_by_username,
               changed_by.password              AS changed_by_password,
               changed_by.active                AS changed_by_active,
               changed_by.email                 AS changed_by_email,
               changed_by.last_login            AS changed_by_last_login,
               changed_by.login_count           AS changed_by_login_count,
               changed_by.fail_login_count      AS changed_by_fail_login_count,
               changed_by.created_on            AS changed_by_created_on,
               changed_by.changed_on            AS changed_by_changed_on,
               changed_by.created_by_fk         AS changed_by_created_by_fk,
               changed_by.changed_by_fk         AS changed_by_changed_by_fk,
               created_by.id                    AS created_by_id,
               created_by.first_name            AS created_by_first_name,
               created_by.last_name             AS created_by_last_name,
               created_by.username              AS created_by_username,
               created_by.password              AS created_by_password,
               created_by.active                AS created_by_active,
               created_by.email                 AS created_by_email,
               created_by.last_login            AS created_by_last_login,
               created_by.login_count           AS created_by_login_count,
               created_by.fail_login_count      AS created_by_fail_login_count,
               created_by.created_on            AS created_by_created_on,
               created_by.changed_on            AS created_by_changed_on,
               created_by.created_by_fk         AS created_by_created_by_fk,
               created_by.changed_by_fk         AS created_by_changed_by_fk
        FROM   dashboards
               LEFT OUTER JOIN ab_user AS changed_by
                            ON dashboards.changed_by_fk = changed_by.id
               LEFT OUTER JOIN ab_user AS created_by
                            ON dashboards.created_by_fk = created_by.id
        ORDER  BY dashboards.changed_on DESC
        LIMIT  :param_1) AS anon_1
ORDER  BY anon_1.dashboards_changed_on DESC 

/api/v1/dataset/1: Response time avg: ~875ms

• Query:

SELECT anon_1.database_id,
       anon_1.database_database_name,
       anon_1.created_by_id,
       anon_1.created_by_first_name,
       anon_1.created_by_last_name,
       anon_1.changed_by_id,
       anon_1.changed_by_first_name,
       anon_1.changed_by_last_name,
       anon_1.tables_created_on,
       anon_1.tables_changed_on,
       anon_1.tables_id,
       anon_1.tables_description,
       anon_1.tables_default_endpoint,
       anon_1.tables_filter_select_enabled,
       anon_1.tables_offset,
       anon_1.tables_cache_timeout,
       anon_1.tables_table_name,
       anon_1.tables_main_dttm_col,
       anon_1.tables_database_id,
       anon_1.tables_fetch_values_predicate,
       anon_1.tables_schema,
       anon_1.tables_sql,
       anon_1.tables_is_sqllab_view,
       anon_1.tables_template_params,
       anon_1.tables_extra,
       anon_1.tables_created_by_fk,
       anon_1.tables_changed_by_fk
FROM   (SELECT tables.uuid                        AS tables_uuid,
               tables.created_on                  AS tables_created_on,
               tables.changed_on                  AS tables_changed_on,
               tables.id                          AS tables_id,
               tables.description                 AS tables_description,
               tables.default_endpoint            AS tables_default_endpoint,
               tables.is_featured                 AS tables_is_featured,
               tables.filter_select_enabled       AS
               tables_filter_select_enabled,
               tables."offset"                    AS tables_offset,
               tables.cache_timeout               AS tables_cache_timeout,
               tables.params                      AS tables_params,
               tables.perm                        AS tables_perm,
               tables.schema_perm                 AS tables_schema_perm,
               tables.is_managed_externally       AS
               tables_is_managed_externally,
               tables.external_url                AS tables_external_url,
               tables.table_name                  AS tables_table_name,
               tables.main_dttm_col               AS tables_main_dttm_col,
               tables.database_id                 AS tables_database_id,
               tables.fetch_values_predicate      AS
               tables_fetch_values_predicate,
               tables.SCHEMA                      AS tables_schema,
               tables.sql                         AS tables_sql,
               tables.is_sqllab_view              AS tables_is_sqllab_view,
               tables.template_params             AS tables_template_params,
               tables.extra                       AS tables_extra,
               tables.created_by_fk               AS tables_created_by_fk,
               tables.changed_by_fk               AS tables_changed_by_fk,
               database.uuid                      AS database_uuid,
               database.created_on                AS database_created_on,
               database.changed_on                AS database_changed_on,
               database.id                        AS database_id,
               database.verbose_name              AS database_verbose_name,
               database.database_name             AS database_database_name,
               database.sqlalchemy_uri            AS database_sqlalchemy_uri,
               database.password                  AS database_password,
               database.cache_timeout             AS database_cache_timeout,
               database.select_as_create_table_as AS
               database_select_as_create_table_as
                      ,
               database.expose_in_sqllab          AS
                      database_expose_in_sqllab,
               database.configuration_method      AS
               database_configuration_method,
               database.allow_run_async           AS database_allow_run_async,
               database.allow_file_upload         AS database_allow_file_upload,
               database.allow_ctas                AS database_allow_ctas,
               database.allow_cvas                AS database_allow_cvas,
               database.allow_dml                 AS database_allow_dml,
               database.force_ctas_schema         AS database_force_ctas_schema,
               database.extra                     AS database_extra,
               database.encrypted_extra           AS database_encrypted_extra,
               database.impersonate_user          AS database_impersonate_user,
               database.server_cert               AS database_server_cert,
               database.is_managed_externally     AS
               database_is_managed_externally,
               database.external_url              AS database_external_url,
               database.created_by_fk             AS database_created_by_fk,
               database.changed_by_fk             AS database_changed_by_fk,
               created_by.id                      AS created_by_id,
               created_by.first_name              AS created_by_first_name,
               created_by.last_name               AS created_by_last_name,
               created_by.username                AS created_by_username,
               created_by.password                AS created_by_password,
               created_by.active                  AS created_by_active,
               created_by.email                   AS created_by_email,
               created_by.last_login              AS created_by_last_login,
               created_by.login_count             AS created_by_login_count,
               created_by.fail_login_count        AS created_by_fail_login_count
               ,
               created_by.created_on              AS
               created_by_created_on,
               created_by.changed_on              AS created_by_changed_on,
               created_by.created_by_fk           AS created_by_created_by_fk,
               created_by.changed_by_fk           AS created_by_changed_by_fk,
               changed_by.id                      AS changed_by_id,
               changed_by.first_name              AS changed_by_first_name,
               changed_by.last_name               AS changed_by_last_name,
               changed_by.username                AS changed_by_username,
               changed_by.password                AS changed_by_password,
               changed_by.active                  AS changed_by_active,
               changed_by.email                   AS changed_by_email,
               changed_by.last_login              AS changed_by_last_login,
               changed_by.login_count             AS changed_by_login_count,
               changed_by.fail_login_count        AS changed_by_fail_login_count
               ,
               changed_by.created_on              AS
               changed_by_created_on,
               changed_by.changed_on              AS changed_by_changed_on,
               changed_by.created_by_fk           AS changed_by_created_by_fk,
               changed_by.changed_by_fk           AS changed_by_changed_by_fk
        FROM   tables
               LEFT OUTER JOIN dbs AS database
                            ON tables.database_id = database.id
               LEFT OUTER JOIN ab_user AS created_by
                            ON tables.created_by_fk = created_by.id
               LEFT OUTER JOIN ab_user AS changed_by
                            ON tables.changed_by_fk = changed_by.id
        WHERE  tables.id = :id_1) AS anon_1 

With this PR and with the FAB change:

/api/v1/dashboard/: Response time avg: ~61ms

• Query: Same has without this PR and with the FAB change

/api/v1/dataset/1: Response time avg: ~610ms

• Query:

SELECT anon_1.database_id,
       anon_1.database_database_name,
       anon_1.created_by_id,
       anon_1.created_by_first_name,
       anon_1.created_by_last_name,
       anon_1.changed_by_id,
       anon_1.changed_by_first_name,
       anon_1.changed_by_last_name,
       anon_1.tables_created_on,
       anon_1.tables_changed_on,
       anon_1.tables_id,
       anon_1.tables_description,
       anon_1.tables_default_endpoint,
       anon_1.tables_filter_select_enabled,
       anon_1.tables_offset,
       anon_1.tables_cache_timeout,
       anon_1.tables_table_name,
       anon_1.tables_main_dttm_col,
       anon_1.tables_database_id,
       anon_1.tables_fetch_values_predicate,
       anon_1.tables_schema,
       anon_1.tables_sql,
       anon_1.tables_is_sqllab_view,
       anon_1.tables_template_params,
       anon_1.tables_extra,
       anon_1.tables_created_by_fk,
       anon_1.tables_changed_by_fk
FROM   (SELECT tables.uuid                        AS tables_uuid,
               tables.created_on                  AS tables_created_on,
               tables.changed_on                  AS tables_changed_on,
               tables.id                          AS tables_id,
               tables.description                 AS tables_description,
               tables.default_endpoint            AS tables_default_endpoint,
               tables.is_featured                 AS tables_is_featured,
               tables.filter_select_enabled       AS
               tables_filter_select_enabled,
               tables."offset"                    AS tables_offset,
               tables.cache_timeout               AS tables_cache_timeout,
               tables.params                      AS tables_params,
               tables.perm                        AS tables_perm,
               tables.schema_perm                 AS tables_schema_perm,
               tables.is_managed_externally       AS
               tables_is_managed_externally,
               tables.external_url                AS tables_external_url,
               tables.table_name                  AS tables_table_name,
               tables.main_dttm_col               AS tables_main_dttm_col,
               tables.database_id                 AS tables_database_id,
               tables.fetch_values_predicate      AS
               tables_fetch_values_predicate,
               tables.SCHEMA                      AS tables_schema,
               tables.sql                         AS tables_sql,
               tables.is_sqllab_view              AS tables_is_sqllab_view,
               tables.template_params             AS tables_template_params,
               tables.extra                       AS tables_extra,
               tables.created_by_fk               AS tables_created_by_fk,
               tables.changed_by_fk               AS tables_changed_by_fk,
               database.uuid                      AS database_uuid,
               database.created_on                AS database_created_on,
               database.changed_on                AS database_changed_on,
               database.id                        AS database_id,
               database.verbose_name              AS database_verbose_name,
               database.database_name             AS database_database_name,
               database.sqlalchemy_uri            AS database_sqlalchemy_uri,
               database.password                  AS database_password,
               database.cache_timeout             AS database_cache_timeout,
               database.select_as_create_table_as AS
               database_select_as_create_table_as
                      ,
               database.expose_in_sqllab          AS
                      database_expose_in_sqllab,
               database.configuration_method      AS
               database_configuration_method,
               database.allow_run_async           AS database_allow_run_async,
               database.allow_file_upload         AS database_allow_file_upload,
               database.allow_ctas                AS database_allow_ctas,
               database.allow_cvas                AS database_allow_cvas,
               database.allow_dml                 AS database_allow_dml,
               database.force_ctas_schema         AS database_force_ctas_schema,
               database.extra                     AS database_extra,
               database.encrypted_extra           AS database_encrypted_extra,
               database.impersonate_user          AS database_impersonate_user,
               database.server_cert               AS database_server_cert,
               database.is_managed_externally     AS
               database_is_managed_externally,
               database.external_url              AS database_external_url,
               database.created_by_fk             AS database_created_by_fk,
               database.changed_by_fk             AS database_changed_by_fk,
               created_by.id                      AS created_by_id,
               created_by.first_name              AS created_by_first_name,
               created_by.last_name               AS created_by_last_name,
               created_by.username                AS created_by_username,
               created_by.password                AS created_by_password,
               created_by.active                  AS created_by_active,
               created_by.email                   AS created_by_email,
               created_by.last_login              AS created_by_last_login,
               created_by.login_count             AS created_by_login_count,
               created_by.fail_login_count        AS created_by_fail_login_count
               ,
               created_by.created_on              AS
               created_by_created_on,
               created_by.changed_on              AS created_by_changed_on,
               created_by.created_by_fk           AS created_by_created_by_fk,
               created_by.changed_by_fk           AS created_by_changed_by_fk,
               changed_by.id                      AS changed_by_id,
               changed_by.first_name              AS changed_by_first_name,
               changed_by.last_name               AS changed_by_last_name,
               changed_by.username                AS changed_by_username,
               changed_by.password                AS changed_by_password,
               changed_by.active                  AS changed_by_active,
               changed_by.email                   AS changed_by_email,
               changed_by.last_login              AS changed_by_last_login,
               changed_by.login_count             AS changed_by_login_count,
               changed_by.fail_login_count        AS changed_by_fail_login_count
               ,
               changed_by.created_on              AS
               changed_by_created_on,
               changed_by.changed_on              AS changed_by_changed_on,
               changed_by.created_by_fk           AS changed_by_created_by_fk,
               changed_by.changed_by_fk           AS changed_by_changed_by_fk
        FROM   tables
               LEFT OUTER JOIN dbs AS database
                            ON tables.database_id = database.id
               LEFT OUTER JOIN ab_user AS created_by
                            ON tables.created_by_fk = created_by.id
               LEFT OUTER JOIN ab_user AS changed_by
                            ON tables.changed_by_fk = changed_by.id
        WHERE  tables.id = :id_1) AS anon_1 

---

Has expected the FAB change will default outer joins to lazy, this will cause general impact on get list since nested responses will generate N+1 queries on this case. I still like your change but it needs to be an opt in feature, example use a config key or a new ModelRestApi attribute.

Regarding specifically /api/v1/dataset/<id> I think that the problem is on the fact that we are nesting a big payload that is not paginated, so your approach with /api/v1/dataset/<id>/column/ is more desirable but with pagination included. On this case we should remove the metrics, columns nested payload out of /api/v1/dataset/<id> and adapt the frontend accordingly. Nested payloads are fine has long has the childs are not way bigger then their parents.

Ultimately do note that we can always implement our own custom endpoints on top of any get or get_list, with our own specific queries, yet the problem with huge non paginated nested payloads will still apply.

[john-bodley]

Thanks @dpgaspar for the detailed analysis. BTW I'm not sure that the two queries which include the FAB change (and their associated timings) are complete as they don't include the additional SELECT statements for fetching the columns and metrics.

The TL;DR—regarding performance—from you analysis is:

FAB	Superset (FWD)	Superset (BWD)	Endpoint	Response Time	Difference
joined	joined*	select	/api/v1/dashboard/	~ 27 ms	N/A
joined	joined *	select	/api/v1/dashboard/1	~ 2,300 ms	N/A
default	select	select	/api/v1/dashboard/	~ 61 ms	+ 34 ms (+ 126%)
default	select	select	/api/v1/dashboard/1	~ 875 ms	- 1,425 ms (- 62%)
default	selectin	joined	/api/v1/dashboard/	~ 61 ms	+ 34 ms (+ 126%)
default	selectin	joined	/api/v1/dashboard/1	~ 610 ms	- 1,690 ms (- 73.5%)

* By enforcement via FAB.

which indicates that there is a performance hit (albeit relatively small from an absolute perspective) for the /api/v1/dashboard/ endpoint, however there is a signification performance improvement (from both an absolute and relative perspective) for the /api/v1/dashboard/1 endpoint, i.e., the results seem quite favorable.

Regarding the get list concern—generating N + 1 queries—I'm not sure if this will materialize given we're using the selectin` eager loading which mentions that,

In most cases, selectin loading is the most simple and efficient way to eagerly load collections of objects.

i.e., though multiple SELECT statements will exists, the number is determined via the number of relationships as opposed to the number of items in the list/collection.

Additionally with regards to #22332, which added the /api/v1/dataset/<id>/column/ endpoint, per #22332 (comment), I realized that this wasn't suffice given what is required by the frontent.

[dpgaspar]

Thanks @dpgaspar for the detailed analysis. BTW I'm not sure that the two queries which include the FAB change (and their associated timings) are complete as they don't include the additional SELECT statements for fetching the columns and metrics.

The TL;DR—regarding performance—from you analysis is:

FAB Superset (FWD) Superset (BWD) Endpoint Response Time Difference
joined joined* select /api/v1/dashboard/ ~ 27 ms N/A
joined joined * select /api/v1/dashboard/1 ~ 2,300 ms N/A
default select select /api/v1/dashboard/ ~ 61 ms + 34 ms (+ 126%)
default select select /api/v1/dashboard/1 ~ 875 ms - 1,425 ms (- 62%)
default selectin joined /api/v1/dashboard/ ~ 61 ms + 34 ms (+ 126%)
default selectin joined /api/v1/dashboard/1 ~ 610 ms - 1,690 ms (- 73.5%)

• By enforcement via FAB.

which indicates that there is a performance hit (albeit relatively small from an absolute perspective) for the /api/v1/dashboard/ endpoint, however there is a signification performance improvement (from both an absolute and relative perspective) for the /api/v1/dashboard/1 endpoint, i.e., the results seem quite favorable.

Regarding the get list concern—generating N + 1 queries—I'm not sure if this will materialize given we're using the selectin` eager loading which mentions that,

In most cases, selectin loading is the most simple and efficient way to eagerly load collections of objects.

i.e., though multiple SELECT statements will exists, the number is determined via the number of relationships as opposed to the number of items in the list/collection.

Additionally with regards to #22332, which added the /api/v1/dataset/<id>/column/ endpoint, per #22332 (comment), I realized that this wasn't suffice given what is required by the frontent.

Ok, I've added a small change on FAB on top of your PR there, now we can have the best from both worlds: dpgaspar/Flask-AppBuilder#1971

This is included on 4.2.0

So use:
show_outer_default_load = True RestModelApi attr when needed to improve performance /api/v1/dataset/<id>

[codecov]

Codecov Report

Merging #22413 (897bf24) into master (1fe4a71) will decrease coverage by 1.82%.
The diff coverage is 100.00%.

❗ Current head 897bf24 differs from pull request most recent head d46ae83. Consider uploading reports for the commit d46ae83 to get more accurate results

@@            Coverage Diff             @@
##           master   #22413      +/-   ##
==========================================
- Coverage   67.28%   65.46%   -1.82%     
==========================================
  Files        1877     1876       -1     
  Lines       72002    71811     -191     
  Branches     7893     7863      -30     
==========================================
- Hits        48444    47011    -1433     
- Misses      21530    22774    +1244     
+ Partials     2028     2026       -2     

Flag	Coverage Δ
hive	?
mysql	78.04% <100.00%> (-0.54%)	⬇️
postgres	78.11% <100.00%> (-0.54%)	⬇️
presto	?
python	78.16% <100.00%> (-3.72%)	⬇️
sqlite	?
unit	?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
superset/models/dashboard.py	76.22% <ø> (-0.39%)	⬇️
superset/connectors/sqla/models.py	88.57% <100.00%> (-0.80%)	⬇️
superset/datasets/api.py	87.50% <100.00%> (+0.10%)	⬆️
superset/examples/birth_names.py	70.00% <100.00%> (ø)
superset/models/helpers.py	38.09% <100.00%> (+0.06%)	⬆️
superset/db_engines/hive.py	0.00% <0.00%> (-85.19%)	⬇️
...set/advanced_data_type/plugins/internet_address.py	16.32% <0.00%> (-79.60%)	⬇️
superset/utils/pandas_postprocessing/boxplot.py	20.51% <0.00%> (-79.49%)	⬇️
superset/charts/post_processing.py	11.76% <0.00%> (-77.95%)	⬇️
...perset/advanced_data_type/plugins/internet_port.py	18.75% <0.00%> (-77.09%)	⬇️
... and 138 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[john-bodley]

Ideally we would use lazy="selectin" rather than lazy="select" (default) but this seems to be causing downstream issues with the associated sqlatable_user table. My current hunch is the selectin option is rather new and there's been a number of reported issues.

[john-bodley]

Databases don't have owners.

[john-bodley]

I was running into a similar issue to this which only happens when I set the lazy="selectin" relationship property which—somewhat peculiarly—only ensures eager loading for the bidirectional relationships when the reciprocal is lazy="joined", i.e., if lazy="select", SQLAlchemy performs N queries for when selecting the TableColumn.table/SqlMetric.table field.

The TL;DR is I've tried a slew of things and the only success—per the Stack Overflow post—was to commit between deleting the dataset and chart. Note this doesn't seem to be an issue in production (per my manual tests) given that one doesn't delete both a dataset/database at the same time.

[john-bodley]

Note I've discovered a performance regression with the /api/v1/explore/?slice_id={pk} RESTful API endpoint which is likley associated with change which I'm currently investigating.