[query] add sum-type `VType` representing types associated with `BaseIR`

hail-is / hail

Cloud-native genomic dataframes and batch computing

https://hail.is

MIT License

974 stars 243 forks source link

[query] add sum-type `VType` representing types associated with `BaseIR` #14678

Closed ehigham closed 2 weeks ago

ehigham commented 3 weeks ago

Adds VType - a useful abstraction for the compiler front-end to differentiate between the types associated with IRs and other instances of BaseType (PType, EType, etc).

ehigham commented 3 weeks ago

The changes around toJSON seem to be unrelated to the introduction of VType. Can you explain the motivation there?

This was added as a step towards a greater refactoring effort where I applied a number of changes to try and make various backend implementations look the same. Type, TableType and MatrixType had a toJSON method, with the exception of BlockMatrixType. Since these are all virtual types, it seemed like a simple change to unify these methods.

ehigham commented 2 weeks ago

Just requesting changes pending a response to Chris's comments. Otherwise this looks ready to go.

I'm going to submit a follow-up pr to this to address the grammar, it's quite fragile and I'd like a dedicated review to that work