Skip to content
This repository has been archived by the owner on Sep 27, 2019. It is now read-only.

Settings refactoring #1432

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

ksaito7
Copy link
Contributor

@ksaito7 ksaito7 commented Jun 26, 2018

This PR refactors setting stuff in order to enable checkpoint manager to recover setting values. In addition, added some convenient functions.

- Fix Points of SettingsManager

  1. Change All set functions to non-static, and move all set/get functions to public.
  2. Add txn argument into set functions in order that a caller of set functions can manage a necessary transaction by itself.
  3. Add set_default argument into set functions. If this flag argument is true, then the default value is updated in addition to updating the setting value.
  4. Add Reset public function to reset the setting value to default value.
  5. Add UpdateSettingListFromCatalog() public function to update setting values in the manager from catalog for checkpoints.
  6. Add/Modify two catalog management private functions: InsertCatalog(), UpdateCatalog().

- Fix Points of SettingsCatalog

  1. Add SettingsCatalogEntry class to acquire all setting information from catalog
  2. Add GetSettingsCatalogEntry() and GetSettingsCatalogEntries() functions.
    *These function names are temporary. The permanent names depend on discussion in PR Catalog code cleanup #1414.
  3. Delete other get functions.
  4. Add UpdateSettingValue() function to update a setting value and a default value.

- Related Issues

  1. Issues Setting issues #1424

@coveralls
Copy link

coveralls commented Jun 29, 2018

Coverage Status

Coverage decreased (-0.1%) to 76.39% when pulling ebc71ad on ksaito7:settings_refactor into 898219f on cmu-db:master.

@ksaito7
Copy link
Contributor Author

ksaito7 commented Jul 5, 2018

This last commit isn't related to this PR directly, but I did it because a part of index names of constraints were weird. (related to #1415 we worked for last time)

@db-ol db-ol removed their request for review July 9, 2018 18:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants