[GitHub] [camel-kafka-connector] omarsmak opened a new pull request #1052: Convert Struct to Map

classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] omarsmak opened a new pull request #1052: Convert Struct to Map

GitBox

omarsmak opened a new pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052


   Fixes #1046
   
   With PR, the Sink will automatically convert any KafkaConnect Struct data type to Map. Theoretically Camel components doesn't need any data type of struct since it is Kafka Connect specific data types and hence just convert it to map that is propagated to Camel components.
   It converts body, key and headers


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] omarsmak commented on pull request #1052: Convert Struct to Map

GitBox

omarsmak commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784151043


   > Looking at it a bit better I guess this should be a separated smt in core, which should be enabled/disable through configuration as any other smt. Not all the camel components expect a map as body, so converting each struct to map makes sense only for particular components
   
   I thought about it at first but then, the Struct itself it is something very specific to Kafka Connect and hence it is pretty useless downstream into Camel components since none will use Struct anyway and the Struct itself it is a Map with schema, then why the extra step to include the SMT since it is not needed. This PR will only convert the Struct to Map if and only if, the Schema is `Struct`, otherwise it will ignore it.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] oscerd commented on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

oscerd commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784152239


   Lets wait for more feedback :-)


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] lburgazzoli commented on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

lburgazzoli commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784168964


   With my limited knowledge on the topic (so feel free to correct me if I'm wrong), I think this functionality should really be provided as an an optional SMT. I do agree that none of the components know about Kafka Connect Struct but there's no assurance that a Map is any better so we are just making things a little bit more unpredictable because it will depend on the specific component.
   
   How Structs should be translated to something the specific sink component is able to digest is - IMHO - something that need to be explicitly configured to avoid unexpected misbehavior.
   
   
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] omarsmak commented on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

omarsmak commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784179779


   > I do agree that none of the components know about Kafka Connect Struct but there's no assurance that a Map is any better so we are just making things a little bit more unpredictable because it will depend on the specific component.
   
   The thing is, `Struct` itself is a decorated Map with the schema support. I understand your concern here but if you want produce data as Map in Kafka Connect, Kafka Connect side of things has only two options, either go with Map or go with Struct in case you want to preserve the schema information, AvroConverter and JsonConverter will produce for you Structs in case the schema information are needed in downstream sinks. However, in our case, Camel components side of things, we don't acknowledge Structs at all and hence, propagating struct vs map downstream has the same effect on downstream component, e.g: If the component expects a String but instead got a Map or Struct.
   However, I'd agree this should be an SMT in case we know there are Camel components that expect Struct but none need that, and hence I couldn't convince myself to ask the user to include a Struct->Map SMT for something we don't even support downstream and hence I just wanted to be automatically converted.
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] omarsmak edited a comment on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

omarsmak edited a comment on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784179779


   > I do agree that none of the components know about Kafka Connect Struct but there's no assurance that a Map is any better so we are just making things a little bit more unpredictable because it will depend on the specific component.
   
   The thing is, `Struct` itself is a decorated Key:Value with the schema support. I understand your concern here but if you want produce data as Map in Kafka Connect, Kafka Connect side of things has only two options, either go with Map or go with Struct in case you want to preserve the schema information, AvroConverter and JsonConverter will produce for you Structs in case the schema information are needed in downstream sinks. However, in our case, Camel components side of things, we don't acknowledge Structs at all and hence, propagating struct vs map downstream has the same effect on downstream component, e.g: If the component expects a String but instead got a Map or Struct.
   However, I'd agree this should be an SMT in case we know there are Camel components that expect Struct but none need that, and hence I couldn't convince myself to ask the user to include a Struct->Map SMT for something we don't even support downstream and hence I just wanted to be automatically converted.
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] oscerd commented on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

oscerd commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784199214


   I think a tradeoff would be to make this behavior configurable. Like enabling exactly this code and this execution flow only if an option is set up. Maybe adding it as false as default. I think it sounds like a good tradeoff just in case. @lburgazzoli @valdar @omarsmak


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] lburgazzoli commented on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

