Thank you all for your feedback, and sorry for the long wait for a reply. I would like to explore the idea of JSONPath-inspired/subset notation a bit further:
It will need to be a much-reduced version of JSONPath: - No full support for JsonPath therefore an additional dependency. - All paths must start with '$' - No functions support or other operators allowed. - JsonPath dotted and square-bracket notations can be supported to avoid escaping dots or other characters: `$a.b.c` and `$['a.b']['c']` - or we could only support the second one as it's more complete. - Add support for arrays with `[<integer>]`, e.g. `$a.[1].b` - Add support for multiple-value paths using array access `$a.*.b` or deep scan `$a..b`. - Some SMTs that will benefit from this: `MaskField`, `Cast`, `ReplaceField`. - We could introduce a `path[s]` config under the current configurations to apply this feature so no compatibility issues are introduced: e.g. `field.path`, `fields.paths`, `exclude.path`. With these, 100, 101, 102, and 103 will be effectively solved. The main challenge that I see at the moment is that by being JSON path-like, there may be some edge cases that I can't foresee at the moment, that could make this hard to implement, test, and maintain in the long run. I'll appreciate your feedback on how this JSONPath-based alternative compares to the dotted notation initially proposed, and if it matches your feedback. I have drafted a copy of the KIP to change the examples to JSONPath approach and validate some differences: https://cwiki.apache.org/confluence/x/9BihD About 104. I agreed with Chris. We can handle this as part of a new KIP. Thanks, Jorge. On Sun, 24 Apr 2022 at 17:27, Chris Egerton <fearthecel...@gmail.com> wrote: > Hi Joshua, > > I have a few reservations about using JsonPath notation here. > > 1. There's likely to be a substantial performance penalty for converting > between the Kafka Connect format and something that a JsonPath library > would understand. > > 2. The complexity of the feature will be significantly higher. It will be > harder to test, document, and implement. There will be many more edge cases > to consider and support, and we'll be on the hook to handle any bugs or > inconsistencies that arise, either as a result of our use of the JsonPath > library we choose, or as a result of bugs in that library. > > 3. It's not clear that JsonPath is superior or even suitable for some of > the SMTs proposed here. What would be the advantages of JsonPath with the > InsertField or HoistField SMTs? > > I also don't think that adding dot notation is unfriendly to users; many > have proposed this type of syntax in the past, and it's frequently used in > informal discussions to refer to nested fields. If the proposed syntax was > not already intuitive then a case against deciding on our own might be more > convincing, but as things stand, simple dot notation is likely going to be > easier for users to understand than JsonPath syntax. > > Cheers, > > Chris > > On Fri, Apr 22, 2022 at 9:31 AM Joshua Grisham <grishamj...@gmail.com> > wrote: > > > Hello all! > > > > Sorry that I come a bit later to the party here, but I am the one who > wrote > > KIP-683 [1] for recursive support (just simply looping through all child > > non-primitive structures for the same matching name(s)) which is a > slightly > > different way to try and solve a similar requirement -- unfortunately at > > the time the dev community was not quite as active and then I also got > busy > > with work and just life in general so wasn't able to follow up or push > it. > > > > I do think it is a very good idea to have some kind of path-like > expression > > to be able to specifically address a nested field, as I can see that the > > simple "recursive" case could potentially result in unwanted or > unexpected > > behavior, plus there is the potential to introduce a bit of a performance > > hit to always loop through everything in cases where the schemas/values > > might be quite large. > > > > One thing I wanted to ask: instead of creating a new "path parser" > > including some bespoke or "borrowed" syntax, why not just use something > > that already exists? Specifically here I am thinking about JsonPath ( > > https://github.com/json-path/JsonPath) > > > > There is already quite nice support in JsonPath for handling special > > characters in field names, for handling different non-primitive types > > (arrays etc), for handling multiple levels of nesting, etc etc. Would it > > be possible to instead to re-think this and maybe have some kind of > > JsonPath-based Schema selector / updater and/or JsonPath-based Value > > selector / updater? Conceptually this feels like it makes sense to me, as > > from the top of my head it would be quite a natural fit to map a Json > data > > structure to the Connect API data structure (and you could potentially > even > > try to leverage the existing Json-to-Connect serializer/deserializer to > > help out with this even in a more "out of the box"-feeling kind of way). > > > > Maybe also as Tom mentioned, this part (in my example, this > JsonPath-based > > "thing") could even be a generic API that could be used by any SMT, > > including used in custom ones built by the community. Then I think to > use > > a completely separate config property somehow related to "path" (as Tom > > also mentioned) would also make a lot of sense here as well. This way, if > > you select based on "path" then this JsonPath-based API would be used, > > otherwise it could use something similar to the existing get-field based > > approach (which I guess could also be refactored into some kind of > utility > > / API as well if it made sense?) > > > > And with that in mind, if this was the kind of direction to go, then a > > "recursive" capability like I pitched in KIP-683 would also become > > unnecessary because you could easily write a JsonPath expression like > > "$..someRecuriveField" and it would do the same thing (on top of anything > > else you would want to do that is already supported by JsonPath). Then we > > could also kill that older KIP and do a bit of clean-up :) > > > > [1] - > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-683%3A+Add+recursive+support+to+Connect+Cast+and+ReplaceField+transforms%2C+and+support+for+casting+complex+types+to+either+a+native+or+JSON+string > > > > Just some extra food for thought. All-in-all I think this is a super > great > > initiative! > > > > Best, > > > > Joshua > > > > > > Den fre 22 apr. 2022 kl 14:50 skrev Chris Egerton < > fearthecel...@gmail.com > > >: > > > > > Hi Tom, > > > > > > Thanks for taking a look at this, and for your thoughtful comments. > I'll > > > leave it up to Jorge to address most of your comments but I wanted to > > share > > > a couple quick thoughts I had regarding 103 and 104. > > > > > > 103. Like you, I was envisioning a possible syntax for array access > that > > > used classic C-style brackets; e.g., `arr[index]`. However, I wonder if > > we > > > could keep things simple and use the same syntax that we're proposing > for > > > nested field access? In other words, instead of `arr[index]`, you'd > write > > > `arr.index`. It'd save us and users the headache of reserving > characters > > > now that would need to be escaped even if their unescaped brethren > aren't > > > used for anything, and also avoid the question of what exactly we > should > > do > > > when we see a config that uses reserved characters that aren't yet > > > supported (throwing an exception seems pretty unfriendly for new > users). > > > > > > 104. This would probably be useful, but it would come with some nasty > > > compatibility questions that would need to be addressed if we'd want > SMTs > > > that leverage this new API to be viable for older versions of Connect. > If > > > we package and distribute this feature as a library (either via an > > entirely > > > new artifact, or as part of the existing connect-transforms or > > connect-api > > > artifacts), then we'd have to either sidestep the existing plugin > > isolation > > > logic [1] that basically makes it impossible for Connect plugins to > ship > > > their own versions of Connect artifacts, or issue a big warning to > people > > > that any SMT that uses this API won't work with any older versions of > > > Connect. There's also some other features we might want to add in an > > > SMT-utils library such as the existing, internal, utils that Connect > uses > > > right now [2]. It may be worth exploring this in a separate KIP of its > > own. > > > > > > [1] - > > > > > > > > > https://github.com/apache/kafka/blob/d480c4aa6e513e36050d8e067931de2270525d18/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation/PluginUtils.java#L46-L143 > > > > > > [2] - > > > > > > > > > https://github.com/apache/kafka/tree/d480c4aa6e513e36050d8e067931de2270525d18/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/util > > > > > > Cheers, > > > > > > Chris > > > > > > On Fri, Apr 22, 2022 at 6:55 AM Tom Bentley <tbent...@redhat.com> > wrote: > > > > > > > Hi Jorge, > > > > > > > > Thanks for the KIP, especially for the examples which are > super-clear. > > > > > > > > 100. The name `field.style` isn't so clear for something like > > > ReplaceField: > > > > it's not so obvious that field.style applies to `include` and > > `exclude`. > > > > > > > > 101. The permitted values for `field.style` don't seem terribly > > intuitive > > > > (to me anyway): the meaning of `plain` isn't very guessable. Why not > > > > `top-level` or `root` instead? Also `nested` could be misconstrued to > > > mean > > > > nested-but-not-top-level, so perhaps `recursive` or `cascading` might > > be > > > > better? > > > > > > > > 102. I'm torn on whether making the interpretation of existing > configs > > > like > > > > `field` be dependent on `field.style` is a good idea. I can see that > > it's > > > > the simplest thing to do, but it just feels a bit odd that sometimes > > the > > > > `field` would actually be a path and have different escaping rules. > An > > > > alternative would be to come up with a parallel set of config names > > (e.g. > > > > as well as "field" an SMT might support "path") which were defined to > > > > always take paths, thus avoiding the changeable interpretation of the > > > > existing configs. The SMT's #configure() would need to throw in the > > case > > > > that both configs were given. I can see that that would be more work > in > > > > implementation, but it feels cleaner. > > > > > > > > 103. I think in order to allow for supporting arrays in a later KIP > > > (which > > > > certainly seems like it could be useful), we'd want to specify the > > syntax > > > > now, even if it wasn't implemented under this KIP. That's because I > > don't > > > > think you can't exclude the possibility that characters such as `[` > and > > > `]` > > > > appear in field names. So you'd have a compatibility problem if > people > > > > started using the features of this KIP to access such fields, only > for > > > > those characters to change their meaning under a later KIP. > > > > > > > > 104. I also wonder whether making paths into a public Java API, for > use > > > by > > > > 3rd party SMTs, would be valuable. > > > > > > > > Thanks again, > > > > > > > > Tom > > > > > > > > > > > > > > > > On Wed, 20 Apr 2022 at 17:53, Chris Egerton <fearthecel...@gmail.com > > > > > > wrote: > > > > > > > > > 💯 Thanks Jorge, LGTM! > > > > > > > > > > On Wed, Apr 20, 2022, 12:40 Jorge Esteban Quilcate Otoya < > > > > > quilcate.jo...@gmail.com> wrote: > > > > > > > > > > > Thank you, Chris! Not possible without your feedback. > > > > > > > > > > > > On Tue, 19 Apr 2022 at 23:04, Chris Egerton < > > fearthecel...@gmail.com > > > > > > > > > > wrote: > > > > > > > > > > > > > Hi Jorge, > > > > > > > > > > > > > > Thank you for sticking through this. I have one small remark > and > > > one > > > > > > small > > > > > > > clarification; assuming you agree with me on them then I'm > ready > > to > > > > > vote > > > > > > on > > > > > > > the KIP. > > > > > > > > > > > > > > 1. InsertField: The "field.on.missing.parent" and > > > > > > "field.on.existing.field" > > > > > > > docs both mention a permitted value of "ingore"; this should be > > > > > "ignore", > > > > > > > right? > > > > > > > > > > > > > > > > > > > Of course, one more typo :) > > > > > > > > > > > > > > > > > > > 2. InsertField: The examples are still missing the > "field.style" > > > > > property > > > > > > > from the configurations. They should all include the property > > > > > > > "transforms.smt1.field.style": "nested", correct? > > > > > > > > > > > > > > > > > > > Yes, it is there. I think I know what you mean now, seems that > > > > Confluence > > > > > > is putting everything in one line when it's in separate lines in > > the > > > > > > editor. > > > > > > Hopefully, it's fixed now. > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks again for working through this, and congratulations on a > > > > > > > well-written KIP! > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > On Tue, Apr 19, 2022 at 2:06 PM Jorge Esteban Quilcate Otoya < > > > > > > > quilcate.jo...@gmail.com> wrote: > > > > > > > > > > > > > > > Thank you, Chris! I apply these improvements to the KIP, let > me > > > > know > > > > > > how > > > > > > > it > > > > > > > > looks now. > > > > > > > > > > > > > > > > On Mon, 11 Apr 2022 at 23:43, Chris Egerton < > > > > fearthecel...@gmail.com > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > Hi Jorge, > > > > > > > > > > > > > > > > > > Wow, those examples are great! A few more remarks, but I > > think > > > > > we're > > > > > > > > > getting close: > > > > > > > > > > > > > > > > > > 1. The examples differ across SMTs with the name of the > > > > > > > newly-introduced > > > > > > > > > style property; some of them use "field.style", and some > use > > > > > > > > > "fields.style". I think for consistency's sake we should > > stick > > > > with > > > > > > > just > > > > > > > > > "field.style"; otherwise it could be painful for users to > > have > > > to > > > > > > > > remember > > > > > > > > > which to use. > > > > > > > > > > > > > > > > > > > > > > > > > Great catch. Agree, I fixed the config names to > `field.style`. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. Some of the examples are off: > > > > > > > > > - TimestampConverter: the input in the second example > ("when > > > > field > > > > > > > names > > > > > > > > > include dots") doesn't contain a field with a dotted name > > > > > > > > > - ValueToKey: the config in the third example ("when field > > > names > > > > > > > include > > > > > > > > > dots") should probably use "parent..child.k2" as the > > > > > > > > > "transforms.smt1.fields" property > > > > > > > > > > > > > > > > > > > > > > > > > Fixed. Thanks! > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. RE changes to InsertField: > > > > > > > > > - The InsertField SMT should also come with the new > > > "field.style" > > > > > > > > property > > > > > > > > > in order to preserve backwards compatibility, right? I > don't > > > see > > > > it > > > > > > > > > included in the example configs for that one, just want to > > make > > > > > sure > > > > > > > > > - I don't know of any cases where we use snake_case for > > > property > > > > > > names > > > > > > > in > > > > > > > > > Kafka; we should probably use "on.missing.parent" and > > > > > > > "on.existing.field" > > > > > > > > > as the new property names for InsertField. > > > > > > > > > - Why is the "on_existing_field" (or "on.existing.field") > > > > property > > > > > > only > > > > > > > > > applied when the field style is nested? Couldn't it be > useful > > > for > > > > > > > > > non-nested fields as well? > > > > > > > > > > > > > > > > > > > > > > > > > Great points! I have applied these suggestions to the KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > On Sat, Apr 9, 2022 at 12:40 PM Jorge Esteban Quilcate > Otoya > > < > > > > > > > > > quilcate.jo...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > Again, great feedback Chris. Much appreciated. > > > > > > > > > > Added my comments below: > > > > > > > > > > > > > > > > > > > > On Tue, 5 Apr 2022 at 20:22, Chris Egerton < > > > > > > fearthecel...@gmail.com> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > Hi Jorge, > > > > > > > > > > > > > > > > > > > > > > Looking good! I have a few comments left but all but > one > > or > > > > two > > > > > > are > > > > > > > > > > minor. > > > > > > > > > > > > > > > > > > > > > > 1. The motivation section states "This KIP is aimed to > > > > include > > > > > > > > support > > > > > > > > > > for > > > > > > > > > > > nested structures on the existing SMTs... and to > include > > > the > > > > > > > > > abstractions > > > > > > > > > > > to reuse this in future SMTs". A good implementation of > > > this > > > > > KIP > > > > > > > will > > > > > > > > > > > definitely isolate reusable logic into a separate > > > abstraction > > > > > > that > > > > > > > > can > > > > > > > > > be > > > > > > > > > > > easily pulled in to the SMTs we want to add nested > field > > > > > support > > > > > > > to, > > > > > > > > > but > > > > > > > > > > > unless we plan on making this kind of abstraction > > publicly > > > > > > > available > > > > > > > > as > > > > > > > > > > > some kind of utility method or class that external SMT > > > > > developers > > > > > > > can > > > > > > > > > > > leverage, we can probably leave this part out as it's > > more > > > of > > > > > an > > > > > > > > > > > implementation detail. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Make sense, will leave this out of the KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. The Cast example is a little misleading, isn't it? > It > > > > > > > demonstrates > > > > > > > > > the > > > > > > > > > > > escape syntax for fields with dot literals in their > > names, > > > > but > > > > > it > > > > > > > > > doesn't > > > > > > > > > > > demonstrate a way to actually use the Cast (or any > other) > > > SMT > > > > > to > > > > > > > > > access a > > > > > > > > > > > nested field in a record, which is the whole point of > the > > > > KIP. > > > > > I > > > > > > > like > > > > > > > > > the > > > > > > > > > > > example of escape syntax but we should probably also > add > > > one > > > > > for > > > > > > > > nested > > > > > > > > > > > field access. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Agree. I have added examples to each SMT to be more clear > > > about > > > > > how > > > > > > > it > > > > > > > > > > affects each function > > > > > > > > > > . > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. With the InsertField SMT, I'm wondering what the > > > specific > > > > > > > behavior > > > > > > > > > > will > > > > > > > > > > > be when some or all of the "middle layer" nested fields > > are > > > > > > > missing. > > > > > > > > > For > > > > > > > > > > > example, if I have a record with a value of { "k1": > "v1 } > > > > and I > > > > > > > apply > > > > > > > > > > > InsertField with topic.field = n1.n2.n3.topic, what > will > > > > > happen? > > > > > > > Will > > > > > > > > > the > > > > > > > > > > > updated value become { "k1": "v1", "n1": { "n2": { > "n3": > > > > > "topic" > > > > > > > }}}, > > > > > > > > > > will > > > > > > > > > > > an exception be thrown, or something else? This seems > > > > analogous > > > > > > to > > > > > > > > the > > > > > > > > > > > command line mkdir command, which (at least on some > > > operating > > > > > > > > systems) > > > > > > > > > > > fails by default if you try to create a new nested > > > directory > > > > > > where > > > > > > > > > > anything > > > > > > > > > > > but the last element in the path doesn't exist, but can > > be > > > > > > invoked > > > > > > > > > with a > > > > > > > > > > > flag that instructs it to go ahead and create all > levels > > of > > > > > > nested > > > > > > > > > > > directory instead. I'm leaning on the side of "just > > create > > > > > > > > everything" > > > > > > > > > > but > > > > > > > > > > > would be interested in your thoughts, and either way, > we > > > > should > > > > > > > > > probably > > > > > > > > > > > make sure the intended behavior is well-defined before > > > > voting. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This is an interesting case, thanks for catching this! > > > > > > > > > > The default behavior I see is to create parents if they > are > > > > > missing > > > > > > > and > > > > > > > > > > overwrite fields > > > > > > > > > > if they already exist. > > > > > > > > > > I'm planning to include the following two flags if there > > is a > > > > > need > > > > > > to > > > > > > > > > > overwrite this behavior: > > > > > > > > > > - `on_missing_parent` = (CREATE|IGNORE), default=CREATE > > > > > > > > > > - `on_existing_field` = (OVERWRITE|IGNORE), > > default=OVERWRITE > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. Similarly, what will the behavior be if any of the > > field > > > > > > > elements > > > > > > > > > > > specified with InsertField already exist in the record > > > value? > > > > > > Will > > > > > > > we > > > > > > > > > > just > > > > > > > > > > > overwrite them? What's the behavior of InsertField > today > > > > under > > > > > > > > similar > > > > > > > > > > > circumstances? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > The current behavior is to overwrite the value. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > On Thu, Mar 31, 2022 at 4:15 PM Jorge Esteban Quilcate > > > Otoya > > > > < > > > > > > > > > > > quilcate.jo...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > Thanks, Chris! Much appreciated all the feedback > here. > > > > > > > > > > > > > > > > > > > > > > > > 1. You nailed it setting the design goal here: "it > > > > shouldn't > > > > > be > > > > > > > > > > > impossible > > > > > > > > > > > > to use this new feature for any field name, no matter > > how > > > > > > > > convoluted. > > > > > > > > > > > It's > > > > > > > > > > > > fine if edge cases introduce difficulty (such as > > > > > less-readable > > > > > > > > > > > > configurations), but it's not fine if they can't be > > > > addressed > > > > > > at > > > > > > > > > all." > > > > > > > > > > > > Back to the previous proposals (using only dots as > > > > > separators) > > > > > > we > > > > > > > > > have > > > > > > > > > > 2 > > > > > > > > > > > > alternatives: > > > > > > > > > > > > 1. escaping with backslashes > > > > > > > > > > > > 2. escaping with dots itself > > > > > > > > > > > > > > > > > > > > > > > > I'll lean for alternative 2, as you proposed before. > > > Feels > > > > to > > > > > > me > > > > > > > > that > > > > > > > > > > > > backslashes have more potential to lead to confusion > in > > > > JSON > > > > > > > > configs, > > > > > > > > > > and > > > > > > > > > > > > CSV seems like a good precedent to use the same > > character > > > > to > > > > > > > escape > > > > > > > > > > > itself. > > > > > > > > > > > > KIP is updated to reflect this. > > > > > > > > > > > > > > > > > > > > > > > > 2. Thanks! I'll add an example, and stick with the > > > current > > > > > > > approach > > > > > > > > > > > > defining the style per individual transform > > > configuration. > > > > > > > > > > > > > > > > > > > > > > > > 3. Yes, thanks! KIP updated. > > > > > > > > > > > > > > > > > > > > > > > > 4. Of course. KIP updated. > > > > > > > > > > > > > > > > > > > > > > > > On Mon, 28 Mar 2022 at 21:59, Chris Egerton < > > > > > > > > fearthecel...@gmail.com > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > Hi Jorge, > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for addressing my comments; the KIP looks > > > > up-to-date > > > > > > and > > > > > > > > > > pretty > > > > > > > > > > > > > readable now, and the rejected alternatives section > > > does > > > > a > > > > > > > great > > > > > > > > > job > > > > > > > > > > of > > > > > > > > > > > > > outlining the discussion so far and providing > context > > > for > > > > > > > anyone > > > > > > > > > else > > > > > > > > > > > who > > > > > > > > > > > > > might want to join in. > > > > > > > > > > > > > > > > > > > > > > > > > > 1. Thoughts on choice of delimiter: > > > > > > > > > > > > > - I like the optimization for simple cases, but I > > think > > > > the > > > > > > new > > > > > > > > > > > proposal > > > > > > > > > > > > is > > > > > > > > > > > > > a little too restrictive. What if there's a field > > whose > > > > > name > > > > > > > > > contains > > > > > > > > > > > all > > > > > > > > > > > > > of the permitted options (currently just ".", ",", > > and > > > > > "/")? > > > > > > > > > > > > > - If we expand the set of permitted delimiters to > > allow > > > > for > > > > > > any > > > > > > > > > > > > > single-character string, configuration complexity > > will > > > > > > increase > > > > > > > > and > > > > > > > > > > > > > readability may decrease > > > > > > > > > > > > > - Also worth pointing out that there is some > > convention > > > > for > > > > > > > > > doubling > > > > > > > > > > a > > > > > > > > > > > > > delimiter character as an escape mechanism with > > formats > > > > > like > > > > > > > CSV > > > > > > > > > [1] > > > > > > > > > > > > > - Overall I think we may be approaching the > > saturation > > > > > point > > > > > > > for > > > > > > > > > > > > productive > > > > > > > > > > > > > discussion on delimiter syntax so I don't want to > > spend > > > > too > > > > > > > much > > > > > > > > > more > > > > > > > > > > > of > > > > > > > > > > > > > your time on it. I think the one point I'd like to > > > leave > > > > > for > > > > > > > now > > > > > > > > is > > > > > > > > > > > that > > > > > > > > > > > > it > > > > > > > > > > > > > shouldn't be impossible to use this new feature for > > any > > > > > field > > > > > > > > name, > > > > > > > > > > no > > > > > > > > > > > > > matter how convoluted. It's fine if edge cases > > > introduce > > > > > > > > difficulty > > > > > > > > > > > (such > > > > > > > > > > > > > as less-readable configurations), but it's not fine > > if > > > > they > > > > > > > can't > > > > > > > > > be > > > > > > > > > > > > > addressed at all. > > > > > > > > > > > > > > > > > > > > > > > > > > 2. > > > > > > > > > > > > > The configuration style where you define > > > > > > > "transforms.field.style" > > > > > > > > > in > > > > > > > > > > > the > > > > > > > > > > > > > connector config, and then this applies to all SMTs > > for > > > > the > > > > > > > > > > connector, > > > > > > > > > > > is > > > > > > > > > > > > > very interesting. However, it doesn't follow > > convention > > > > for > > > > > > > > > existing > > > > > > > > > > > > SMTs. > > > > > > > > > > > > > Right now, if you want to configure an SMT, you > > define > > > > its > > > > > > name > > > > > > > > in > > > > > > > > > > the > > > > > > > > > > > > > connector config (for example, "transforms": > "smt1"), > > > and > > > > > > then > > > > > > > > > define > > > > > > > > > > > all > > > > > > > > > > > > > of the properties for that SMT in the connector > > config > > > > > using > > > > > > a > > > > > > > > > > > > namespacing > > > > > > > > > > > > > mechanism specific to that SMT (for example, > > > > > > > > > "transforms.smt1.prop1": > > > > > > > > > > > > > "val1"). That SMT then sees only the properties > > defined > > > > in > > > > > > that > > > > > > > > > > > > namespace, > > > > > > > > > > > > > with the prefix stripped (for example, "prop1": > > "val1") > > > > in > > > > > > its > > > > > > > > > > > configure > > > > > > > > > > > > > [2] [3] method. > > > > > > > > > > > > > If we want to continue to follow this convention, > > then > > > > > > instead > > > > > > > of > > > > > > > > > > > > > specifying "transforms.field.style" in a connector > > > > config, > > > > > we > > > > > > > > would > > > > > > > > > > > > expect > > > > > > > > > > > > > users to configure "transforms.<name>.field.style", > > for > > > > > each > > > > > > > SMT > > > > > > > > > that > > > > > > > > > > > > they > > > > > > > > > > > > > want to configure a field style for. This would > > require > > > > > more > > > > > > > work > > > > > > > > > on > > > > > > > > > > > the > > > > > > > > > > > > > part of the user, but would be simpler to reason > > about > > > > and > > > > > > > easier > > > > > > > > > to > > > > > > > > > > > > > implement. > > > > > > > > > > > > > If we want to explore an alternative where users > can > > > > > specify > > > > > > > > global > > > > > > > > > > > > > properties that apply to all transforms in a > > connector > > > > > > config, > > > > > > > > then > > > > > > > > > > the > > > > > > > > > > > > > semantics for this need to be defined in the KIP. > > This > > > > > would > > > > > > > have > > > > > > > > > to > > > > > > > > > > > > > include whether this will apply only for the > special > > > case > > > > > of > > > > > > > the > > > > > > > > > > > > > "field.style" and possibly "field.separator" > > properties > > > > or > > > > > if > > > > > > > it > > > > > > > > > > would > > > > > > > > > > > be > > > > > > > > > > > > > available more generally for other properties, > > whether > > > it > > > > > > will > > > > > > > > > apply > > > > > > > > > > > only > > > > > > > > > > > > > for the SMTs outlined in the KIP or if the > > > "field.style" > > > > > and > > > > > > > > > possibly > > > > > > > > > > > > > "field.separator" properties would also be passed > > into > > > > > custom > > > > > > > > SMTs > > > > > > > > > so > > > > > > > > > > > > that > > > > > > > > > > > > > they could choose to act on them if applicable, how > > > edge > > > > > > cases > > > > > > > > like > > > > > > > > > > > > having > > > > > > > > > > > > > an SMT named "field" in your connector config would > > be > > > > > > handled, > > > > > > > > > etc. > > > > > > > > > > > > > Either way, it might help to have an example in the > > KIP > > > > > > > outlining > > > > > > > > > how > > > > > > > > > > > one > > > > > > > > > > > > > of the to-be-augmented SMTs can be configured with > > this > > > > new > > > > > > > > feature > > > > > > > > > > > and a > > > > > > > > > > > > > before/after of how a record value would be > > transformed > > > > > with > > > > > > > that > > > > > > > > > > > > > configuration. > > > > > > > > > > > > > > > > > > > > > > > > > > 3. The docstring for the "transforms.field.style" > > > > property > > > > > > > > mentions > > > > > > > > > > > that > > > > > > > > > > > > > the permitted values are "plain" and "nested", but > > then > > > > > > > describes > > > > > > > > > > > > behavior > > > > > > > > > > > > > with a value of "root". Should that be "plain" > > instead? > > > > > > > > > > > > > > > > > > > > > > > > > > 4. The docstring for the > "transforms.field.separator" > > > > > > property > > > > > > > > > > > > exclusively > > > > > > > > > > > > > mentions structs, but the feature is intended to > work > > > > with > > > > > > maps > > > > > > > > as > > > > > > > > > > > well. > > > > > > > > > > > > > Can we update it to reflect this? > > > > > > > > > > > > > > > > > > > > > > > > > > References: > > > > > > > > > > > > > > > > > > > > > > > > > > [1] - https://stackoverflow.com/a/17808731 > > > > > > > > > > > > > [2] - > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/kafka/blob/7243facb8d69a7252e6b9556b5eaee13e41bab7f/connect/api/src/main/java/org/apache/kafka/connect/transforms/Transformation.java#L30 > > > > > > > > > > > > > [3] - > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://github.com/apache/kafka/blob/7243facb8d69a7252e6b9556b5eaee13e41bab7f/clients/src/main/java/org/apache/kafka/common/Configurable.java#L26-L29 > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Mar 28, 2022 at 1:32 PM Jorge Esteban > > Quilcate > > > > > Otoya > > > > > > < > > > > > > > > > > > > > quilcate.jo...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, Chris! > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. I'd argue "this..field.child" could be harder > to > > > > grasp > > > > > > > than > > > > > > > > > > > > > > "this.field/child" + separator: "/". > > > > > > > > > > > > > > Even though this represents additional > information, > > > it > > > > > > > follows > > > > > > > > a > > > > > > > > > > > > similar > > > > > > > > > > > > > > approach as the "Flatten#delimeter" > configuration. > > > > > > > > > > > > > > I want to give the separator approach another > try, > > > so I > > > > > > have > > > > > > > > > > updated > > > > > > > > > > > > the > > > > > > > > > > > > > > KIP with the separator proposal, sticking to > only 2 > > > > > > > > alternatives > > > > > > > > > > that > > > > > > > > > > > > > > should hopefully cover most scenarios. > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. Agree. KIP has been updated with this > > improvement. > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. You're right. I have updated this section > > > > accordingly. > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. Good catch! I've replaced it with > `DropHeaders`. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Looking forward to your feedback. > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > Jorge. > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, 9 Mar 2022 at 21:33, Chris Egerton < > > > > > > > > > > fearthecel...@gmail.com> > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Jorge, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Looking good! Got a few more thoughts. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. Sorry to revisit this, but I think we may > want > > > to > > > > > > adopt > > > > > > > a > > > > > > > > > > > slightly > > > > > > > > > > > > > > > different escape syntax style. Backslashes are > > > great, > > > > > but > > > > > > > > since > > > > > > > > > > > > they're > > > > > > > > > > > > > > > already used by JSON, using them as an escape > > > > sequence > > > > > in > > > > > > > > field > > > > > > > > > > > > > notation > > > > > > > > > > > > > > > would also lead to some pretty ugly connector > > > > configs. > > > > > > > Anyone > > > > > > > > > > who's > > > > > > > > > > > > had > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > write regular expressions with backslashes in > > Java > > > is > > > > > > > > probably > > > > > > > > > > > > already > > > > > > > > > > > > > > > familiar with this: > > > > > > > > > "this\\\\.is\\\\.not\\\\.very\\\\.readable". > > > > > > > > > > > What > > > > > > > > > > > > > do > > > > > > > > > > > > > > > you think about using the dot character to > escape > > > > > itself? > > > > > > > In > > > > > > > > > > other > > > > > > > > > > > > > words, > > > > > > > > > > > > > > > to access a single field named "this.field", > > > instead > > > > of > > > > > > > using > > > > > > > > > the > > > > > > > > > > > > > syntax > > > > > > > > > > > > > > > "this\.field" (which in JSON would have to be > > > > expressed > > > > > > as > > > > > > > > > > > > > > "this\\.field"), > > > > > > > > > > > > > > > we could use "this..field", and for a single > > field > > > > > named > > > > > > > > > > > > "this\field", > > > > > > > > > > > > > > > instead of using the syntax "this\\field" (or, > in > > > > JSON, > > > > > > > > > > > > > "this\\\\field"), > > > > > > > > > > > > > > > we could use "this\field" (or, in JSON, > > > > "this\\field"). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. Could you flesh out the details on the new > > > > > > "field.style" > > > > > > > > > > > property, > > > > > > > > > > > > > > > including the type, default value, importance, > > and > > > a > > > > > > > > > preliminary > > > > > > > > > > > > > > docstring? > > > > > > > > > > > > > > > See > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-618%3A+Exactly-Once+Support+for+Source+Connectors#KIP618:ExactlyOnceSupportforSourceConnectors-Newproperties > > > > > > > > > > > > > > > for an example. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. Is the "Compatibility, Deprecation, and > > > Migration > > > > > > Plan" > > > > > > > > > > section > > > > > > > > > > > > > still > > > > > > > > > > > > > > > accurate after the latest update? Seems like > it's > > > > still > > > > > > > > written > > > > > > > > > > > with > > > > > > > > > > > > > the > > > > > > > > > > > > > > > assumption that nested field syntax will be > > > hardcoded > > > > > or > > > > > > > > > opt-in, > > > > > > > > > > > > which > > > > > > > > > > > > > > IIUC > > > > > > > > > > > > > > > isn't the case anymore. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. Nit: The "These SMTs do not require nested > > > > structure > > > > > > > > > support" > > > > > > > > > > > > > section > > > > > > > > > > > > > > > mentions a "Drop" SMT. I think this may be > > > referring > > > > to > > > > > > the > > > > > > > > > > > Confluent > > > > > > > > > > > > > > Drop > > > > > > > > > > > > > > > SMT, which isn't a part of Apache Kafka. Should > > we > > > > drop > > > > > > > (heh) > > > > > > > > > > that > > > > > > > > > > > > SMT > > > > > > > > > > > > > > from > > > > > > > > > > > > > > > the list? Or perhaps just replace it with > > > > > "DropHeaders", > > > > > > > > which > > > > > > > > > is > > > > > > > > > > > > > > currently > > > > > > > > > > > > > > > missing from the list and shouldn't require any > > > > > > > nested-field > > > > > > > > > > > related > > > > > > > > > > > > > > > updates? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, Feb 28, 2022 at 2:12 PM Jorge Esteban > > > > Quilcate > > > > > > > Otoya > > > > > > > > < > > > > > > > > > > > > > > > quilcate.jo...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thank you, Chris! and sorry for the delayed > > > > response. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Please, find my comments below: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Mon, 14 Feb 2022 at 17:34, Chris Egerton > > > > > > > > > > > > > > <chr...@confluent.io.invalid > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Jorge, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks for the KIP! I'd love to see support > > for > > > > > > nested > > > > > > > > > fields > > > > > > > > > > > > added > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > out-of-the-box SMTs provided with Connect. > > Here > > > > are > > > > > > my > > > > > > > > > > initial > > > > > > > > > > > > > > > thoughts: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 1. I agree that there's a case to be made > for > > > > > > expanding > > > > > > > > > > > > HoistField > > > > > > > > > > > > > > > with a > > > > > > > > > > > > > > > > > new config property for identifying a > nested, > > > > > > > > to-be-hoisted > > > > > > > > > > > > field, > > > > > > > > > > > > > > but > > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > example in the KIP doesn't really > demonstrate > > > why > > > > > > this > > > > > > > > > would > > > > > > > > > > be > > > > > > > > > > > > > > > > valuable. I > > > > > > > > > > > > > > > > > think it'd be helpful to expand the example > > to > > > > add > > > > > > > other > > > > > > > > > > fields > > > > > > > > > > > > in > > > > > > > > > > > > > > > order > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > show how adding nested field support > enables > > > > users > > > > > to > > > > > > > > > hoist a > > > > > > > > > > > > > nested > > > > > > > > > > > > > > > > field > > > > > > > > > > > > > > > > > without dropping other fields from the > value. > > > > Maybe > > > > > > > > > something > > > > > > > > > > > > like > > > > > > > > > > > > > > > this: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > source = nested.val > > > > > > > > > > > > > > > > > field = line > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > value (before): > > > > > > > > > > > > > > > > > { > > > > > > > > > > > > > > > > > "nested": { > > > > > > > > > > > > > > > > > "val": 42, > > > > > > > > > > > > > > > > > "other val": 96 > > > > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > value (after): > > > > > > > > > > > > > > > > > { > > > > > > > > > > > > > > > > > "nested": { > > > > > > > > > > > > > > > > > "line": { > > > > > > > > > > > > > > > > > "val": 42, > > > > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > "other val": 96 > > > > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > } > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2. Nit: I think "source" is a little > strange > > > for > > > > > the > > > > > > > new > > > > > > > > > > > > HoistField > > > > > > > > > > > > > > > > > property name. Maybe "hoisted" or > > > "hoisted.field" > > > > > > would > > > > > > > > be > > > > > > > > > > more > > > > > > > > > > > > > > > > > descriptive? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > About 1. and 2.: > > > > > > > > > > > > > > > > Agree. The example for this SMT is updated > and > > > have > > > > > > added > > > > > > > > the > > > > > > > > > > > > > `hoisted` > > > > > > > > > > > > > > > > configuration. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 3. Is there a reasonable use case for > > expanding > > > > > > Flatten > > > > > > > > to > > > > > > > > > be > > > > > > > > > > > > able > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > flatten specific fields? My understanding > is > > > that > > > > > > it's > > > > > > > > > mostly > > > > > > > > > > > > > useful > > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > writing to systems like databases that > don't > > > > > support > > > > > > > > nested > > > > > > > > > > > > values > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > require everything to be a flat list of > > > key-value > > > > > > > pairs. > > > > > > > > > > Being > > > > > > > > > > > > able > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > flatten a nested field wouldn't provide any > > > > > advantage > > > > > > > for > > > > > > > > > > that > > > > > > > > > > > > use > > > > > > > > > > > > > > > case. > > > > > > > > > > > > > > > > > Are there other cases where it would? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 4. I don't think we should unconditionally > > > change > > > > > the > > > > > > > > > default > > > > > > > > > > > > > > delimiter > > > > > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > Flatten. It's a backwards-incompatible, > > > breaking > > > > > > change > > > > > > > > > that > > > > > > > > > > > > could > > > > > > > > > > > > > > > cause > > > > > > > > > > > > > > > > > headaches for users. It might be reasonable > > to > > > > > change > > > > > > > the > > > > > > > > > > > default > > > > > > > > > > > > > > value > > > > > > > > > > > > > > > > > dynamically based on whether the user has > > > > > specified a > > > > > > > > value > > > > > > > > > > for > > > > > > > > > > > > the > > > > > > > > > > > > > > > > "field" > > > > > > > > > > > > > > > > > property, but considering the motivation > for > > > > > changing > > > > > > > the > > > > > > > > > > > default > > > > > > > > > > > > > is > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > it creates conflicts with the > > to-be-introduced > > > > > nested > > > > > > > > field > > > > > > > > > > > > syntax > > > > > > > > > > > > > > > (which > > > > > > > > > > > > > > > > > could arise with downstream SMTs regardless > > of > > > > > > whether > > > > > > > > the > > > > > > > > > > user > > > > > > > > > > > > has > > > > > > > > > > > > > > > > > explicitly configured Flatten with the > > "field" > > > > > > > > property), I > > > > > > > > > > > don't > > > > > > > > > > > > > > know > > > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > > > this would be too useful either. I have > some > > > > > thoughts > > > > > > > > below > > > > > > > > > > on > > > > > > > > > > > > how > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > handle possible conflicts between names > with > > > dots > > > > > in > > > > > > > > their > > > > > > > > > > > names > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > dotted > > > > > > > > > > > > > > > > > syntax for nested field references that > > should > > > > > > > hopefully > > > > > > > > > make > > > > > > > > > > > > > either > > > > > > > > > > > > > > > > change > > > > > > > > > > > > > > > > > unnecessary. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Fair enough. With the support for nested > fields > > > in > > > > > > other > > > > > > > > > SMTs, > > > > > > > > > > > > > Flatten > > > > > > > > > > > > > > > > could stay as it is. > > > > > > > > > > > > > > > > This removes the need for (4) changing > Flatten > > > > config > > > > > > as > > > > > > > > > well. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 5. I think it's fine to expand ExtractField > > to > > > > > > support > > > > > > > > > nested > > > > > > > > > > > > > > notation, > > > > > > > > > > > > > > > > but > > > > > > > > > > > > > > > > > it might be worth noting in the rejected > > > > > alternatives > > > > > > > > > section > > > > > > > > > > > > that > > > > > > > > > > > > > > this > > > > > > > > > > > > > > > > > isn't strictly necessary since you can > > replace > > > > any > > > > > > > single > > > > > > > > > > > > > invocation > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > that SMT that uses nested field notation > with > > > > > > multiple > > > > > > > > > > > > invocations > > > > > > > > > > > > > of > > > > > > > > > > > > > > > it > > > > > > > > > > > > > > > > > that use non-nested notation. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Agree. Adding it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 6. Nit: "RegerRouter" should be > "RegexRouter" > > > in > > > > > the > > > > > > > list > > > > > > > > > of > > > > > > > > > > > SMTs > > > > > > > > > > > > > > that > > > > > > > > > > > > > > > do > > > > > > > > > > > > > > > > > not require nested structure support. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Ack. Fixing it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 7. It may be rare for dots in field names > to > > > > occur > > > > > in > > > > > > > the > > > > > > > > > > wild > > > > > > > > > > > > > > > (although > > > > > > > > > > > > > > > > I > > > > > > > > > > > > > > > > > wouldn't be so certain of this), but unless > > we > > > > want > > > > > > to > > > > > > > > > > inflict > > > > > > > > > > > > > > > headaches > > > > > > > > > > > > > > > > on > > > > > > > > > > > > > > > > > users of Flatten, I think we're going to > have > > > to > > > > > > think > > > > > > > > > about > > > > > > > > > > > > > > conflicts > > > > > > > > > > > > > > > > > between dotted notation and non-nested > fields > > > > whose > > > > > > > names > > > > > > > > > > > contain > > > > > > > > > > > > > > > dots. I > > > > > > > > > > > > > > > > > don't think this is actually such a bad > > thing, > > > > > > though. > > > > > > > I > > > > > > > > > > agree > > > > > > > > > > > > that > > > > > > > > > > > > > > > > dotted > > > > > > > > > > > > > > > > > notation is intuitive and pretty > commonplace > > > (in > > > > > > tools > > > > > > > > like > > > > > > > > > > jq, > > > > > > > > > > > > for > > > > > > > > > > > > > > > > > example), so I'd like it if we could stick > to > > > it. > > > > > > What > > > > > > > > > about > > > > > > > > > > > > > > > introducing > > > > > > > > > > > > > > > > > escape syntax, using a backslash? That way, > > > users > > > > > > could > > > > > > > > > > > > > disambiguate > > > > > > > > > > > > > > > > > between "this.field" (which would refer to > > the > > > > > nested > > > > > > > > field > > > > > > > > > > > > "field" > > > > > > > > > > > > > > > under > > > > > > > > > > > > > > > > > the top-level "this" field), and > > "this\.field" > > > > > (which > > > > > > > > would > > > > > > > > > > > refer > > > > > > > > > > > > > to > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > field named "this.field"). Like with most > > > > languages > > > > > > > that > > > > > > > > > use > > > > > > > > > > > the > > > > > > > > > > > > > > > > backslash > > > > > > > > > > > > > > > > > for escape sequences, it could also be used > > to > > > > > escape > > > > > > > > > itself, > > > > > > > > > > > in > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > event > > > > > > > > > > > > > > > > > that a field name contains a backslash. I > > think > > > > > this > > > > > > is > > > > > > > > > more > > > > > > > > > > > > > > intuitive > > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > simpler than, e.g., adding a new config > > > property > > > > to > > > > > > > > toggle > > > > > > > > > > the > > > > > > > > > > > > > > > delimiter > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > be used when parsing nested field > references. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I like this approach. Adding to the KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 8. I don't think we can unconditionally > turn > > > this > > > > > > > feature > > > > > > > > > on. > > > > > > > > > > > The > > > > > > > > > > > > > > risk > > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > > breaking existing pipelines (especially > ones > > > that > > > > > > > > involve, > > > > > > > > > > for > > > > > > > > > > > > > > > example, a > > > > > > > > > > > > > > > > > combination of the Flatten and Cast SMTs) > is > > > > pretty > > > > > > > > high. I > > > > > > > > > > > think > > > > > > > > > > > > > > this > > > > > > > > > > > > > > > > > should be an opt-in feature, at least until > > the > > > > > next > > > > > > > > major > > > > > > > > > > > > release. > > > > > > > > > > > > > > One > > > > > > > > > > > > > > > > way > > > > > > > > > > > > > > > > > we could accomplish this is by introducing > a > > > new > > > > > > > > > > "field.style" > > > > > > > > > > > > > (name > > > > > > > > > > > > > > > > > obviously subject to change) property with > > > values > > > > > of > > > > > > > > > "plain" > > > > > > > > > > > > > > (default) > > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > > "nested". If set to "plain" then the > current > > > > > > non-nested > > > > > > > > > > > behavior > > > > > > > > > > > > is > > > > > > > > > > > > > > > used, > > > > > > > > > > > > > > > > > and if set to "nested", then the proposed > > > nested > > > > > > > behavior > > > > > > > > > is. > > > > > > > > > > > We > > > > > > > > > > > > > can > > > > > > > > > > > > > > > > > consider updating the default value to > > "nested" > > > > in > > > > > a > > > > > > > > future > > > > > > > > > > > major > > > > > > > > > > > > > > > release > > > > > > > > > > > > > > > > > (or even codify that plan in this KIP, if > > > there's > > > > > > > enough > > > > > > > > > > > support > > > > > > > > > > > > > for > > > > > > > > > > > > > > > it). > > > > > > > > > > > > > > > > > This would also leave the door open for > > adding > > > > > > > recursive > > > > > > > > > > > support > > > > > > > > > > > > to > > > > > > > > > > > > > > > SMTs > > > > > > > > > > > > > > > > in > > > > > > > > > > > > > > > > > the future by adding a permitted value of > > > > > > "recursive". > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 9. One of the linked tickets in the > > > "Motivation" > > > > > > > section, > > > > > > > > > > > > > > > > > > > > > https://issues.apache.org/jira/browse/KAFKA-10640, > > > > > > has > > > > > > > > an > > > > > > > > > > open > > > > > > > > > > > > PR > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > KIP > > > > > > > > > > > > > > > > > that propose adding recursive support to > some > > > > SMTs. > > > > > > > Have > > > > > > > > > you > > > > > > > > > > > > > > considered > > > > > > > > > > > > > > > > > this type of functionality for your KIP? Or > > is > > > > your > > > > > > aim > > > > > > > > to > > > > > > > > > > > stick > > > > > > > > > > > > > > solely > > > > > > > > > > > > > > > > to > > > > > > > > > > > > > > > > > nested field support? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I like the `field.style` configuration flag > > > > approach. > > > > > > > > > > > > > > > > Thanks for pointing out the `recursive` > > approach. > > > > > Will > > > > > > > add > > > > > > > > > > > `nested` > > > > > > > > > > > > > at > > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > moment, let's check the demand for > `recursive` > > to > > > > > > > consider > > > > > > > > it > > > > > > > > > > as > > > > > > > > > > > > part > > > > > > > > > > > > > > of > > > > > > > > > > > > > > > > this or another KIP. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I have added the following on the KIP: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ``` > > > > > > > > > > > > > > > > Future KIPs could extend this support for: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - Recursive notation: name a field and apply > it > > > to > > > > > all > > > > > > > > fields > > > > > > > > > > > > across > > > > > > > > > > > > > > the > > > > > > > > > > > > > > > > schema matching that name. > > > > > > > > > > > > > > > > - Access to arrays: Adding `[]` notation to > > > > represent > > > > > > > > access > > > > > > > > > to > > > > > > > > > > > > > arrays > > > > > > > > > > > > > > > and > > > > > > > > > > > > > > > > applying SMTs to fields within an array. > > > > > > > > > > > > > > > > ``` > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Cheers, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Chris > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Feb 8, 2022 at 1:23 PM Jorge > Esteban > > > > > Quilcate > > > > > > > > > Otoya < > > > > > > > > > > > > > > > > > quilcate.jo...@gmail.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Hi Dev team, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I'd like to start a new discussion thread > > on > > > > > Kafka > > > > > > > > > Connect > > > > > > > > > > > > > KIP-821: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-821%3A+Connect+Transforms+support+for+nested+structures > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > This KIP is aimed to include support for > > > nested > > > > > > > > > structures > > > > > > > > > > on > > > > > > > > > > > > the > > > > > > > > > > > > > > > > > existing > > > > > > > > > > > > > > > > > > SMTs — where this make sense. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Looking forward to your feedback. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > > > > > > Jorge. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >