Skip to content
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

Change schedule primary key from consensus_timestamp to schedule_id #2382

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

steven-sheehy
Copy link
Member

Description:

  • Change schedule primary key from consensus_timestamp to schedule_id
  • Drop unused consensus_timestamp index
  • Fix schedule REST API ordering by wrong field

Related issue(s):

Fixes #2380

Notes for reviewer:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@steven-sheehy steven-sheehy added bug Type: Something isn't working P2 database Area: Database labels Aug 9, 2021
@steven-sheehy steven-sheehy added this to the Mirror 0.39.0 milestone Aug 9, 2021
@steven-sheehy steven-sheehy self-assigned this Aug 9, 2021
@codecov
Copy link

codecov bot commented Aug 9, 2021

Codecov Report

Merging #2382 (b560191) into main (1533d0e) will decrease coverage by 0.00%.
The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2382      +/-   ##
============================================
- Coverage     84.35%   84.35%   -0.01%     
- Complexity     2308     2309       +1     
============================================
  Files           439      439              
  Lines         11982    11984       +2     
  Branches       1020     1020              
============================================
+ Hits          10108    10109       +1     
  Misses         1556     1556              
- Partials        318      319       +1     
Impacted Files Coverage Δ
...va/com/hedera/mirror/importer/domain/Schedule.java 80.00% <66.66%> (-7.50%) ⬇️
...er/parser/record/entity/sql/SqlEntityListener.java 93.36% <100.00%> (ø)
...epository/upsert/ScheduleUpsertQueryGenerator.java 92.30% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1533d0e...b560191. Read the comment docs.

Signed-off-by: Steven Sheehy <steven.sheehy@hedera.com>
@sonarcloud
Copy link

sonarcloud bot commented Aug 9, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@steven-sheehy steven-sheehy marked this pull request as ready for review August 9, 2021 20:08
@steven-sheehy steven-sheehy requested a review from a team August 9, 2021 20:08
on schedule (schedule_id desc, consensus_timestamp desc);

alter table if exists schedule
add primary key (schedule_id);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may be missing a REST endpoint for filter by timestamp, which is where an additional timestamp index might be useful to speed up queries.
We can address that later though

Copy link
Contributor

@Nana-EC Nana-EC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@xin-hedera xin-hedera left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@steven-sheehy steven-sheehy merged commit e9aa742 into main Aug 10, 2021
@steven-sheehy steven-sheehy deleted the schedule-primary-key branch August 10, 2021 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Type: Something isn't working database Area: Database P2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Schedule consensus timestamp cannot be used for upsert
4 participants