Feature: add support for text and uuid type control columns

[akulapid]

This change introduces two new parameters on create_parent, a pair of encoder/decoder functions that users can define to describe how a text/uuid column maps to time information. Once we derive time from the column, we can leverage existing time partitioning logic to manage partitions for text columns.

Allowing users to define functions brings flexibility to support various formats of identifiers such as UUIDv7, ULID, Snowflake IDs etc. As a convenience, encoder/decoder functions are included for UUIDv7 given the widespread standard.

Issue tracker: #528

[keithf4]

This is an awesome PR! Thank you!

Some questions and syntax corrections to review

[akulapid]

Thanks, have pushed a new commit addressing all the changes

[akulapid]

Hi @keithf4, just wanted to follow up on this, hope this addresses the changes. Let me know if there's anything else needed from my side

[keithf4]

Hi @keithf4, just wanted to follow up on this, hope this addresses the changes. Let me know if there's anything else needed from my side

Yes, I am keeping an eye on it! One additional note on the show_partitions function. I may be slow to respond for a bit. Just have a lot of other stuff going on. Shared this PR around the office and people are really impressed with it. So thank you!

[akulapid]

Good to hear! I understand this may take some time, will try not to bump it up too often :)

[keithf4]

This looks really good!

Only have one more ask and it doesn't have to be done as part of this commit either (I may have this merged before you even get to it). Any chance you could write a test with the uuid7 datatype with this? I understand it has another non-default datatype requirement, but since that was one of the main reasons for requesting this feature, I'd like to have a test for it since it may be a common usage going forward.

Make a subfolder in the test folder called test_uuid7 and place them in there. I'm working on organizing the tests so that if specific requirements are needed for them to pass, those tests are all isolated in their own folder.

[akulapid]

Good point, I missed this scenario. I have the commit ready, I can push to this same branch if you'd like to keep everything together before merging. Otherwise, happy to raise a separate PR. Let me know.

[rauanmayemir]

Anxious to try this. 😄

[keithf4]

Anxious to try this. 😄

Go ahead and commit to this branch. Sorry I missed your first reply.

[akulapid]

Done, have pushed the change. There are some conflicts now with development branch though, and I wasn't able to rebase because there are failing tests on development. Is this something you can take up when merging? Otherwise, I can rebase once development is stable again

[keithf4]

Hmm didn't realize failing tests would block that. I'll try and get it fixed when I have a chance.

[keithf4]

I fixed the merge conflicts for now.
One of the new uuid tests is failing. I'll try and look into it further when I have a chance, but figured I'd get this fixed so you might be able to look at it too before I am able to.

pg_prove -ovf test/test-uuid-time-subpart-custom-start.sql

...
ok 232 - Check id_taptest_table_p20_p20241101 does not exist
ok 233 - Check id_taptest_table_p20_p20241031 does not exist
not ok 234 - Check id_taptest_table_p30_p20241103 does not exist
# Failed test 234: "Check id_taptest_table_p30_p20241103 does not exist"
not ok 235 - Check id_taptest_table_p30_p20241102 does not exist
# Failed test 235: "Check id_taptest_table_p30_p20241102 does not exist"
not ok 236 - Check id_taptest_table_p30_p20241101 does not exist
# Failed test 236: "Check id_taptest_table_p30_p20241101 does not exist"
not ok 237 - Check id_taptest_table_p30_p20241031 does not exist
# Failed test 237: "Check id_taptest_table_p30_p20241031 does not exist"
not ok 238 - Check id_taptest_table_p40_p20241103 does not exist
# Failed test 238: "Check id_taptest_table_p40_p20241103 does not exist"
not ok 239 - Check id_taptest_table_p40_p20241102 does not exist
# Failed test 239: "Check id_taptest_table_p40_p20241102 does not exist"
not ok 240 - Check id_taptest_table_p40_p20241101 does not exist
# Failed test 240: "Check id_taptest_table_p40_p20241101 does not exist"
not ok 241 - Check id_taptest_table_p40_p20241031 does not exist
# Failed test 241: "Check id_taptest_table_p40_p20241031 does not exist"
not ok 242 - Check id_taptest_table_p50_p20241103 does not exist
# Failed test 242: "Check id_taptest_table_p50_p20241103 does not exist"
not ok 243 - Check id_taptest_table_p50_p20241102 does not exist
# Failed test 243: "Check id_taptest_table_p50_p20241102 does not exist"
ok 244 - Check id_taptest_table_p50_p20241101 does not exist
ok 245 - Check id_taptest_table_p50_p20241031 does not exist
not ok 246 - Check id_taptest_table_p60_p20241103 does not exist
# Failed test 246: "Check id_taptest_table_p60_p20241103 does not exist"
not ok 247 - Check id_taptest_table_p60_p20241102 does not exist
# Failed test 247: "Check id_taptest_table_p60_p20241102 does not exist"
...

[akulapid]

I hope development is not failing only for me? If not, could I suggest holding off this merge until that is fixed?
test-id-time-subpart-custom-start is failing for me on development, so some sub-partitioning logic seems to be broken there. My changes, which were up-to-date with master, should otherwise be green. It should be easier to merge this once development is also green, and we shouldn't really expect any more test failures at that point.
Let me know if this makes sense. I hope I'm not missing something about the workflow here!

[keithf4]

I hope development is not failing only for me? If not, could I suggest holding off this merge until that is fixed? test-id-time-subpart-custom-start is failing for me on development, so some sub-partitioning logic seems to be broken there. My changes, which were up-to-date with master, should otherwise be green. It should be easier to merge this once development is also green, and we shouldn't really expect any more test failures at that point. Let me know if this makes sense. I hope I'm not missing something about the workflow here!

Apologies, yeah, the original test you copied that one off of is failing as well. I'll look into that and see what's going on and let you know if anything in your test needs fixing as well.

[keithf4]

I've fixed the issue with test-id-time-subpart-custom-start not working. Was a regression in trying to adjust when retention runs. All the tests in the top level of the test folder are now passing for me if you want to give it another try

[keithf4]

I tested merging development into your fork and the uuid tests seem to be passing. Would appreciate if you could do the same just to double-check then push the upstream merge to this PR. If all looks good on your end, this will be the last feature merge before 5.2 release. Just need to finish some other unit tests then

[akulapid]

Thanks, can confirm it's passing for me too. Just wanted to clarify once on how to proceed next. I generally prefer to rebase PR changes with the trunk, only merging into trunk and not the other way. This way the history is cleaner and the PR only has changes pertaining to it.

Would it be okay if I reverted your merge commits on this branch, instead rebase the original changes with development? I'll need to force push this branch, but the history should be cleaner in the end. Or let me know if I should do it another way.

[keithf4]

Sure if that works for you, go ahead. Hopefully the commits I did can help you fix the conflicts that I resolved with them. If not, I'm happy to try and resolve them as well if they're still there.

[akulapid]

Yep, I'm cross checking the conflicts with this. Have rebased and pushed, hope this should be good now