Skip to content

Preserve explicit zero config values when applying defaults#177

Open
Vlasdislav wants to merge 2 commits into
open-gpdb:masterfrom
Vlasdislav:feat/config-functional-options
Open

Preserve explicit zero config values when applying defaults#177
Vlasdislav wants to merge 2 commits into
open-gpdb:masterfrom
Vlasdislav:feat/config-functional-options

Conversation

@Vlasdislav

Copy link
Copy Markdown
Contributor

Prefill config defaults before YAML/TOML/JSON decoding so explicit zero and false values from user config are preserved. Move defaults to their config sections and cover the behavior with tests.

@Vlasdislav
Vlasdislav requested review from diPhantxm and reshke July 17, 2026 15:16

@leborchuk leborchuk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, using opts and With methods is more go-way then checking default values

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR changes config-loading to prefill defaults before YAML/TOML/JSON decoding so that explicitly configured zero/false values are preserved (instead of being overwritten by “zero means unset” defaulting).

Changes:

  • Replaces EmbedDefaults with builder-style default constructors (BuildInstance, BuildStorage, BuildVacuum) and applies defaults by initializing the config struct prior to decoding.
  • Removes EmbedDefaults calls from copy flows and updates a garbage-collection test to construct vacuum defaults explicitly.
  • Adds config/instance_test.go to verify defaults and preservation of explicit zero/false values across YAML/JSON/TOML.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
pkg/proc/interaction.go Removes post-decode default embedding when reading the “old bucket” instance config.
pkg/proc/delete_handler_test.go Updates test setup to use BuildVacuum defaults (with check_backup=false) instead of EmbedDefaults.
pkg/core/pg/pg.go Removes post-decode default embedding when reading the “old bucket” instance config.
config/vacuum.go Moves vacuum defaults into the vacuum config section and adds option/builder helpers.
config/storage.go Moves storage defaults into the storage config section and adds option/builder helpers.
config/instance.go Introduces instance builder/options and switches to “prefill then decode” initialization flow.
config/instance_test.go Adds coverage ensuring defaults apply when unset and explicit zero/false values are preserved.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread config/instance.go
Comment on lines +127 to 132
if strings.HasSuffix(file.Name(), ".yaml") {
return yaml.NewDecoder(file).Decode(&cfgInstance)
}
if cfgInstance.VacuumCnf.ProtectionWindow == 0 {
cfgInstance.VacuumCnf.ProtectionWindow = DefaultProtectionWindow
if strings.HasSuffix(file.Name(), ".json") {
return json.NewDecoder(file).Decode(&cfgInstance)
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants