Support vector value type - #456
Conversation
abd2cb6 to
64ce513
Compare
| expression_struct = { CURLY_OPEN ~ struct_key ~ COLON ~ struct_value ~ CURLY_CLOSE } | ||
| struct_value = { expression_value | expression_struct } | ||
|
|
||
| vector_literal = { VECTOR ~ PAREN_OPEN ~ expression_list ~ COMMA ~ vector_precision ~ PAREN_CLOSE } |
There was a problem hiding this comment.
If not function, why function shaped :(
There was a problem hiding this comment.
@krishnangovindraj rust has the same:
// 1. Definition
struct Color(i32, i32, i32);
struct Point(f64, f64);
There was a problem hiding this comment.
as a reader i don't find it confusing as it's easy to differentiate them based on the context. in schema declaration where a type is expected, seeing vector(64, "float32") immediately tells me that 64 and "float32" is a type parameter of vector rather than arguments to a function in a function call.
There was a problem hiding this comment.
But this is the context for an expression rather than a type-declaration. It's not crazy to overload syntax, but this looks like it is just a built-in function named vector? And maybe it should be, because:
- It's not grouped with the other
value_literals, so it presumably can't be used in contexts where they can. - The first argument is an
expression_list, so it's going to be sent to theExpressionExecutorall the time anyway. - It's technically not a "literal" if it has to be evaluated.
There was a problem hiding this comment.
A side effect of this is that nonsense like vector([2024-12-12], "supercalifragilisticexpialidocious"); is not rejected by this grammar. It will fail to parse as a vector_literal, fall back to an expression_function, and the user gets an error saying Could not resolve function with name 'vector'. instead of a more useful error message like Built-in function 'vector' cannot be applied to ....
If the vector literal constructor is function-like, I don't think it belongs in the grammar.
| let span = node.span(); | ||
| let mut children = node.into_children(); | ||
| children.skip_expected(Rule::VECTOR); | ||
| let list = visit_expression_list(children.consume_expected(Rule::expression_list)); |
There was a problem hiding this comment.
This will return a vector of expressions to the server yes? Do you want to do the validation there? I don't see any we wouldn't allow it, but just to be certain - you'll allow things like vector([1+1, 5.0], "float32") yes?
| value_type_optional = { value_type ~ QUESTION } | ||
| value_type_list = { value_type ~ SQ_BRACKET_OPEN ~ SQ_BRACKET_CLOSE } | ||
| value_type_vector = { VECTOR ~ PAREN_OPEN ~ integer_literal ~ COMMA ~ vector_precision ~ PAREN_CLOSE } | ||
| vector_precision = @{ "\"" ~ "float32" ~ "\"" } |
There was a problem hiding this comment.
"float32" (etc. down the line) should be separate rules in the pest grammar, and vector_precision should be compound atomic ${ }.
We already have the parser, we shouldn't need to trim its outputs.
| expression_struct = { CURLY_OPEN ~ struct_key ~ COLON ~ struct_value ~ CURLY_CLOSE } | ||
| struct_value = { expression_value | expression_struct } | ||
|
|
||
| vector_literal = { VECTOR ~ PAREN_OPEN ~ expression_list ~ COMMA ~ vector_precision ~ PAREN_CLOSE } |
There was a problem hiding this comment.
A side effect of this is that nonsense like vector([2024-12-12], "supercalifragilisticexpialidocious"); is not rejected by this grammar. It will fail to parse as a vector_literal, fall back to an expression_function, and the user gets an error saying Could not resolve function with name 'vector'. instead of a more useful error message like Built-in function 'vector' cannot be applied to ....
If the vector literal constructor is function-like, I don't think it belongs in the grammar.
Product change and motivation
Adds support for declaring
vector. This lets users work with embeddings, and later express vector-similarity queries.givensyntaxImplementation
BuiltinValueTypeArraystruct and visitors