Describe the bug
In extract_raw_features in the QualX default featurizer, the branch taken when node_level_supp is supplied and qualtool_filter is stage assigns the supported-stage subset to sql_job_agg_tbl, and the next statement immediately reassigns sql_job_agg_tbl from the unfiltered job_stage_agg_tbl before the groupby. The subset is therefore only ever read by the emptiness check above it, and the aggregated features are computed over supported and unsupported stages alike, contrary to the comment on that branch.
Unsupported stages really do reach that frame: stages_supp keeps rows with Exec Is Supported false, and the join that attaches the column is inner only on stage identity. So this is not a redundant filter that some upstream step already applied.
Steps/Code to reproduce bug
No runtime reproduction. This is a code-inspection finding.
if node_level_supp is not None and (qualtool_filter == 'stage'):
# if supported exec info supplied aggregate features only over supported stages
sql_job_agg_tbl = job_stage_agg_tbl.loc[job_stage_agg_tbl['Exec Is Supported']]
if sql_job_agg_tbl.empty:
log_fallback(logger, unique_app_ids, fallback_reason='No fully supported stages found')
return pd.DataFrame(columns=list(expected_raw_features))
# aggregate using reduce ops, recomputing duration_mean
sql_job_agg_tbl = job_stage_agg_tbl.groupby(
['appId', 'appName', 'sqlID'], as_index=False
).agg(job_stage_reduce_cols)The filter is not entirely inert. It still decides the fallback, and because the frame is concatenated across every app in the batch, that guard fires only when no stage anywhere in the batch is supported, and then aborts the whole batch. When at least one supported stage exists it does not restrict the aggregation at all.
Expected behavior
This is the question the issue needs answered, because the comment and the code disagree and either could be the mistake. If the filter was meant to apply, the groupby should read sql_job_agg_tbl and this is a one-line fix. If it was deliberately abandoned, the comment should go instead. Which did you intend?
One piece of evidence, not proof: as ported in #1076 this branch had a matching else: sql_job_agg_tbl = job_stage_agg_tbl, which is a dead store unless the following groupby was meant to read sql_job_agg_tbl. The shape has been unchanged since.
Note that fraction_supported is derived from the same column and does reach the features, so the model is not blind to supportedness.
Environment details (please complete the following information)
- Environment location: not applicable. This is the Python
user_tools QualX featurizer and does not depend on the Spark deployment.
Additional context
Found while tracing the QualX consumption path for the duration_min change in #2154, not by touching this code. Unrelated to that fix and pre-existing. Raising it separately for the QualX owners; severity is unmeasured, since whether it matters depends on how often unsupported stages carry materially different metrics, which nobody has checked.
Describe the bug
In
extract_raw_featuresin the QualX default featurizer, the branch taken whennode_level_suppis supplied andqualtool_filterisstageassigns the supported-stage subset tosql_job_agg_tbl, and the next statement immediately reassignssql_job_agg_tblfrom the unfilteredjob_stage_agg_tblbefore the groupby. The subset is therefore only ever read by the emptiness check above it, and the aggregated features are computed over supported and unsupported stages alike, contrary to the comment on that branch.Unsupported stages really do reach that frame:
stages_suppkeeps rows withExec Is Supportedfalse, and the join that attaches the column is inner only on stage identity. So this is not a redundant filter that some upstream step already applied.Steps/Code to reproduce bug
No runtime reproduction. This is a code-inspection finding.
The filter is not entirely inert. It still decides the fallback, and because the frame is concatenated across every app in the batch, that guard fires only when no stage anywhere in the batch is supported, and then aborts the whole batch. When at least one supported stage exists it does not restrict the aggregation at all.
Expected behavior
This is the question the issue needs answered, because the comment and the code disagree and either could be the mistake. If the filter was meant to apply, the groupby should read
sql_job_agg_tbland this is a one-line fix. If it was deliberately abandoned, the comment should go instead. Which did you intend?One piece of evidence, not proof: as ported in #1076 this branch had a matching
else: sql_job_agg_tbl = job_stage_agg_tbl, which is a dead store unless the following groupby was meant to readsql_job_agg_tbl. The shape has been unchanged since.Note that
fraction_supportedis derived from the same column and does reach the features, so the model is not blind to supportedness.Environment details (please complete the following information)
user_toolsQualX featurizer and does not depend on the Spark deployment.Additional context
Found while tracing the QualX consumption path for the
duration_minchange in #2154, not by touching this code. Unrelated to that fix and pre-existing. Raising it separately for the QualX owners; severity is unmeasured, since whether it matters depends on how often unsupported stages carry materially different metrics, which nobody has checked.