feat: Support uuid as split column in postgres#180
feat: Support uuid as split column in postgres#180rajivharlalka wants to merge 6 commits intodatazip-inc:stagingfrom
Conversation
|
@hash-data any reviews please? |
|
@rajivharlalka reviewing today |
|
@hash-data any updates? |
|
@rajivharlalka can you also mention the cases in which you have tested it. |
hash-data
changed the title
|
|
|
@hash-data any further updates here? |
Add checks if the splitColumn is a UUID(string in olake) dattatype modify the SQL query to type-caste the value and column to TEXT for comparisons between uuid.
Signed-off-by: Rajiv Harlalka <rajivharlalka009@gmail.com>
Signed-off-by: Rajiv Harlalka <rajivharlalka009@gmail.com>
rajivharlalka
force-pushed
the
rajivharlalka/backfill-uuid
branch
from
8f51fec to
76054d3
Compare
|
@hash-data @vikash390 Any updates here? Could you test it out or should I help with a test PostgreSQL dump that could help. |
|
@rajivharlalka, we were busy with some ad hoc tasks and will try to finish testing today |
|
hey @rajivharlalka as we are approaching towards final merge, could you just resolve the comments so we can get it merged soon? |
Signed-off-by: Rajiv Harlalka <rajivharlalka009@gmail.com>
|
@hash-data do let me know if there is any more changes needed. |
| return fmt.Sprintf("%s AND %s", | ||
| formatter(filterColumn, ">=", chunk.Min), | ||
| formatter(filterColumn, "<=", chunk.Max)) | ||
| } |
There was a problem hiding this comment.
buildChunkCondition function has a problem. As of no, what it is doing is it includes both the chunk min and chunk max boundary in a single chunk, so there is data duplication. Basically the chunk min for every chunk is getting duplicated as the max of previous chunk is the min of current chunk.
Expected behaviour => When he first chunk is formed, only then the all the data between the chunk min and chunk max including must be in that chunk, from next chunk onwards the chunk min (which was chunk max of previous chunk) should not be included in the chunk. It should only be included in the first chunk made.
There was a problem hiding this comment.
olake/drivers/postgres/internal/backfill.go
Lines 32 to 34 in 5687fd0
I feel this is an already understood problem and it's solution isn't in the buildChunk function. I read the todo and hence left the known problem on the idea that it'll get fixed later.
There was a problem hiding this comment.
This is already solved. We have merged it to staging. Please take the latest pull of staging and modify code logic to include the uuid as split column.
@rajivharlalka
|
Hi @rajivharlalka , I have created a PR for this uuid thingy in postgres. If you can possibly pull the current staging and do changes then I will be able to merge your PR and your contribution will be reflected else the PR i have created will be merged. |
5 participants
Description
Fixes #144
Added checks to modify the SQL query with typecasting to TEXT datatype if the splitcolumn is stored as a string datatype in Olake.
Type of change
How Has This Been Tested?
Added a table with fields
id uuid(Primary Key), name textand checked backfill with id as splitkey.Added a table with fields
id uuid(Primary Key), name text, roll intand checked backfill with id as the splitkeyScreenshots or Recordings
Related PR's (If Any):