Skip to content

Commit 3231e9a

Browse files
jshearerjshearer
committed
validation: auto-promote source.targetNaming: withSchema to top-level targetNaming
During validation, materializations with `source.targetNaming` set to `withSchema` (or the legacy alias `fromSourceName`) and no top-level `targetNaming` now get promoted to `targetNaming: { strategy: matchSourceStructure }` as a model fix. These two settings produce identical resource paths, so the promotion is behavior-preserving. This enables progressive migration away from the legacy `source.targetNaming` field. The old field remains functional via the fallback in `update_materialization_resource_spec`, so specs that haven't been re-published yet continue to work. Once the UI is updated to write the new top-level field directly, the legacy path can be removed. * Add promotion logic in `walk_materialization` that fires only when `target_naming` is `None` * Add `test_target_naming_promotion` verifying the fix fires and sets `MatchSourceStructure` * Add `test_target_naming_no_promotion_when_set` verifying an explicit top-level value is not overwritten
1 parent 61635c4 commit 3231e9a

4 files changed

Lines changed: 260 additions & 1 deletion

File tree

‎crates/validation/src/materialization.rs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ async fn walk_materialization<C: Connectors>(
114114
let models::MaterializationDef {
115115
on_incompatible_schema_change,
116116
source: sources,
117-
target_naming,
117+
mut target_naming,
118118
endpoint,
119119
bindings: bindings_model,
120120
mut shards,
@@ -124,6 +124,23 @@ async fn walk_materialization<C: Connectors>(
124124
reset,
125125
} = model;
126126

127+
// Model fix: Promote source.target_naming == WithSchema to top-level
128+
// MatchSourceStructure. These represent identical behavior, and this
129+
// progressive upgrade lets us eventually remove the legacy field.
130+
if target_naming.is_none() {
131+
if let Some(source) = &sources {
132+
if source.to_normalized_def().target_naming == models::TargetNaming::WithSchema {
133+
target_naming = Some(models::TargetNamingStrategy::MatchSourceStructure {
134+
table_template: None,
135+
schema_template: None,
136+
});
137+
model_fixes.push(
138+
"promoted source.targetNaming 'withSchema' to top-level targetNaming 'matchSourceStructure'".to_string(),
139+
);
140+
}
141+
}
142+
}
143+
127144
indexed::walk_name(
128145
scope,
129146
"materialization",

‎crates/validation/tests/snapshots/transition_tests__target_naming_no_promotion_when_set.snap‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
---
2+
source: crates/validation/tests/transition_tests.rs
3+
expression: "(&outcome.built_materializations[0].model,\n&outcome.built_materializations[0].model_fixes)"
4+
---
5+
(
6+
Some(
7+
MaterializationDef {
8+
source: Some(
9+
Configured(
10+
SourceDef {
11+
capture: Some(
12+
Capture(
13+
"the/capture",
14+
),
15+
),
16+
target_naming: WithSchema,
17+
delta_updates: false,
18+
fields_recommended: Bool(
19+
true,
20+
),
21+
},
22+
),
23+
),
24+
target_naming: Some(
25+
SingleSchema {
26+
schema: "my_schema",
27+
table_template: None,
28+
},
29+
),
30+
on_incompatible_schema_change: Backfill,
31+
endpoint: Connector(
32+
ConnectorConfig {
33+
image: "other/image",
34+
config: RawValue(
35+
{"a":"config"},
36+
),
37+
},
38+
),
39+
bindings: [
40+
MaterializationBinding {
41+
resource: RawValue(
42+
{"_meta":{"path":["table","path"]},"table":"bar"},
43+
),
44+
source: Collection(
45+
Collection(
46+
"the/collection",
47+
),
48+
),
49+
disable: false,
50+
priority: 0,
51+
fields: MaterializationFields {
52+
group_by: [],
53+
require: {
54+
Field(
55+
"F1",
56+
): RawValue(
57+
{},
58+
),
59+
Field(
60+
"f_two",
61+
): RawValue(
62+
{},
63+
),
64+
},
65+
exclude: [
66+
Field(
67+
"F2",
68+
),
69+
],
70+
recommended: Bool(
71+
true,
72+
),
73+
},
74+
backfill: 0,
75+
on_incompatible_schema_change: None,
76+
},
77+
],
78+
shards: ShardTemplate {
79+
disable: false,
80+
min_txn_duration: None,
81+
max_txn_duration: None,
82+
hot_standbys: None,
83+
ring_buffer_size: None,
84+
read_channel_size: None,
85+
log_level: None,
86+
flags: {},
87+
},
88+
expect_pub_id: None,
89+
triggers: None,
90+
delete: false,
91+
reset: false,
92+
},
93+
),
94+
[
95+
"removed dropped exclude projection FY of source collection the/collection",
96+
"removed dropped exclude projection does/not/exist of source collection the/collection",
97+
],
98+
)

‎crates/validation/tests/snapshots/transition_tests__target_naming_promotion.snap‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
---
2+
source: crates/validation/tests/transition_tests.rs
3+
expression: "(&outcome.built_materializations[0].model,\n&outcome.built_materializations[0].model_fixes)"
4+
---
5+
(
6+
Some(
7+
MaterializationDef {
8+
source: Some(
9+
Configured(
10+
SourceDef {
11+
capture: Some(
12+
Capture(
13+
"the/capture",
14+
),
15+
),
16+
target_naming: WithSchema,
17+
delta_updates: false,
18+
fields_recommended: Bool(
19+
true,
20+
),
21+
},
22+
),
23+
),
24+
target_naming: Some(
25+
MatchSourceStructure {
26+
table_template: None,
27+
schema_template: None,
28+
},
29+
),
30+
on_incompatible_schema_change: Backfill,
31+
endpoint: Connector(
32+
ConnectorConfig {
33+
image: "other/image",
34+
config: RawValue(
35+
{"a":"config"},
36+
),
37+
},
38+
),
39+
bindings: [
40+
MaterializationBinding {
41+
resource: RawValue(
42+
{"_meta":{"path":["table","path"]},"table":"bar"},
43+
),
44+
source: Collection(
45+
Collection(
46+
"the/collection",
47+
),
48+
),
49+
disable: false,
50+
priority: 0,
51+
fields: MaterializationFields {
52+
group_by: [],
53+
require: {
54+
Field(
55+
"F1",
56+
): RawValue(
57+
{},
58+
),
59+
Field(
60+
"f_two",
61+
): RawValue(
62+
{},
63+
),
64+
},
65+
exclude: [
66+
Field(
67+
"F2",
68+
),
69+
],
70+
recommended: Bool(
71+
true,
72+
),
73+
},
74+
backfill: 0,
75+
on_incompatible_schema_change: None,
76+
},
77+
],
78+
shards: ShardTemplate {
79+
disable: false,
80+
min_txn_duration: None,
81+
max_txn_duration: None,
82+
hot_standbys: None,
83+
ring_buffer_size: None,
84+
read_channel_size: None,
85+
log_level: None,
86+
flags: {},
87+
},
88+
expect_pub_id: None,
89+
triggers: None,
90+
delete: false,
91+
reset: false,
92+
},
93+
),
94+
[
95+
"promoted source.targetNaming 'withSchema' to top-level targetNaming 'matchSourceStructure'",
96+
"removed dropped exclude projection FY of source collection the/collection",
97+
"removed dropped exclude projection does/not/exist of source collection the/collection",
98+
],
99+
)

‎crates/validation/tests/transition_tests.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,51 @@ driver:
643643
));
644644
}
645645

