[chrisvittal]

It has been supplanted by other struct STypes for projections. And it's
not used at current and cannot be constructed outside of the parser. It
was never serialized in a durable fashion and so its removal is safe.

Security Assessment

• This change cannot impact the Hail Batch instance as deployed by Broad Institute in GCP

[coderabbitai]

Walkthrough

The PSubsetStruct physical type and its parser syntax were removed. PhysicalTestUtils now uses sourceType to create the source region value.

Changes: PSubsetStruct removal

Layer / File(s)	Summary
Remove PSubsetStruct support hail/hail/src/is/hail/types/physical/PSubsetStruct.scala, hail/hail/src/is/hail/expr/ir/Parser.scala, hail/hail/test/src/is/hail/types/physical/PhysicalTestUtils.scala	The PSubsetStruct implementation and parser branch were removed. PhysicalTestUtils now creates the source region value using sourceType.

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Refactor

Possibly related PRs

• hail-is/hail#15868: Updates the IRParser.ptype_expr contract used by the parser branch removed here.
• hail-is/hail#15872: Updates PSubsetStruct method signatures before the type is removed here.

Merge Risk: Minimal · up to f4b36

No concrete merge-blocking regression is established. Compatibility with older files containing serialized PSubsetStruct values remains unverified.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly identifies the main change: removal of PSubsetStruct.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description gives a short rationale: PSubsetStruct was supplanted by other struct STypes for projections, is unused, could only be built inside the parser, and was never serialized durably, so rem…

Full details: Description check

Explanation

The description gives a short rationale: PSubsetStruct was supplanted by other struct STypes for projections, is unused, could only be built inside the parser, and was never serialized durably, so removal is safe. It also includes the Security Assessment section with the 'cannot impact' option selected. The template's Change Description section has no linked issue and no 'Fixes #' line, and the author did not delete that placeholder. The Impact Rating, Impact Description, and Appsec Review sections were correctly removed because the change has no impact. The rationale is brief, but it covers the required points for a removal PR, so the description is mostly complete.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[chrisvittal]

This deals with the resolved wontfix comment by the bot on #15665. The model objected to the potential possibility of a double field in the subset struct. This removes that possibility.

[coderabbitai]

Additional comments: 1

hail/hail/src/is/hail/expr/ir/Parser.scala-457-457: 1

457-457: Minor · Potential issue · Heavy lift

Unverified finding
We couldn't verify this from the code. It is shown for review, not as a verified issue.

Deleting the PSubsetStruct parser branch may make older serialized physical-type strings unreadable through IRParser.parsePType or another caller of ptype_expr.

The file read fails while parsing its type metadata.

This can occur when: An existing file contains a serialized physical type using PSubsetStruct syntax and is opened after the upgrade.

---

Review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 81b92b86-163d-4e81-8270-05de57acfd38

Commits

Reviewing files that changed from the base of the PR and between de32429 and f4b3622.

Files selected for processing: 3

• hail/hail/src/is/hail/expr/ir/Parser.scala
• hail/hail/src/is/hail/types/physical/PSubsetStruct.scala
• hail/hail/test/src/is/hail/types/physical/PhysicalTestUtils.scala

Files with no reviewable changes: 1

• hail/hail/src/is/hail/types/physical/PSubsetStruct.scala

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.