Follow-up from #794. Raised by the oracle-db-admin review of that change (finding 6) and deferred deliberately so #794 could land.
The gap
#794 makes config-vs-database precedence depend on system_settings.origin ('seeded' / 'explicit' / NULL). Every test added for it runs on SQLite:
api/models/system_setting_test.goapi/settings_precedence_test.goapi/settings_service_test.gocmd/dbtool/config_origin_test.gocmd/server/startup_checks_test.gointernal/dbschema/ backfill tests
SQLite cannot reproduce the mechanisms the feature depends on. It does not collapse '' to NULL, does not uppercase identifiers, and does not exercise godror's VARCHAR2 scan path. The review confirmed by inspection that NullableDBVarchar.Value()/Scan() round-trip 'seeded'/'explicit' intact on Oracle and that no path turns a written origin into NULL — but that is a code reading, not a test.
This matters more than usual because the failure is silent and directional. If an origin value were ever lost on Oracle, the row would read as not-explicit, config would win, and nothing would error — it would just quietly resolve to a different value than the same code resolves to on PostgreSQL.
What to add
An _Integration test (the suffix is required for make test-integration-oci to pick it up) that, against a real ADB:
- Seeds a row with
origin = 'seeded', reads it back through GetResolvedString with an explicit config value present, asserts the config value wins.
- Sets a row explicitly (through
SettingsService.Set), asserts the database value wins over an explicit config value.
- Asserts a NULL-origin row reads as seeded, i.e.
IsExplicit() is false — the fail-safe polarity.
- Round-trips
'seeded' and 'explicit' through write → read and asserts neither comes back NULL or empty.
- Exercises the
BackfillSystemSettingOrigin migration and its CHECK constraint on Oracle, including running it twice for idempotency.
Note the ADB is Always Free and auto-stops when idle, so it needs to be started before the run (see scripts/oci-env.sh).
Related
Follow-up from #794. Raised by the
oracle-db-adminreview of that change (finding 6) and deferred deliberately so #794 could land.The gap
#794 makes config-vs-database precedence depend on
system_settings.origin('seeded'/'explicit'/ NULL). Every test added for it runs on SQLite:api/models/system_setting_test.goapi/settings_precedence_test.goapi/settings_service_test.gocmd/dbtool/config_origin_test.gocmd/server/startup_checks_test.gointernal/dbschema/backfill testsSQLite cannot reproduce the mechanisms the feature depends on. It does not collapse
''to NULL, does not uppercase identifiers, and does not exercise godror's VARCHAR2 scan path. The review confirmed by inspection thatNullableDBVarchar.Value()/Scan()round-trip'seeded'/'explicit'intact on Oracle and that no path turns a written origin into NULL — but that is a code reading, not a test.This matters more than usual because the failure is silent and directional. If an origin value were ever lost on Oracle, the row would read as not-explicit, config would win, and nothing would error — it would just quietly resolve to a different value than the same code resolves to on PostgreSQL.
What to add
An
_Integrationtest (the suffix is required formake test-integration-ocito pick it up) that, against a real ADB:origin = 'seeded', reads it back throughGetResolvedStringwith an explicit config value present, asserts the config value wins.SettingsService.Set), asserts the database value wins over an explicit config value.IsExplicit()is false — the fail-safe polarity.'seeded'and'explicit'through write → read and asserts neither comes back NULL or empty.BackfillSystemSettingOriginmigration and its CHECK constraint on Oracle, including running it twice for idempotency.Note the ADB is Always Free and auto-stops when idle, so it needs to be started before the run (see
scripts/oci-env.sh).Related
originthrough the admin settings API