lburgazzoli commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784202294


   > > I do agree that none of the components know about Kafka Connect Struct but there's no assurance that a Map is any better so we are just making things a little bit more unpredictable because it will depend on the specific component.
   >
   > The thing is, `Struct` itself is a decorated Key:Value with the schema support. I understand your concern here but if you want produce data as Map in Kafka Connect, Kafka Connect side of things has only two options, either go with Map or go with Struct in case you want to preserve the schema information, AvroConverter and JsonConverter will produce for you Structs in case the schema information are needed in downstream sinks. However, in our case, Camel components side of things, we don't acknowledge Structs at all and hence, propagating struct vs map downstream has the same effect on downstream component, e.g: If the component expects a String but instead got a Map or Struct.
   > However, I'd agree this should be an SMT in case we know there are Camel components that expect Struct but none need that, and hence I couldn't convince myself to ask the user to include a Struct->Map SMT for something we don't even support downstream and hence I just wanted to be automatically converted.
   
   My point is that, as input data is component and external system specific, then you can certainly  convert Struct to a Map and maybe you are lucky but that's really not deterministic. I believe SMT would be better because at this stage you requires to have knowledge of the data shape a component support so to me, it looks like an explicit choice users have to to take. But I'm the least knowledgeable here so I may completely miss the point :)
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] lburgazzoli edited a comment on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

lburgazzoli edited a comment on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784202294


   > The thing is, `Struct` itself is a decorated Key:Value with the schema support. I understand your concern here but if you want produce data as Map in Kafka Connect, Kafka Connect side of things has only two options, either go with Map or go with Struct in case you want to preserve the schema information, AvroConverter and JsonConverter will produce for you Structs in case the schema information are needed in downstream sinks. However, in our case, Camel components side of things, we don't acknowledge Structs at all and hence, propagating struct vs map downstream has the same effect on downstream component, e.g: If the component expects a String but instead got a Map or Struct.
   > However, I'd agree this should be an SMT in case we know there are Camel components that expect Struct but none need that, and hence I couldn't convince myself to ask the user to include a Struct->Map SMT for something we don't even support downstream and hence I just wanted to be automatically converted.
   
   My point is that, as input data is component and external system specific, then you can certainly  convert Struct to a Map and maybe you are lucky but that's really not deterministic. I believe SMT would be better because at this stage you requires to have knowledge of the data shape a component support so to me, it looks like an explicit choice users have to to take. But I'm the least knowledgeable here so I may completely miss the point :)
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] omarsmak commented on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

omarsmak commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784212341


   > I think a tradeoff would be to make this behavior configurable. Like enabling exactly this code and this execution flow only if an option is set up. Maybe adding it as false as default. I think it sounds like a good tradeoff just in case. @lburgazzoli @valdar @omarsmak
   
   We can add this as an option, as it is a good compromise. We already do this for other part exchange and properties. If others are fine anyway.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] omarsmak commented on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

omarsmak commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784263832


   @oscerd @lburgazzoli @valdar I have made it configurable and by default is disable (`false`)


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] omarsmak edited a comment on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

omarsmak edited a comment on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784263832


   @oscerd @lburgazzoli @valdar I have made it configurable and by default is disabled (`false`)


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] valdar commented on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

valdar commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784296410


   I understand the point you are making @omarsmak , still it feels not totally right to me to "hardcode" such behaviour for all components (present and future one) by placing it in `core`. A configurable approach is a decent trade off. Another option would be to have a `typeconverter` (in fact you could just use the one in `camel-debezium-common`) that can be enabled/disabled as needed, in such way, if the needs manifest, we could add other `typeconverters` to handle from struct to something else scenarios.
   
   Moreover there is an ongoing effort to use kamelets in ckc, it is not a trivial change that in my opinion is best to do in steps. Once all the steps are completed we could much more easily change things "per connector" so all of this would make much less sense. In the meantime each addiction to `core` makes the first step toward kamelets usage in ckc little bit more difficult....


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] omarsmak commented on pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

omarsmak commented on pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#issuecomment-784302520


   > A configurable approach is a decent trade off.
   
   It is configurable now, but I got what you mean, all what you listed, were in my mind. For example camel-debezium-common dependency which didn't make sense in my head, the SMT option was also in my head but however as I was hesitant due to the headers information and hence I settled on this. Again, I am not convinced of having this as SMT due the reasons I mentioned but if you guys insist on this, I will change this to SMT although now at least is configurable


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] rk3rn3r commented on a change in pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

rk3rn3r commented on a change in pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#discussion_r581187230



##########
File path: core/src/main/java/org/apache/camel/kafkaconnector/CamelSinkTask.java
##########
@@ -225,15 +230,47 @@ If the CamelMainSupport instance fails to be instantiated (ie.: due to missing c
         }
     }
 
-    private static void mapHeader(Header header, String prefix, Map<String, Object> destination) {
+    private void mapHeader(Header header, String prefix, Map<String, Object> destination) {
         final String key = StringHelper.after(header.key(), prefix, header.key());
         final Schema schema = header.schema();
 
         if (schema.type().equals(Schema.BYTES_SCHEMA.type()) && Objects.equals(schema.name(), Decimal.LOGICAL_NAME)) {
             destination.put(key, Decimal.toLogical(schema, (byte[]) header.value()));
         } else {
-            destination.put(key, header.value());
+            destination.put(key, convertValueFromStruct(header.schema(), header.value()));
+        }
+    }
+
+    private Object convertValueFromStruct(Schema schema, Object value) {

Review comment:
       This also converts message keys, so another name like "convertStructToMap()" makes more sense.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] rk3rn3r commented on a change in pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

rk3rn3r commented on a change in pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#discussion_r581188017



##########
File path: core/src/main/java/org/apache/camel/kafkaconnector/CamelSinkTask.java
##########
@@ -110,7 +114,8 @@ public void start(Map<String, String> props) {
             final String headersRemovePattern = config.getString(CamelSinkConnectorConfig.CAMEL_CONNECTOR_REMOVE_HEADERS_PATTERN_CONF);
             mapProperties = config.getBoolean(CamelSinkConnectorConfig.CAMEL_CONNECTOR_MAP_PROPERTIES_CONF);
             mapHeaders = config.getBoolean(CamelSinkConnectorConfig.CAMEL_CONNECTOR_MAP_HEADERS_CONF);
-            
+            convertStructToMap = config.getBoolean(CamelSinkConnectorConfig.CAMEL_SINK_STRUCT_TO_MAP_CONF);

Review comment:
       "isConvertStructToMapEnabled" might be a name that better fits the purpose?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] rk3rn3r commented on a change in pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

rk3rn3r commented on a change in pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#discussion_r581189826



##########
File path: core/src/main/java/org/apache/camel/kafkaconnector/CamelSinkTask.java
##########
@@ -175,8 +180,8 @@ public void put(Collection<SinkRecord> sinkRecords) {
             TaskHelper.logRecordContent(LOG, loggingLevel, record);
 
             Exchange exchange = new DefaultExchange(producer.getCamelContext());
-            exchange.getMessage().setBody(record.value());
-            exchange.getMessage().setHeader(KAFKA_RECORD_KEY_HEADER, record.key());
+            exchange.getMessage().setBody(convertValueFromStruct(record.valueSchema(), record.value()));

Review comment:
       Is this safe for NPEs on NULL-records which can arrive from SMTs under certain conditions?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] rk3rn3r commented on a change in pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

rk3rn3r commented on a change in pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#discussion_r581191758



##########
File path: core/src/main/java/org/apache/camel/kafkaconnector/CamelSinkTask.java
##########
@@ -225,15 +230,47 @@ If the CamelMainSupport instance fails to be instantiated (ie.: due to missing c
         }
     }
 
