-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow DDL changes to Compressed Hypertables via Migration #21
base: master
Are you sure you want to change the base?
Allow DDL changes to Compressed Hypertables via Migration #21
Conversation
…ation To avoid having to decompress an entire hypertable to make a DDL change this procedure facilitates copying data to a new table with the correct structure. It copies whole chunks at a time, and compresses them in the new table as it goes.
|
||
|
||
CREATE OR REPLACE procedure migrate_data( | ||
old_table text, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd use regclasses for these to properly account for schema qualification
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this needs a bit more work, there's some edge cases we're not accounting for. I think one thing that would be really helpful to move it forward would be to create a small test procedure after this that created the hypertable, added some data, compressed it and moved it over. Ideally a hypertable with space partitioning. We should then run full outer join and check for nulls to make sure that all the rows are there etc. WE should probably also check and make sure the chunks we expect to be compressed are in the new hypertable.
-- specified. | ||
select a.show_chunks into last_chunk from | ||
(select * from | ||
show_chunks('copyto', older_than => older_than) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The static string in this seems wrong to me?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is. Will fix.
-- hypertable | ||
FOR c_row in c_curs LOOP | ||
EXECUTE format('insert into %I | ||
(select * from %s)', new_table, c_row.chunkname); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switching to regclasses should help here, we may have to do some casting to text beforehand into other variables, but shouldn't be too bad, and then we won't be switching between %c/%I and we'll actually make sure things are formatted correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem wasn't their classes so much as you have to do this as a dynamic query in order for the variables to be read in. Or at least, that was the only way that made it work.
DECLARE | ||
c_row record; | ||
c_curs cursor for | ||
select * from chunknames; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you'll have to open this later given that the temp table isn't created until later in the run, or at least it's a bit confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might even just avoid the temp table all together and use an array, which you can also loop through.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the same, but it does actually work. Though I'm not against an array. I'll just move to an array. That would mean I wouldn't need the cursor either anyway.
EXECUTE format('insert into %I | ||
(select * from %s)', new_table, c_row.chunkname); | ||
|
||
-- get most recent chunk from the new hypertable after copying the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this whole thing is not accounting correctly for space partitioning...also, does selecting directly from compressed chunks work or do you have to go through the hypertable in order to get proper routing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does not - I somehow managed to do all this testing with an uncompressed copyfrom table. Oops
To avoid having to decompress an entire hypertable to make a DDL change,
this procedure facilitates copying data to a new table with the correct
structure. It copies whole chunks at a time, and compresses them in the
new table as it goes.