-
Notifications
You must be signed in to change notification settings - Fork 884
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
Add tests for unique index creation with Hypercore TAM #7491
base: main
Are you sure you want to change the base?
Add tests for unique index creation with Hypercore TAM #7491
Conversation
Unlike regular compressed tables, Hypercore TAM supports creating unique indexes after compression is enabled. Add tests for this use case.
@gayyappan, @svenklemm: please review this pull request.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7491 +/- ##
==========================================
+ Coverage 80.06% 82.13% +2.06%
==========================================
Files 190 230 +40
Lines 37181 43185 +6004
Branches 9450 10864 +1414
==========================================
+ Hits 29770 35471 +5701
- Misses 2997 3388 +391
+ Partials 4414 4326 -88 ☔ View full report in Codecov by Sentry. |
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.
One suggestion for extending the test, otherwise looks fine.
insert into uniquetable values ('2024-01-01 01:00', 3); | ||
-- Unique index creation on compressed chunk not supported | ||
\set ON_ERROR_STOP 0 | ||
create unique index time_key on uniquetable (time); | ||
ERROR: operation not supported on hypertables that have compression enabled | ||
\set ON_ERROR_STOP 1 |
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.
Although it does not harm to have this check here, I am not sure what potential bug you wanted to catch here.
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.
Just to make clear there is a difference and that this error is intact despite supporting unique indexes on Hypercore TAM.
select * from uniquetable where time = '2024-01-01 01:00'; | ||
time | value | ||
------------------------------+------- | ||
Mon Jan 01 01:00:00 2024 PST | 1 | ||
(1 row) | ||
|
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.
Now that you have a unique index, you should probably have tests that tries to insert rows that violate this constraint and demonstrate that it fails.
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.
Don't we already have those tests?
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.
There's actually some overlap with these tests and old tests. We might want to reconcile.
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.
Since it is already covered, I do not think this is necessary for this pull request.
Unlike regular compressed tables, Hypercore TAM supports creating unique indexes after compression is enabled. Add tests for this use case.
Disable-check: force-changelog-file