Value unification - #140
Conversation
|
I think I'm going to make another PR that removes the current coln-js-runtime on the trunk-for-2026-09-demo branch. |
There was a problem hiding this comment.
This looks really nice! Given this is a big refactoring, and does not have much functional change, perhaps merge it into main once it's ready, because it will affect almost all future work.
I skimmed through it, but the only functional difference is the liven_all method is that right? I think it otherwise looks good.
Also is the coln-js-runtime problem easy to fix?
Now the user should not need to call .into()
00ba94e to
c433986
Compare
| ir::Term::Lit { | ||
| lit: ir::Lit::Int { value }, | ||
| lit: ir::Lit::Int { | ||
| value: value.try_into().unwrap(), |
There was a problem hiding this comment.
@lstwn this is a bit unsatisfying, but Owen temporarily changed the value in the IR to be a i32, to make the JS side nice, I think. Seems to be a pretty substantial change to make all values in coln-query i32, so I did this. What do you think?
There was a problem hiding this comment.
Fine, it's just a test util and it's never called with a value that doesn't fit an i32 so we could also change call sites. But given this is just temporary, we can keep it like this.
This pull request unifies the various "value" enums into a single generic enum that looks like
Unfortunately, this means that some of the conversions that used to be done with
.intohave to be a little more explicit, because we can't implement a genericFrom<I>.We also rename the various
RowIdvariants to be a bit more consistent. The renaming is across the following axesTxnvs noTxn-- row ids that might appear temporarily as part of a transaction, or permanent row idsLivevs.Wirevs.Packed--Liveis row ids created as part of a transaction that automatically turn into permanent row ids after the transaction is done (which we will remove soon),Wireis row ids in the wire format (e.g. with commit hash), andPackedis row ids in the packed format (e.g. with commit id produced by an IdMapper).So what used to be
RowIdis nowWireRowId, what used to beRowHandleisTxnLiveRowId, what used to beRowRefisTxnWireRowId,PackedRowIdremains the same.As a convenience, we have type aliases
type XXXValue = Value<XXXRowId>, soWireValue = Value<WireRowId>, and so on.