rusackas commented on code in PR #37516:
URL: https://github.com/apache/superset/pull/37516#discussion_r3657702766
##########
superset/common/query_context_processor.py:
##########
@@ -396,9 +475,362 @@ def get_payload(
},
self.get_cache_timeout(),
)
- return_value["cache_key"] = cache_key # type: ignore
+ return QueryContextExecutionResult(
+ queries=query_results,
+ cache_key=cache_key,
+ context_cache_write_outcome=context_cache_write_outcome,
+ )
- return return_value
+ def _execute_query_plan(
+ self,
+ force_cached: bool,
+ materialize: bool,
+ ) -> tuple[QueryDataResult, ...]:
+ """Execute one dependency-aware plan for every response mode."""
+
+ contribution_plan = self._contribution_plan()
+ requested_result_types = {
+ query_idx: get_effective_result_type(self._query_context, query)
+ for query_idx, query in enumerate(self._query_context.queries)
+ }
+ contribution_dependencies = self._acquire_contribution_dependencies(
+ contribution_plan,
+ requested_result_types,
+ force_cached,
+ materialize,
+ )
+ output_acquisitions = dict(contribution_dependencies.reusable_outputs)
+ query_results: dict[int, QueryDataResult] = {}
+
+ for query_idx, query_obj in enumerate(self._query_context.queries):
+ if query_idx in output_acquisitions:
+ continue
+ result_type = requested_result_types[query_idx]
+ if not is_data_result_type(result_type):
+ query_results[query_idx] = (
+ get_query_results_with_timing(
+ result_type,
+ self._query_context,
+ query_obj,
+ force_cached,
+ )
+ if materialize
+ else get_query_results_cache_only(
+ result_type,
+ self._query_context,
+ query_obj,
+ )
+ )
+ continue
+
+ totals_idx =
contribution_dependencies.consumer_to_producer.get(query_idx)
+ totals = (
+ contribution_dependencies.totals_by_producer.get(totals_idx)
+ if totals_idx is not None
+ else None
+ )
+ if totals_idx is not None and (
+ totals is None
+ or not self._totals_support_consumer(query_obj, totals.values)
+ ):
+ query_results[query_idx] = self._dependency_failed_result(
+ _("Contribution totals query failed")
+ )
+ continue
+ execution_query = (
+ self._with_contribution_totals(
+ query_obj,
+ totals.values,
+ )
+ if totals is not None
+ else query_obj
+ )
+ acquired = acquire_query_data(
+ result_type,
+ self._query_context,
+ execution_query,
+ force_cached if materialize else False,
+ detect_currency_value=materialize,
+ cache_key_extra=(
+ {"contribution_totals_cache_key": totals.cache_key}
+ if totals is not None
+ else None
+ ),
+ )
+ if acquired is not None:
+ output_acquisitions[query_idx] = acquired
+ continue
+ raise QueryObjectValidationError(
+ _("Data-backed result type did not produce a dataframe")
+ )
+
+ for query_idx, acquired in output_acquisitions.items():
+ query_results[query_idx] = (
+ materialize_acquired_query(self._query_context, acquired)
+ if materialize
+ else cache_acquired_query(acquired)
+ )
+ return tuple(
+ query_results[query_idx]
+ for query_idx in range(len(self._query_context.queries))
+ )
+
+ def _acquire_contribution_dependencies(
+ self,
+ contribution_plan: dict[int, int],
+ requested_result_types: dict[int, ChartDataResultType],
+ force_cached: bool,
+ materialize: bool,
+ ) -> _ContributionDependencies:
+ """Acquire dataframe producers needed by data-backed consumers."""
+
+ data_plan: dict[int, int] = {
+ consumer_idx: producer_idx
+ for consumer_idx, producer_idx in contribution_plan.items()
+ if is_data_result_type(requested_result_types[consumer_idx])
+ }
+ reusable_outputs: dict[int, AcquiredQuery] = {}
+ totals: dict[int, _ContributionTotals] = {}
+ for producer_idx in sorted(set(data_plan.values())):
+ producer = self._query_context.queries[producer_idx]
+ requested_result_type = requested_result_types[producer_idx]
+ dependency_query = self._totals_query(producer_idx)
+ reuse_for_output = (
+ requested_result_type in _REUSABLE_CONTRIBUTION_RESULT_TYPES
+ and self._same_acquisition_identity(producer, dependency_query)
+ )
+ if reuse_for_output:
+ dependency_query = producer
+ dependency_result_type = requested_result_type
+ else:
+ dependency_query.result_type = ChartDataResultType.FULL
+ dependency_result_type = ChartDataResultType.FULL
+ acquired = acquire_query_data(
Review Comment:
I think this one's backwards. `dependency_query.result_type = FULL` only
runs in the `else` branch, and there `dependency_query` is the copy from
`_totals_query()` (`copy.copy`), not the producer. The producer only gets
aliased to `dependency_query` in the `reuse_for_output` branch, and that branch
never touches `.result_type`. So the original producer object in
`self._query_context.queries` isn't mutated here.
--
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]
