nit:

if (left.isNumeric() && right.isNumeric())

then remove Preconditions.checkState on the next line.

done

nit: new line

done

nit: new line.

done

nit: testCaseTransformFunctionWithFloatResults -> testCaseTransformFunctionWithDoubleResults

done

It is not clear to me what is the difference between _value and _bigDecimalValue in each case.

• If literal.getSetField is DOUBLE_VALUE or LONG_VALUE, _value is either a Double or a Long.
• If literal.getSetField is STRING_VALUE:
  • And the literal is an actual number (like "123" or "1.3"), then _value is the more specific implementation Number possible.
  • And the literal is not an actual number:
    * If it is a boolean, _value is a String like "true" or "false" and _bigDecimanValue is 1 or 0.
    * If it is a timestamp, _value is a String (why?) like "21312312" and _bigDecimanValue is the representation as a BigDecimal.

Then _bigDecimalValue is only used in getXValue where X is Int, Double or BigDecimal.

For the context, BigDecimal instances are quite large. They contain several attributes and therefore they consume a significant amount of memory. I guess normal queries should not have tons of instances of LiteralContext, but we will receive degenerated queries like where X in [list with hundreds/thousand of elements] or where X = L1 or X = L2 or ... X = L1000. I had to deal with these cases in other databases and suddenly inefficiencies that looked innocent imply OOMs.

It also seems that getXValue are always being called in the initialization of the transformation functions. Therefore they are not in the hotpath and catching the BigDecimal doesn't seem to be that useful. This is why I would recommend to:

• In getIntValue and getDoubleValue:
  • When _value is instance of Number, cast it and call the appropriate method.
  • Otherwise, calculate the value lazily each time the method is called.
• In getBigDecimalValue case I would just calculate the value lazily each time the method is called.

By doing that we may reduce the amount of memory used in degenerated queries like the ones I listed above.

PS: I edited the comment because in the original text I was assuming getXValue was called in the hotpath. After reading the rest of the PR I would say it is not the case.

_bigDecimalValue is a cached representation for numerical types. We did the same cache in LiteralTransform func so we don't need to calculate numerical representation of value multiple times.

Correct me if I'm wrong, but LiteralContext.getXValue is only called once from LiteralTransform constructor/init, right? Then we don't need to catch it.

I think LiteralTransform shouldn't catch all the values just in case someone needs them. I'm not saying we should change LiteralTransform asap, but it would be great to do not replicate antipatterns

not really. for example, LiteralContext.getIntValue() gets called in HistogramAggregationFunction. that's why I cache it.

Why this change in the hash code?

because originally we are not table to tell "123" and 123. After the type reservation, "123" and 123 are different. thus we add the type encoding into the hashcode.

because originally we are not table to tell "123" and 123. After the type reservation

Yeah, but I mean... why do we add the hash of types? That isn't usual when producing hashes. In fact if you do return Objects.hashCode(_value, _type) it will produce better hashCode (at the cost of creating a short living array).

good point. changed to Objects.hash(...)

nit: In order to reduce the number of changes... why not just change this code to call _literal.getStringValue()?

because we want caller to explicitly specify what type they want to use. so they have to use getLiteral().getTypeLiteral()