-
Notifications
You must be signed in to change notification settings - Fork 213
feat(structured-ingestion) DBI-1096: Generic flattening schema projection for structured ingestion #4781
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
Merged
pfcoperez
merged 14 commits into
main
from
DBI-1095/connectors/structured-ingestion/abstractions
Sep 11, 2026
Merged
feat(structured-ingestion) DBI-1096: Generic flattening schema projection for structured ingestion #4781
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
60a80b9
ClickHouse destination connector: Factor out CHtype -> QKind conversi…
pfcoperez 898e5d0
Structured ingestion: Unexpected values tracker generating a JSON rep…
pfcoperez db9ce41
Schema projector from a lazy iterator of QValues (like a walk through…
pfcoperez a119932
Malformed data should not be wrapped around a field named "malformed_…
pfcoperez 36fc254
Fix linting issues
pfcoperez f8f9505
Remove unused SchemaProjector.Columns method and its test assertions
github-actions[bot] 340d978
PR Comments: Extract common JSON range validation common checks into …
pfcoperez bf8a9a8
PR Comments: Protect against nested values containing non-JSON repres…
pfcoperez 1d03fde
PR Comments: Bug -> Unexpected values were always included regardless…
pfcoperez 368a9fa
PR Comments: Detect duplicates in record projection
pfcoperez 25551fb
PR Comments: Rename 'unexpected' malformed-data reason to 'unexpected…
github-actions[bot] ccc26f8
PR Comments: Move QValueKindForType from flow/pkg/clickhouse into the…
github-actions[bot] c8bd1ce
PR Comments: Rename malformed data column to _peerdb_malformed_data
github-actions[bot] ad480b6
PR COmments
pfcoperez File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| package structured | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "fmt" | ||
| "math" | ||
|
|
||
| "github.com/PeerDB-io/peerdb/flow/generated/protos" | ||
| "github.com/PeerDB-io/peerdb/flow/shared/types" | ||
| ) | ||
|
|
||
| const MalformedDataColumn = "_peerdb_malformed_data" | ||
|
|
||
| type MalformedReason int | ||
|
|
||
| const ( | ||
| ReasonUnexpected MalformedReason = iota | ||
| ReasonTypeMismatch | ||
| ReasonNaN | ||
| ReasonDuplicatedFields | ||
| ) | ||
|
|
||
| func (r MalformedReason) String() string { | ||
| switch r { | ||
| case ReasonUnexpected: | ||
| return "unexpected_field" | ||
| case ReasonTypeMismatch: | ||
| return "type_mismatch" | ||
| case ReasonNaN: | ||
| return "not_a_number" | ||
| case ReasonDuplicatedFields: | ||
| return "duplicated_fields" | ||
| default: | ||
| return "" | ||
| } | ||
| } | ||
|
|
||
| // This type represents malformed data state. | ||
| // As structured is applied to a document based on a ClickHouse schema, problematic fields | ||
| // get recorded in this state. | ||
| type MalformedData struct { | ||
| // Map from seen problematic fields to their respective reasons. | ||
| reasons map[string]MalformedReason | ||
| // Optionally, the problematic values | ||
| values map[string]types.QValue | ||
| } | ||
|
|
||
| // NewMalformedData creates a new instance of MalformedData. | ||
| func NewMalformedData() *MalformedData { | ||
| return &MalformedData{ | ||
| reasons: make(map[string]MalformedReason), | ||
| values: make(map[string]types.QValue), | ||
| } | ||
| } | ||
|
|
||
| // AddField adds a problematic field along with its reason and, when value is not nil, its value to the | ||
| // MalformedData. | ||
| func (m *MalformedData) AddField(field string, reason MalformedReason, value types.QValue) { | ||
| m.reasons[field] = reason | ||
| if value != nil { | ||
| m.values[field] = value | ||
| } | ||
| } | ||
|
|
||
| func (m *MalformedData) IsEmpty() bool { | ||
| return len(m.reasons) == 0 | ||
| } | ||
|
|
||
| // MarshalJSON implements json.Marshaler, serializing the malformed fields state in the shape of a JSON object | ||
| // which is query friendly if ingested into ClickHouse: | ||
| // | ||
| // {"<field>": {"<reason>": true, "value": <value>}} | ||
| func (m *MalformedData) MarshalJSON() ([]byte, error) { | ||
| fields := make(map[string]map[string]any, len(m.reasons)) | ||
| for name, reason := range m.reasons { | ||
| reasonName := reason.String() | ||
| if reasonName == "" { | ||
| return nil, fmt.Errorf("malformed field %q has unknown reason %d", name, int(reason)) | ||
| } | ||
| fields[name] = map[string]any{reasonName: true} | ||
| } | ||
| for name, value := range m.values { | ||
| field, ok := fields[name] | ||
| if !ok { | ||
| return nil, fmt.Errorf("malformed field %q has a value but no reason", name) | ||
| } | ||
| // encoded from the Go value: nulls become JSON null, QValueJSON stays a (quoted) string | ||
| var jsonValue any | ||
| if value != nil { | ||
| jsonValue = value.Value() | ||
| } | ||
| if isJSONRepresentable(jsonValue) { | ||
| marshaledValue, err := json.Marshal(jsonValue) | ||
| if err != nil { | ||
| field["value"] = fmt.Sprintf("%v", jsonValue) | ||
| } else { | ||
| field["value"] = json.RawMessage(marshaledValue) | ||
| } | ||
| } else { | ||
| fields[name] = map[string]any{ReasonNaN.String(): true} | ||
| } | ||
| } | ||
| return json.Marshal(fields) | ||
| } | ||
|
|
||
| func (m *MalformedData) AsQValue() (types.QValue, error) { | ||
| jsonData, err := m.MarshalJSON() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| return types.QValueJSON{Val: string(jsonData)}, nil | ||
| } | ||
|
|
||
| // Describes the column malformed data is recorded in. | ||
| func MalformedDataFieldDescription() *protos.FieldDescription { | ||
| return &protos.FieldDescription{ | ||
| Name: MalformedDataColumn, | ||
| Type: string(types.QValueKindJSON), | ||
| TypeModifier: -1, | ||
| Nullable: true, | ||
| } | ||
| } | ||
|
|
||
| // isJSONRepresentable reports whether v can be encoded by encoding/json, which rejects NaN and ±Inf floats. | ||
| func isJSONRepresentable(v any) bool { | ||
| validateFloat := func(f float64) bool { | ||
| return !math.IsNaN(f) && !math.IsInf(f, 0) | ||
| } | ||
| switch f := v.(type) { | ||
| case float64: | ||
| return validateFloat(f) | ||
| case float32: | ||
| return validateFloat(float64(f)) | ||
| default: | ||
| return true | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think claude already mentioned this but here it's overriding the default reason and iiuc is only set because json.Marshal does not support it. could instead convert the NaN/-Inf/+Inf value as string and then the original reason can be used and reasonNaN can be removed (since ClickHouse supports NaN/Inf/-Inf for floats)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes Claude flagged it too. I disregarded its comment for this reason: In these cases, the data could be malformed for two different reasons: The out of domain value and the potentially being an unexpected or wrong type column.
I want to keep the malformed column simple and avoid the situation were we record or need to detect all the possible ways in which a value is broken. So I decided that it was better to flag the completely out of domain value because:
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe I am misunderstanding something here so want to clarify.
my understanding is that
ReasonNaNhere does not mean that the data itself cannot be replicated due to NaN (because clickhouse supports it for floats), but it's set to NaN because _peerdb_malformed_data column is JSON type and that does not support NaN if the data is deemed incompatible for some other reason (e.g. unexpected field, or type mismatch, but it shouldn't be ambiguous here since unexpected field means the field is not expected from the schema, or type mismatch which means the actual type does not match the inferred type). if my understanding is correct, then i think this code here is hiding that root cause, and overriding with NaN instead.I can't think of a valid scenario where NaN is a valid reason on during inferencing itself: if it's a float, clickhouse can handle it; if it's not a float, then it's a typeMismatch error of some sort.
Does this match your understanding or did I miss something here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jgao54 You understood it perfectly!
There is a wrong premise in this comment of mine:
I assumed we didn't accept these float values for ingestion but you clarified that they do, thank you!
Corrected in #4798