You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "mattp5657 (via GitHub)" <gi...@apache.org> on 2026/09/30 15:10:45 UTC
[PR] Fix/field mapping clone before clear [iggy]
mattp5657 opened a new pull request, #4362:
URL: https://github.com/apache/iggy/pull/4362
# PR description draft: field-mapping clone-before-clear
Draft body for the PR closing
[issue-field-mapping-clone-before-clear.md](issue-field-mapping-clone-before-clear.md).
Copy into the PR; fill in the real issue number.
---
## Which issue does this PR address?
Closes #4307
## Rationale
Eight sites renamed JSON keys for `field_mappings` by cloning every
key/value into a side buffer, clearing the map, then reinserting —
copying data that was already about to be dropped.
## What changed?
New `convert::apply_field_mappings` helper (`.drain()`s the map instead
of cloning); all 8 sites now call it. Behavior is byte-for-byte
identical to `master`, including on a collision — same outcome, just via
a move instead of a clone. No new error paths, no signature changes.
**Out of scope:** a rename collision (two keys onto the same target, or
a duplicate key already in the payload) still silently keeps one value
and drops the other, same as `master`. Which value survives is
deterministic on small objects but unspecified once `simd_json` swaps
to its hash-backed map. Pre-existing, not introduced by this PR. Filing
a follow-up issue for how these collisions should be handled and
whether `field_mappings` validation should be systematic across all 9
config types (0 covered today).
## Test coverage
Fixed one vacuous test (`decoders/proto.rs` configured `use_any_wrapper`
with no schema, so `field_mappings` was never actually exercised —
rewritten against a real schema). Strengthened several tests that only
checked key presence/absence to also assert the renamed value survived.
Added a collision test and a 40-key test covering `simd_json`'s
backing-map swap.
## Local Execution
- `cargo test -p iggy_connector_sdk` (202 unit + 10 flatbuffer + 21
proto integration) and `-p iggy-connectors` (237): passing.
- `fmt` / `sort` / `clippy -D warnings` / license/whitespace/newline
scripts: clean.
## AI Usage
1. **Tools:** Claude
2. **Use:** `.drain()` rewrite across all 8 sites, shared helper.
3. **Verification:** Full local test suite.
4. Reviewed and can explain every line if asked.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix/field mapping clone before clear [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #4362:
URL: https://github.com/apache/iggy/pull/4362#issuecomment-5915539313
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/4362?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `44.76534%` with `153 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 56.67%. Comparing base ([`8d0e28d`](https://app.codecov.io/gh/apache/iggy/commit/8d0e28d9cebaa174b74709f8d256db510e067418?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2a741d5`](https://app.codecov.io/gh/apache/iggy/commit/2a741d5639b3bc2407bb97dca75ff5016bf4b239?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 38 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/4362?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [core/connectors/sdk/src/convert.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fconvert.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvY29udmVydC5ycw==) | 44.88% | [96 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [core/connectors/sdk/src/encoders/proto.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fencoders%2Fproto.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZW5jb2RlcnMvcHJvdG8ucnM=) | 45.23% | [23 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [core/connectors/sdk/src/encoders/avro.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fencoders%2Favro.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZW5jb2RlcnMvYXZyby5ycw==) | 18.75% | [13 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [core/connectors/sdk/src/encoders/flatbuffer.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fencoders%2Fflatbuffer.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZW5jb2RlcnMvZmxhdGJ1ZmZlci5ycw==) | 21.42% | [11 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [core/connectors/sdk/src/decoders/proto.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fdecoders%2Fproto.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZGVjb2RlcnMvcHJvdG8ucnM=) | 27.27% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ore/connectors/sdk/src/transforms/proto\_convert.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Ftransforms%2Fproto_convert.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvdHJhbnNmb3Jtcy9wcm90b19jb252ZXJ0LnJz) | 75.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
:x: Your patch check has failed because the patch coverage (44.76%) is below the target coverage (50.00%). You can increase the patch coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #4362 +/- ##
=============================================
- Coverage 87.56% 56.67% -30.89%
Complexity 1575 1575
=============================================
Files 1284 1289 +5
Lines 225457 185959 -39498
Branches 188821 149323 -39498
=============================================
- Hits 197416 105389 -92027
- Misses 23314 76072 +52758
+ Partials 4727 4498 -229
```
| [Components](https://app.codecov.io/gh/apache/iggy/pull/4362/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [Rust Core](https://app.codecov.io/gh/apache/iggy/pull/4362/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `49.95% <44.76%> (-38.81%)` | :arrow_down: |
| [Java SDK](https://app.codecov.io/gh/apache/iggy/pull/4362/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.68% <ø> (ø)` | |
| [C# SDK](https://app.codecov.io/gh/apache/iggy/pull/4362/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `77.42% <ø> (ø)` | |
| [Python SDK](https://app.codecov.io/gh/apache/iggy/pull/4362/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.97% <ø> (ø)` | |
| [PHP SDK](https://app.codecov.io/gh/apache/iggy/pull/4362/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `85.67% <ø> (ø)` | |
| [Node SDK](https://app.codecov.io/gh/apache/iggy/pull/4362/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `94.74% <ø> (ø)` | |
| [Go SDK](https://app.codecov.io/gh/apache/iggy/pull/4362/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.05% <ø> (ø)` | |
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/4362?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [core/connectors/sdk/src/decoders/avro.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fdecoders%2Favro.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZGVjb2RlcnMvYXZyby5ycw==) | `37.41% <100.00%> (-47.00%)` | :arrow_down: |
| [core/connectors/sdk/src/decoders/flatbuffer.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fdecoders%2Fflatbuffer.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZGVjb2RlcnMvZmxhdGJ1ZmZlci5ycw==) | `68.96% <100.00%> (-23.63%)` | :arrow_down: |
| [core/connectors/sdk/src/transforms/avro\_convert.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Ftransforms%2Favro_convert.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvdHJhbnNmb3Jtcy9hdnJvX2NvbnZlcnQucnM=) | `63.53% <100.00%> (-21.47%)` | :arrow_down: |
| [...ore/connectors/sdk/src/transforms/proto\_convert.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Ftransforms%2Fproto_convert.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvdHJhbnNmb3Jtcy9wcm90b19jb252ZXJ0LnJz) | `29.08% <75.00%> (-51.90%)` | :arrow_down: |
| [core/connectors/sdk/src/decoders/proto.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fdecoders%2Fproto.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZGVjb2RlcnMvcHJvdG8ucnM=) | `57.74% <27.27%> (-32.24%)` | :arrow_down: |
| [core/connectors/sdk/src/encoders/flatbuffer.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fencoders%2Fflatbuffer.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZW5jb2RlcnMvZmxhdGJ1ZmZlci5ycw==) | `42.19% <21.42%> (-38.47%)` | :arrow_down: |
| [core/connectors/sdk/src/encoders/avro.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fencoders%2Favro.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZW5jb2RlcnMvYXZyby5ycw==) | `19.31% <18.75%> (-52.91%)` | :arrow_down: |
| [core/connectors/sdk/src/encoders/proto.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fencoders%2Fproto.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvZW5jb2RlcnMvcHJvdG8ucnM=) | `57.30% <45.23%> (-28.01%)` | :arrow_down: |
| [core/connectors/sdk/src/convert.rs](https://app.codecov.io/gh/apache/iggy/pull/4362?src=pr&el=tree&filepath=core%2Fconnectors%2Fsdk%2Fsrc%2Fconvert.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9jb25uZWN0b3JzL3Nkay9zcmMvY29udmVydC5ycw==) | `54.75% <44.88%> (-45.25%)` | :arrow_down: |
... and [594 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/4362/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix/field mapping clone before clear [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #4362:
URL: https://github.com/apache/iggy/pull/4362#issuecomment-5942136211
> please fix PR title :) i have no idea how did it slip thru CI.
Sorry about that - I actually thought to myself it looked weird but then saw it passed CI. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix/field mapping clone before clear [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on PR #4362:
URL: https://github.com/apache/iggy/pull/4362#issuecomment-5941946904
/skill team-review-slim
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix/field mapping clone before clear [iggy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4362:
URL: https://github.com/apache/iggy/pull/4362#issuecomment-5914096891
<!-- iggy-pr-triage-welcome -->
Thanks for the PR. It is labeled `S-waiting-on-review` and queued for review.
Slash commands (own line, regular comment) move it around the queue:
- `/ready` - back to `S-waiting-on-review` after addressing feedback
- `/author` - flip to `S-waiting-on-author` while you finish changes
- `/request-review @user-or-team` - request a reviewer
- `/pin` - exempt the PR from the stale bot, `/unpin` to undo
See [CONTRIBUTING.md](https://github.com/apache/iggy/blob/master/CONTRIBUTING.md#pr-triage-commands) for details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(connectors): drain field_mappings instead of clone-then-clear [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio merged PR #4362:
URL: https://github.com/apache/iggy/pull/4362
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix/field mapping clone before clear [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on PR #4362:
URL: https://github.com/apache/iggy/pull/4362#issuecomment-5942081355
please fix PR title :) i have no idea how did it slip thru CI.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix/field mapping clone before clear [iggy]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #4362:
URL: https://github.com/apache/iggy/pull/4362#discussion_r4161213879
##########
core/connectors/sdk/src/convert.rs:
##########
@@ -49,11 +51,38 @@ pub fn owned_value_to_serde_json(value: &simd_json::OwnedValue) -> serde_json::V
}
}
+/// Renames each key of a `Payload::Json` object via `rename`; other payloads pass through. Collisions keep the later entry while the map is Vec-backed, unspecified once hash-backed.
+pub(crate) fn apply_field_mappings(
+ payload: Payload,
+ mut rename: impl FnMut(String) -> String,
+) -> Payload {
+ match payload {
+ Payload::Json(json_value) => {
+ if let simd_json::OwnedValue::Object(mut map) = json_value {
+ let renamed: Vec<(String, simd_json::OwnedValue)> = map
+ .drain()
+ .map(|(key, value)| (rename(key), value))
+ .collect();
+ for (key, value) in renamed {
Review Comment:
warning: When two keys rename to one name, `apply_field_mappings` silently drops one of the two values. Log a warning on the collision, or reject the mapping when the config loads.
##########
core/connectors/sdk/src/encoders/avro.rs:
##########
@@ -444,6 +425,20 @@ mod tests {
assert!(result.is_ok());
let encoded_data = result.unwrap();
assert!(!encoded_data.is_empty());
+
+ // Decode the real bytes and check the value; schema strictness alone isn't proof the rename worked.
+ let schema = AvroSchema::parse_str(&schema_json).unwrap();
+ let mut reader: &[u8] = &encoded_data;
+ let decoded = GenericDatumReader::builder(&schema)
+ .build()
+ .unwrap()
+ .read_value(&mut reader)
+ .unwrap();
+ let decoded_json = serde_json::Value::try_from(decoded).unwrap();
+
+ assert_eq!(decoded_json["name"], serde_json::json!("Alice"));
+ assert_eq!(decoded_json["age"], serde_json::json!(30));
+ assert!(decoded_json.get("full_name").is_none());
Review Comment:
simplification: `decoded_json` holds only the schema fields `name` and `age`, so this assertion can never fail. Delete the line; the equality check on line 439 already proves the rename.
##########
core/connectors/sdk/src/encoders/proto.rs:
##########
@@ -279,37 +279,17 @@ impl ProtoStreamEncoder {
Ok(())
}
- fn apply_field_transformations(&self, payload: Payload) -> Result<Payload, Error> {
+ fn apply_field_transformations(&self, payload: Payload) -> Payload {
if let Some(mappings) = &self.config.field_mappings {
- match payload {
- Payload::Json(json_value) => {
- if let simd_json::OwnedValue::Object(mut map) = json_value {
- let mut new_entries = Vec::new();
-
- for (key, value) in map.iter() {
- let proto_key = mappings
- .iter()
- .find(|(_, json_name)| *json_name == key)
- .map(|(proto_name, _)| proto_name.clone())
- .unwrap_or_else(|| key.clone());
-
- new_entries.push((proto_key, value.clone()));
- }
-
- map.clear();
- for (key, value) in new_entries {
- map.insert(key, value);
- }
-
- Ok(Payload::Json(simd_json::OwnedValue::Object(map)))
- } else {
- Ok(Payload::Json(json_value))
- }
- }
- other => Ok(other),
- }
+ apply_field_mappings(payload, |key| {
+ mappings
+ .iter()
+ .find(|(_, json_name)| json_name.as_str() == key.as_str())
Review Comment:
warning: The proto encoder renames to an arbitrary proto field when two mappings share a JSON name, so a restart can change the wire field. Sort the candidates, or reject duplicate JSON names at config load.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] perf(connectors): drain field_mappings instead of clone-then-clear [iggy]
Posted by "mattp5657 (via GitHub)" <gi...@apache.org>.
mattp5657 commented on PR #4362:
URL: https://github.com/apache/iggy/pull/4362#issuecomment-5955676105
@hubcio For these warnings I kept parity with master to keep this PR single concern.
I opened #4386 to cover them, including the proto reverse lookup.
A runtime check added complexity and gave false positives (simd_json keeps duplicate keys when parsing), so I'm proposing a config check at load time plus a lighter payload safeguard. Happy to discuss on the issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org