vector: add support for vector type #5
Conversation
There was a problem hiding this comment.
I know that this is work in progress, but leaving some comments anyway.
Please use Fixes instead of Closes in the PR description. We use "Fixes" for referring to issues that will be fixed by the PR; we use "Closes" in merge commits to close PRs when they are merged to master.
There was a problem hiding this comment.
The PR is still a draft, but you should squash "fix ..." commits with commits which introduce the need to do the fix.
Fix means there existed a bug in the code and now you are fixing it. You shouldn't do a bug/todo in one commit and fix it later in the same PR.
QuerthDP
force-pushed
the
2-implement-vector-data-type
branch
from
22f0cd4 to
113c5e4
Compare
|
Changelog:
|
|
test report from running |
QuerthDP
force-pushed
the
2-implement-vector-data-type
branch
from
12b7aef to
83604d7
Compare
|
Changelog:
|
There was a problem hiding this comment.
The code and the tests in the PR are comprehensive and should be generally fine. What should be improved is 1) commit messages 2) structure of the PR 3) documentation.
The standard formatting rule for the commit messages is: try to keep the title up to max 50 characters, then keep the lines up to 72 characters max. Please format your commit messages according to it (stick at least to the second rule).
Please use the imperative mood, or at least past tense in your commit messages. Try to avoid writing in first person when you say what your commit is doing - think about commits as entities which introduce changes themselves. So, "Add this and this to the code" is recommended. Some people (me included, sometimes) use the form "This commit adds this and this to the code", which is not imperative mood but better fits the narrative about commits doing things. It is fine to use first person when talking about the motivation behind the commit, though.
About the documentation: please look around in the documentation directory, but right away I can tell you that you will have to update at least docs/cql/types.rst. Please follow the instructions in docs/README.md to learn how to build the documentation locally (protip: if you encounter some keyring-related problems with running the poetry command, I found out that defininng the PYTHON_KEYRING_BACKEND=keyring.backends.null.Keyring environment variable helps).
QuerthDP
force-pushed
the
2-implement-vector-data-type
branch
from
df2a84d to
fae9759
Compare
|
Changelog:
|
Implement serialization, deserialization, comparison, JSON and Lua support, and other functionalities. Co-authored-by: Dawid Pawlik <501149991dp@gmail.com>
Introduce vector_type CQL syntax: VECTOR<`cql_type`, `integer`>. The parameters are respectively a type of elements of the vector and the vector's dimension (number of elements). Co-authored-by: Jan Łakomy <janpiotrlakomy@gmail.com>
This change is introduced due to lack of support for vector class name, used by type_parser to create data_type based on given class name (especially compound class name with inner types or other parameters). Add function that parses vector type parameters from a class name.
Contains two type_parser tests: one for a valid vector and another for invalid vector.
These tests check serialization and deserialization (including JSON), basic inserts and selects, aggregate functions, element validation, vector usage in user defined types and functions. test_vector_between_user_types is a translated Apache Cassandra test to check if it is handled properly internally.
Motivation for this changes is to provide a distinguishable interface for vector type expressions. The square bracket literal is ambigious for lists and vectors, so that we need to perform a distinction not using CQL layer. At first we should use the collection constructor to manage both lists and vectors (although a vector is not a collection). Later during preparation of expressions we should be able to get to know the exact type using given receiver (column specification). Knowing the type of expression we may use their respective constructor (in this case the vector constructor being introduced), which would make the implementation more precise. In particular, this separates the implementation from collections, which could've been quite misleading and inelegant. This commit introduces vector constructor, functions making use of it, and it's necessary visitor appearances. However vector constructor is not yet used anywhere, the next commit should adjust collection constructor and make use of the vector constructor and it's features.
Like mentioned in the previous commit, this changes introduce usage of vector constructor and adjusts the functions using collection constructor to distinguish vectors from lists. Rename collection constructor style list to list_or_vector.
Add and adjust tests using vector_constructor and collection_constructor with list_or_vector style before preparation. Implemented utilities used in expr_test similar to those added in 8f6309b.
This change has been introduced to enable CQL drivers to recognize vector type in query results. The encoding has been imported from Apache Cassandra implementation to match Cassandra's and latest drivers' behaviour. Co-authored-by: Dawid Pawlik <501149991dp@gmail.com>
Add cql_vector_test which tests the basic functionalities of the vector type using CQL. Add vectors_test which tests if descending ordering of vector is supported.
Add missing vector type documentation including: definition of vector, adjustment of term definition, JSON encoding, Lua and cql3 type mapping, vector dimension limit, and keyword specification.
janpiotrlakomy
force-pushed
the
2-implement-vector-data-type
branch
from
3dfcd3d to
209d47d
Compare
|
Changelog:
|
This pull request is an implementation of vector data type similar to one used by Apache Cassandra.
The patch contains:
Fixes #2