Fixing operations on Mixed types

[radeusgd]

Pull Request Description

• Fixes Table type checks now allow Mixed columns if all values satisfy a more precise type, but the Storage vectorized operations are not prepared for it #7231
• Cleans up vectorized operations to distinguish unary and binary operations.
• Introduces MixedStorage which may pretend to be a more specialized storage on demand.
• Ensures that operations request a more specialized storage on right-hand side to ensure compatibility with reported inferred storage type.
• Ensures that a dataflow error returned by an Enso callback in Java is propagated as a polyglot exception and can be caught back in Enso
• Tests for comparison of Mixed storages with each other and other types
• Started using Set for Filter_Condition.Is_In for better performance.
• Migrated Column.map and Column.zip to use the Java-to-Enso callbacks.
  • This does not forward warnings. IMO we should not be losing them. We can switch and add a ticket to fix the warnings, but that would be a regression (current implementation handles them correctly). Instead, we should first gain some ability to work with warnings in polyglot. I created a ticket to get this figured out Migrating Column.map and Column.zip to Java-to-Enso callbacks #7371
• Trying to avoid conversions when calling Enso functions from Java.
  • Needs extra care as dataflow errors may not be handled right then. So only works for simple functions that should not error.
  • Not sure how much it really helps. Benchmarks suggested it could improve the performance quite significantly, but the practical solution is not exactly the same as the one measured, so we may have to measure and tune it to get the best results.
  • Created Investigate if we can avoid Polyglot conversions and if it will be beneficial in practice #7378 to track this.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

• The documentation has been updated, if necessary.
• Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
• All code follows the
  Scala,
  Java,
  and
  Rust
  style guides. In case you are using a language not listed above, follow the Rust style guide.
• All code has been tested:
  • Unit tests have been written where possible.
  • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

[GregoryTravis]

Is this computing self.is_nothing twice?

[radeusgd]

Good observation!! Yes, apparently it is.
Well, if it's a LongStorage, then it is a O(1) operation, because it just re-uses the bitset used to mark missing values. But I guess better to cache it just to be sure.

[radeusgd]

No sorry, I'm reading the code again and realise it's not the case.

See how it's bracketed:

if treat_nans_as_blank 
	then self.is_nothing.iif True self.internal_is_nan 
	else self.is_nothing

So we can see that in each branch, is_nothing is computed once.

If only we had multiline if to make stuff like this more readable...

---

Only thing we could optimize here is to compute self.is_nothing.iif True self.internal_is_nan in a single pass, by creating a variant of internal_is_nan that will return True for null values. But that seems out of scope of this PR.

[radeusgd]

On occassion of limitting when the convertPolyglotValue helper is invoked (as the #7270 benchmarks showed that it accounts for a significant part of the runtime of various operations), I tried checking if it is even "still" needed.

After reminding myself of why it has been added in the first place, I conclude that it is needed and it will be needed whenever we are accepting date-time values, at least as long as we keep the architecture like the one we have currently.

1. The Graal Polyglot architecture does not auto-convert our Enso date-time values (EnsoDate, EnsoDateTime, EnsoTimeOfDay) into their Java counterparts, like it does for primitive types (converting Enso Text to Java String) and this is rather unlikely to change as it seems to be done on purpose.

The Graal polyglot values essentially can report isDate, isTime and isZone, specifying if they hold a date, a time-of-day and a timezone. A single value can hold any combination of these three. Graal cannot autoconvert our EnsoDateTime value (which reports true to all three) to ZonedDateTime. Technically, it probably could - but it does not do it. I hypothesize it may be because there may exsit different date-time implementations between languages, so there is no guarantee that such conversion would be lossless (in Enso its essentially a wrapper, so it would be, but that is not guaranteed in any language) and so it is not certain if it should be performed - so Graal leaves that decision to the consumer of the value.

2. Our current architecture stores date-time values in in-memory Table Columns as Java values (LocalDate, LocalTime and ZonedDateTime). So whenever we get an Object/Value from Enso, we need to inspect if it should be coerced to the date, because as noted in (1) it will not be auto-converted - and that is what convertPolyglotValue is doing.

If we wanted to get rid of convertPolyglotValue, we'd need to somehow change the architecture. Probably to be more Graal-friendly and instead store date/time columns as Object[] or Value[] and then convert to dates more 'lazily'. To get the date/time information from the value in Java we'd have to do the kind of conversion that convertPolyglotValue does anyway at some point and it is unclear whether doing it more lazily would be any better.

Alternatively, hypothetically, to make it 'faster' we could try getting rid of EnsoDate et al. and using raw LocalDate and other Java values in Enso - then the conversion would be necessary. That is of course not a great idea, since the whole EnsoDate wrappers were done for a reason, to get a more user friendly and Enso-centric API for Date. It would also not fully solve the problem, because we can still get date-time values into our columns that come from other languages (e.g. JS, Python).

All-in-all I don't think we can avoid convertPolyglotValue in general (if I'm missing something here and it is possible, I'd be happy to be proved wrong 🙂). What we can do is try running it only if it is really needed - when we are building columns of number where any value that is not a number will be rejected anyway, calling the conversion will be unnecessary. So we only need to call it if the value may be a date and won't be rejected if it is a date.

[jdunkerley]

Looks good and great to have this more sorted.
One slight name sort out please.

[jdunkerley]

shouldn't be the implementation of contains within Vector?

[radeusgd]

There is one, but using set will be faster.

This way here we construct the Set (O(N)) operation once, and save that set in the closure of the generated function, so then we can use the quick (~O(1), hashing-based) set checks.

[jdunkerley]

this is getting very long...
I suggest we drop supports_ from names. Not necessarily in this PR!