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

Database table names are inconsistently singular/plural #3625

Closed
shekhirin opened this issue Jul 6, 2023 · 16 comments
Closed

Database table names are inconsistently singular/plural #3625

shekhirin opened this issue Jul 6, 2023 · 16 comments
Assignees
Labels
A-db Related to the database C-debt A section of code is hard to understand or change D-good-first-issue Nice and easy! A great choice to get started S-breaking This PR includes a breaking change

Comments

@shekhirin
Copy link
Collaborator

shekhirin commented Jul 6, 2023

Describe the feature

Tidy up

tables!([
(CanonicalHeaders, TableType::Table),
(HeaderTD, TableType::Table),
(HeaderNumbers, TableType::Table),
(Headers, TableType::Table),
(BlockBodyIndices, TableType::Table),
(BlockOmmers, TableType::Table),
(BlockWithdrawals, TableType::Table),
(TransactionBlock, TableType::Table),
(Transactions, TableType::Table),
(TxHashNumber, TableType::Table),
(Receipts, TableType::Table),
(PlainAccountState, TableType::Table),
(PlainStorageState, TableType::DupSort),
(Bytecodes, TableType::Table),
(AccountHistory, TableType::Table),
(StorageHistory, TableType::Table),
(AccountChangeSet, TableType::DupSort),
(StorageChangeSet, TableType::DupSort),
(HashedAccount, TableType::Table),
(HashedStorage, TableType::DupSort),
(AccountsTrie, TableType::Table),
(StoragesTrie, TableType::DupSort),
(TxSenders, TableType::Table),
(SyncStage, TableType::Table),
(SyncStageProgress, TableType::Table)
]);

Additional context

No response

@shekhirin shekhirin added C-debt A section of code is hard to understand or change A-db Related to the database S-breaking This PR includes a breaking change labels Jul 6, 2023
@mattsse
Copy link
Collaborator

mattsse commented Jul 6, 2023

hehe a tale as old as time

@onbjerg
Copy link
Member

onbjerg commented Jul 6, 2023

related #3464

@github-actions
Copy link
Contributor

This issue is stale because it has been open for 14 days with no activity.

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Jul 21, 2023
@shekhirin shekhirin added D-good-first-issue Nice and easy! A great choice to get started and removed S-stale This issue/PR is stale and will close with no further activity labels Jul 21, 2023
@yongkangc
Copy link

Is anyone working on this right now? And would this be a suitable first ticket for public to work on since its S-breaking? Looking to take on this ticket to familiarize myself to the codebase and contribute deeper in the future.

@shekhirin
Copy link
Collaborator Author

@ExtremelySunnyYK no one is working on it right now, assigned you! S-breaking for the first ticket is fine, as it'll pass the review as usually and will not be merged until the whole batch of breaking PRs is ready to be released.

@yongkangc
Copy link

thank you! for assigning it to me @shekhirin appreciate it, will start working on this at this week.

@shekhirin
Copy link
Collaborator Author

hey @ExtremelySunnyYK, let me know if you're no longer working on this, as someone else could take this good first issue!

@JosepBove
Copy link
Contributor

I could take it if he is inactive

@shekhirin shekhirin assigned JosepBove and unassigned yongkangc Sep 5, 2023
@shekhirin
Copy link
Collaborator Author

@JosepBove sounds good, assigned!

@JosepBove
Copy link
Contributor

Thanks, will work on this tomorrow!

@JosepBove
Copy link
Contributor

Does everyone agree with this names?

 tables!([ 
     (CanonicalHeaders, TableType::Table), 
     (HeaderTD, TableType::Table), 
     (HeaderNumbers, TableType::Table), 
     (Headers, TableType::Table), 
     (BlockBodyIndices, TableType::Table), 
     (BlockOmmers, TableType::Table), 
     (BlockWithdrawals, TableType::Table), 
     (TransactionBlock, TableType::Table), 
     (Transactions, TableType::Table), 
     (TransactionHashNumbers, TableType::Table), 
     (Receipts, TableType::Table), 
     (PlainAccountStates, TableType::Table), 
     (PlainStorageStates, TableType::DupSort), 
     (Bytecodes, TableType::Table), 
     (AccountHistory, TableType::Table), 
     (StorageHistory, TableType::Table), 
     (AccountChangeSets, TableType::DupSort), 
     (StorageChangeSets, TableType::DupSort), 
     (HashedAccounts, TableType::Table), 
     (HashedStorages, TableType::DupSort), 
     (AccountsTries, TableType::Table), 
     (StoragesTries, TableType::DupSort), 
     (TransactionSenders, TableType::Table), 
     (SyncStages, TableType::Table), 
     (SyncStageProgresses, TableType::Table) 
 ]); 

@shekhirin
Copy link
Collaborator Author

@JosepBove IMO:

  1. HeaderTD -> HeaderTerminalDifficulties, just to be clear what's TD
  2. TransactionBlock -> TransactionBlocks
  3. Plain(Account|Storage)States -> Plain(Account|Storage)State, because it contains the latest version of the state and not multiple
  4. (Accounts|Storages)Tries -> (Accounts|Storages)Trie, same as state

@JosepBove
Copy link
Contributor

JosepBove commented Sep 6, 2023

Okay, I will keep this discussion open for today, if no one has anything else to say I will implement the changes

@shekhirin
Copy link
Collaborator Author

@mattsse @onbjerg thoughts?

@JosepBove
Copy link
Contributor

As no comments were made I start the implementation

@DaniPopes
Copy link
Member

#6787

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-db Related to the database C-debt A section of code is hard to understand or change D-good-first-issue Nice and easy! A great choice to get started S-breaking This PR includes a breaking change
Projects
Archived in project
Development

No branches or pull requests

6 participants