646+
#[test]
647+
fn test_target_naming_promotion() {
648+
// When source.targetNaming is "withSchema" and no top-level targetNaming is set,
649+
// validation should promote it to MatchSourceStructure as a model fix.
650+
let outcome = common::run(
651+
MODEL_YAML,
652+
r#"
653+
test://example/catalog.yaml:
654+
materializations:
655+
the/materialization:
656+
source:
657+
capture: the/capture
658+
targetNaming: withSchema
659+
"#,
660+
);
661+
insta::assert_debug_snapshot!((
662+
&outcome.built_materializations[0].model,
663+
&outcome.built_materializations[0].model_fixes
664+
));
665+
}
666+
667+
#[test]
668+
fn test_target_naming_no_promotion_when_set() {
669+
// When top-level targetNaming is already set, source.targetNaming should not
670+
// trigger promotion (the top-level value takes precedence).
671+
let outcome = common::run(
672+
MODEL_YAML,
673+
r#"
674+
test://example/catalog.yaml:
675+
materializations:
676+
the/materialization:
677+
source:
678+
capture: the/capture
679+
targetNaming: withSchema
680+
targetNaming:
681+
strategy: singleSchema
682+
schema: my_schema
683+
"#,
684+
);
685+
insta::assert_debug_snapshot!((
686+
&outcome.built_materializations[0].model,
687+
&outcome.built_materializations[0].model_fixes
688+
));
689+
}
690+
646691
#[test]
647692
fn test_manual_redact_salt_override() {
648693
// Test that manually specified redact_salt overrides existing salt

0 commit comments

Comments
 (0)