[GitHub] [superset] mayurnewase commented on a change in pull request #12482: refactor(connectors): refactor sqla query generation

[GitBox <gi...@apache.org>]

mayurnewase commented on a change in pull request #12482:
URL: https://github.com/apache/superset/pull/12482#discussion_r559077178



##########
File path: superset/connectors/sqla/models.py
##########
@@ -871,102 +908,76 @@ def _get_sqla_row_level_filters(
                 _("Error in jinja expression in RLS filters: %(msg)s", msg=ex.message,)
             )
 
-    def get_sqla_query(  # pylint: disable=too-many-arguments,too-many-locals,too-many-branches,too-many-statements
-        self,
-        metrics: List[Metric],
-        granularity: str,
-        from_dttm: Optional[datetime],
-        to_dttm: Optional[datetime],
-        columns: Optional[List[str]] = None,
-        groupby: Optional[List[str]] = None,
-        filter: Optional[  # pylint: disable=redefined-builtin
-            List[Dict[str, Any]]
-        ] = None,
-        is_timeseries: bool = True,
-        timeseries_limit: int = 15,
-        timeseries_limit_metric: Optional[Metric] = None,
-        row_limit: Optional[int] = None,
-        row_offset: Optional[int] = None,
-        inner_from_dttm: Optional[datetime] = None,
-        inner_to_dttm: Optional[datetime] = None,
-        orderby: Optional[List[Tuple[ColumnElement, bool]]] = None,
-        extras: Optional[Dict[str, Any]] = None,
-        order_desc: bool = True,
-    ) -> SqlaQuery:
-        """Querying any sqla table from this common interface"""
-        template_kwargs = {
-            "from_dttm": from_dttm.isoformat() if from_dttm else None,
-            "groupby": groupby,
-            "metrics": metrics,
-            "row_limit": row_limit,
-            "row_offset": row_offset,
-            "to_dttm": to_dttm.isoformat() if to_dttm else None,
-            "filter": filter,
-            "columns": [col.column_name for col in self.columns],
-        }
-        is_sip_38 = is_feature_enabled("SIP_38_VIZ_REARCHITECTURE")
+    def _update_template_kwargs(self, template_kwargs: Dict[str, Any]) -> None:
         template_kwargs.update(self.template_params_dict)
-        extra_cache_keys: List[Any] = []
-        template_kwargs["extra_cache_keys"] = extra_cache_keys
-        template_processor = self.get_template_processor(**template_kwargs)
-        db_engine_spec = self.database.db_engine_spec
-        prequeries: List[str] = []
 
-        orderby = orderby or []
+    def _get_columns_by_name(self) -> Dict[str, TableColumn]:
+        return {col.column_name: col for col in self.columns}
 
-        # For backward compatibility
-        if granularity not in self.dttm_cols:
-            granularity = self.main_dttm_col
+    def _get_metrics_by_name(self) -> Dict[str, SqlMetric]:
+        return {m.metric_name: m for m in self.metrics}
 
-        # Database spec supports join-free timeslot grouping
-        time_groupby_inline = db_engine_spec.time_groupby_inline
-
-        columns_by_name: Dict[str, TableColumn] = {
-            col.column_name: col for col in self.columns
-        }
-        metrics_by_name: Dict[str, SqlMetric] = {m.metric_name: m for m in self.metrics}
-
-        if not granularity and is_timeseries:
-            raise QueryObjectValidationError(
-                _(
-                    "Datetime column not provided as part table configuration "
-                    "and is required by this type of chart"
-                )
-            )
-        if (
-            not metrics
-            and not columns
-            and (is_sip_38 or (not is_sip_38 and not groupby))
-        ):
-            raise QueryObjectValidationError(_("Empty query?"))
-        metrics_exprs: List[ColumnElement] = []
+    def _get_metric_expressions(
+        self,
+        metrics: List[Metric],
+        columns_by_name: Dict[str, TableColumn],
+        metrics_by_name: Dict[str, SqlMetric],
+    ) -> List[Label]:

Review comment:
       Moved few,but these use class attributes/other class in this file in some way,which produce circular imports.




----------------------------------------------------------------
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.

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