Add option to specify the schema for the table #26
Conversation
jgpruitt
left a comment
jgpruitt
left a comment
There was a problem hiding this comment.
nit: the doc comments need updating. Missing the schema parameter.
| | table_name | Name of the table to use for storing the embeddings. Think of this as the collection name | | ||
| | num_dimensions | Number of dimensions in the vector | | ||
|
|
||
| You can also specify the schema name, distance type, primary key type, |
There was a problem hiding this comment.
Why not adding it to the table above with a column marking it as optional?
There was a problem hiding this comment.
I want to keep this part as simple as possible
| """ | ||
| self.builder = QueryBuilder( | ||
| table_name, num_dimensions, distance_type, id_type, time_partition_interval, infer_filters) | ||
| table_name, num_dimensions, distance_type, id_type, time_partition_interval, infer_filters, schema_name) |
There was a problem hiding this comment.
Need to add schema_name to the docs. Would it make it easier if instead of None we default to "public"? That way we don't have to do the if self.schema_name is not None checks.
There was a problem hiding this comment.
None and public are not the same since public is not always the default schema. None means use the default schema defined by the db
| time_partition_interval: Optional[timedelta] = None, | ||
| max_db_connections: Optional[int] = None, | ||
| infer_filters: bool = True, | ||
| schema_name: Optional[str] = None, |
There was a problem hiding this comment.
Need to add this to the constructor docs
No description provided.