Skip to content

fix: ignore structural tags when lifting expression coverage - #5471

Draft
sunchao wants to merge 1 commit into
apache:mainfrom
sunchao:codex/coverage-tag-isolation
Draft

fix: ignore structural tags when lifting expression coverage#5471
sunchao wants to merge 1 commit into
apache:mainfrom
sunchao:codex/coverage-tag-isolation

Conversation

@sunchao

@sunchao sunchao commented Aug 26, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

No issue is automatically closed. This is a standalone follow-up to the existing structural-expression tag filtering.

Rationale for this change

Expression rewrites lift coverage tags from a rebuilt tree to its original owner. The lift currently includes tags on structural nodes, so stale tags on a shared literal can be copied onto an alias and appear as legitimate native or codegen-dispatch coverage in extended explain output.

What changes are included in this PR?

  • Reuse CometExplainInfo.collectExprTagValues to filter structural nodes before lifting either coverage-tag category.
  • Keep the helper private and preserve real expression coverage.
  • Extend the existing shared-TrueLiteral regression through decimal promotion. It checks that real checkoverflow coverage survives while planted native and dispatch tags do not reach the alias or unrelated plan.

Only QueryPlanSerde and the existing CometCodegenSuite change; query execution semantics are unchanged.

How are these changes tested?

Draft: JVM validation is blocked, so no passing JVM test result is claimed.

  • Built a matching native library from public main 5baa6b03.
  • Spotless checks on the final public patch and git diff --check passed.
  • Runtime validation is pending because the public Maven dependency resolution is blocked in the local environment. No repository configuration was changed.

Intended focused check:

./mvnw test -Pspark-4.0 -Dtest=none \
  -Dsuites=org.apache.comet.CometCodegenSuite \
  '-Dtests=tags copied onto the shared TrueLiteral,expression coverage stats'

The expanded singleton regression and existing expression-coverage tests still need to be run before this draft is ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant