Fix: Rename "table" to "table_name" and switch to REGCLASS for better schema handling #163
Conversation
|
To ensure backward compatibility I have added into new migration file |
|
@ChuckHend I am not pretty about schema param removal. Can you check it out that what you are expecting? |
What i am wondering is if we still need the schema parameter, or can we determine schema from the oid? |
| @@ -21,6 +21,7 @@ lazy_static = "1.4.0" | |||
| log = "0.4.21" | |||
| ollama-rs = "=0.2.1" | |||
| pgmq = "0.29" | |||
| pgrx = "=0.12.5" | |||
There was a problem hiding this comment.
remove pgrx from ./core/Cargo.toml. Core is separate so we can have a library that has no dependency on pgrx or any postgres service side.
There was a problem hiding this comment.
for PgOid we need it if we support
There was a problem hiding this comment.
the only place PgOid type would be used is on the vectorize.table() outer most function, in the signature and then using it to determine table and schema, right? everything else could refer to either schema.table as a string or schema and table as separate strings would be ok too i think.
|
But removing schema param going on rabbit hole |
| @@ -239,7 +239,7 @@ fn append_embedding_column(job_name: &str, schema: &str, table: &str, col_type: | |||
| ) | |||
| } | |||
|
|
|||
| pub fn get_column_datatype(schema: &str, table: &str, column: &str) -> Result<String> { | |||
| pub fn get_column_datatype(table: &str, column: &str) -> Result<String> { | |||
There was a problem hiding this comment.
i think here we still need the schema parameter so that we can determine the data type of the specified column. otherwise, if two tables with same name but different schema, we will get multiple values in the result.
|
@ChuckHend I think we need the schema parameter. Can I proceed with only renaming the table? |
| @@ -28,14 +27,16 @@ pub fn init_table( | |||
| // cron-like for a cron based update model, or 'realtime' for a trigger-based | |||
| schedule: &str, | |||
| ) -> Result<String> { | |||
| let table_name_str = pg_oid_to_table_name(table_name); | |||
There was a problem hiding this comment.
I am hoping that we can determine the schema from the oid too. so maybe we have a let schema = pg_oid_to_schema() or something similar here
can the |
Closes #145
/claim #145