Handle Varchar(n) in Velox

[aditi-pandit]

Both Presto and Spark allow specifying an optional max length for varchar type
https://prestodb.io/docs/current/language/types.html#string
https://spark.apache.org/docs/latest/sql-ref-datatypes.html

Velox Varchar type doesn't support max length semantics.

Right now, Presto always uses Velox varchar irrespective if the varchar has a max length. This causes incorrect results for many SQL.

presto:tpch> select cast(varchar_value as varchar(3)) from (values ('9898.122')) t(varchar_value);
_col0
9898.122

Note:-> This is incorrect output. The correct result should be 989

I tried to error out such invalid CAST expressions at the Prestissimo layer, but that failed many SQL queries in the native tests. So I feel doing so is very restrictive. It would be better to support varchar(n) natively in Velox.

Starting this discussion to understand :

• Why Velox doesn't support the max length semantics ? There is the required work of adding a new type, casting functions etc. But is there a more fundamental limitation of Velox to support max length semantics ?
• The complexity to support the max length in the future.

@mbasmanova @pedroerp @oerling

[aditi-pandit]

@tdcmeehan brought up an interesting Presto discussion around char and varchar collation semantics related to blank padding.
prestodb/presto#21001. This is an issue with varchar as well.

[pedroerp]

@mbasmanova I remember a similar discussion back in the day, but not sure if there was a technical reason why we decided not add support for it, or just something we haven't had time to look at.

More generally, I guess in order to support this we would need a fixed-length array/string vector type. There was an initial implementation for something similar back in the day which someone created to support tensors, but I don't think we it was properly hooked up around the codebase.

@mbasmanova do you know if we removed that code?

[mbasmanova]

FixedSizeArray used to be a specialization of an ArrayType. We should not use that to implement char(n) because it would be quite inefficient to model char(n) as an array of single-byte entries.

Instead, I imagine char(n) and varchar(n) would be logical types based on VARCHAR physical type.

The first bulk of work is to identify semantic differences between char(n), varchar(n) and varchar; document these and come up with a design.

[aditi-pandit]

@mbasmanova : Thanks for clearing the understanding around FixedSizeArray. (I presumed it was for fixed n-size entries in an array). We definitely don't want to go by addressing single byte entries.

I'm glad to know the reservations for adding this type in Velox is from the amount of work in identifying semantic differences between char(n) and varchar(n) and varchar.

The high level steps for this work could be :

• Identify the semantic differences between char(n), varchar(n) and varchar.
• Understanding the plumbing to wire them as logical types of VARCHAR.
• Identify optimizations around memory allocations and addressing on account of the fixed length characteristic.
• If worthwhile, abstract a physical type (leveraging the optimizations above) that can provide APIs like velox::StringView. Make char(n) and varchar(n) logical types of this new physical type.

@czentgr

[mbasmanova]

If worthwhile, abstract a physical type that can provide APIs like velox::StringView. Make char(n) and varchar(n) logical types of this new physical type.

Curious what is this about. Would you elaborate a bit?

Let's also add an item to add support to Fuzzers for the new types.

[mbasmanova]

@aditi-pandit One more item is to add support for decimal and char(n), varchar(n) types to SimpleFunction API. It doesn't support decimal types right now and it makes things difficult. Having to write all function using these types as VectorFunctions won't scale. Maybe step 1 would be to figure out how to add support for decimal types.

[mbasmanova]

PR #9096 added support for decimal types to Simple Function API.

[aditi-pandit]

If worthwhile, abstract a physical type that can provide APIs like velox::StringView. Make char(n) and varchar(n) logical types of this new physical type.

Curious what is this about. Would you elaborate a bit?

@mbasmanova : If we decide that the optimizations for memory allocation or addressing warrant a new physical type, then my thinking was that if we expose this physical type via a StringView like API, then it would be easy to flip any existing logic written using StringView (and VARCHAR type) to the new physical type. This might be overthinking for now :)

[mbasmanova]

If we decide that the optimizations for memory allocation or addressing warrant a new physical type

What kind of optimizations you are thinking about? Would you elaborate a bit?

[czentgr]

I've put together a document about the support. I've made a prototype with the changes and still working some aspects around the fuzzer. If you get a chance please take a look.
https://docs.google.com/document/d/1wsNcS3Op2E5yyW9uH28gNtzXpJqvUHE6Ds4Xi_w9w54/edit?usp=sharing

[mbasmanova]

@czentgr Hi Christian, thank you for putting together a design document and sharing it. I left some comments.

[czentgr]

Thank you for your quick review! I will work through your comments.