-    private static void mapHeader(Header header, String prefix, Map<String, Object> destination) {
+    private void mapHeader(Header header, String prefix, Map<String, Object> destination) {
         final String key = StringHelper.after(header.key(), prefix, header.key());
         final Schema schema = header.schema();
 
         if (schema.type().equals(Schema.BYTES_SCHEMA.type()) && Objects.equals(schema.name(), Decimal.LOGICAL_NAME)) {
             destination.put(key, Decimal.toLogical(schema, (byte[]) header.value()));
         } else {
-            destination.put(key, header.value());
+            destination.put(key, convertValueFromStruct(header.schema(), header.value()));

Review comment:
       Does this convert headers to values? I saw somewhere else that there's a way to handover headers (maybe with a prefix?!)




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] rk3rn3r commented on a change in pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

rk3rn3r commented on a change in pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#discussion_r581193345



##########
File path: core/src/main/java/org/apache/camel/kafkaconnector/CamelSinkTask.java
##########
@@ -225,15 +230,47 @@ If the CamelMainSupport instance fails to be instantiated (ie.: due to missing c
         }
     }
 
-    private static void mapHeader(Header header, String prefix, Map<String, Object> destination) {
+    private void mapHeader(Header header, String prefix, Map<String, Object> destination) {
         final String key = StringHelper.after(header.key(), prefix, header.key());
         final Schema schema = header.schema();
 
         if (schema.type().equals(Schema.BYTES_SCHEMA.type()) && Objects.equals(schema.name(), Decimal.LOGICAL_NAME)) {
             destination.put(key, Decimal.toLogical(schema, (byte[]) header.value()));
         } else {
-            destination.put(key, header.value());
+            destination.put(key, convertValueFromStruct(header.schema(), header.value()));
+        }
+    }
+
+    private Object convertValueFromStruct(Schema schema, Object value) {
+        // if we have have the struct to map flag enabled and
+        // if we have a schema of type Struct, we convert it to map, otherwise
+        // we just return the value as it is
+        if (convertStructToMap && schema != null && value != null && Schema.Type.STRUCT == schema.type()) {
+            return toMap((Struct) value);
         }
+
+        return value;
+    }
+
+    private static Map<String, Object> toMap(final Struct struct) {
+        final HashMap<String, Object> fieldsToValues = new HashMap<>();
+
+        struct.schema().fields().forEach(field -> {
+            try {
+                Object value = struct.get(field);
+
+                // recursive call if we have nested structs
+                if (value instanceof Struct) {
+                    fieldsToValues.put(field.name(), toMap((Struct) value));
+                } else {
+                    fieldsToValues.put(field.name(), value);
+                }
+            } catch (DataException e) {

Review comment:
       Isn't DataException only happening on putting content into a Struct but not when adding to a Map?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


Reply | Threaded
Open this post in threaded view
|

[GitHub] [camel-kafka-connector] omarsmak commented on a change in pull request #1052: Convert Struct to Map

GitBox
In reply to this post by GitBox

omarsmak commented on a change in pull request #1052:
URL: https://github.com/apache/camel-kafka-connector/pull/1052#discussion_r581206194



##########
File path: core/src/main/java/org/apache/camel/kafkaconnector/CamelSinkTask.java
##########
@@ -225,15 +230,47 @@ If the CamelMainSupport instance fails to be instantiated (ie.: due to missing c
         }
     }
 
-    private static void mapHeader(Header header, String prefix, Map<String, Object> destination) {
+    private void mapHeader(Header header, String prefix, Map<String, Object> destination) {
         final String key = StringHelper.after(header.key(), prefix, header.key());
         final Schema schema = header.schema();
 
         if (schema.type().equals(Schema.BYTES_SCHEMA.type()) && Objects.equals(schema.name(), Decimal.LOGICAL_NAME)) {
             destination.put(key, Decimal.toLogical(schema, (byte[]) header.value()));
         } else {
-            destination.put(key, header.value());
+            destination.put(key, convertValueFromStruct(header.schema(), header.value()));
+        }
+    }
+
+    private Object convertValueFromStruct(Schema schema, Object value) {
+        // if we have have the struct to map flag enabled and
+        // if we have a schema of type Struct, we convert it to map, otherwise
+        // we just return the value as it is
+        if (convertStructToMap && schema != null && value != null && Schema.Type.STRUCT == schema.type()) {
+            return toMap((Struct) value);
         }
+
+        return value;
+    }
+
+    private static Map<String, Object> toMap(final Struct struct) {
+        final HashMap<String, Object> fieldsToValues = new HashMap<>();
+
+        struct.schema().fields().forEach(field -> {
+            try {
+                Object value = struct.get(field);
+
+                // recursive call if we have nested structs
+                if (value instanceof Struct) {
+                    fieldsToValues.put(field.name(), toMap((Struct) value));
+                } else {
+                    fieldsToValues.put(field.name(), value);
+                }
+            } catch (DataException e) {

Review comment:
       it is happening when Struct is looking up for the field name, if it doesn't find the field name, it will just throw DataException




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[hidden email]


12