You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@baremaps.apache.org by "bchapuis (via GitHub)" <gi...@apache.org> on 2024/06/01 09:23:30 UTC

[PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

bchapuis opened a new pull request, #860:
URL: https://github.com/apache/incubator-baremaps/pull/860

   (no comment)


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#issuecomment-2143416349

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [2 New issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolutions=WONTFIX)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=860&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860)
   
   


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis commented on code in PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#discussion_r1624722329


##########
baremaps-core/src/main/java/org/apache/baremaps/storage/geoparquet/GeoParquetTypeConversion.java:
##########
@@ -69,9 +80,33 @@ public static List<Object> asRowValues(GeoParquetGroup group) {
         case DOUBLE -> values.add(group.getDoubleValue(i));
         case STRING -> values.add(group.getStringValue(i));
         case GEOMETRY -> values.add(group.getGeometryValue(i));
-        case GROUP -> values.add(null); // TODO: values.add(asDataRow(group.getGroupValue(i)));
+        case ENVELOPE -> values.add(group.getEnvelopeValue(i));
+        case GROUP -> values.add(asNested(group.getGroupValue(i)));
       }
     }
     return values;
   }
+
+  public static Map<String, Object> asNested(GeoParquetGroup group) {
+    Map<String, Object> nested = new HashMap<>();
+    Schema schema = group.getSchema();
+    List<Field> fields = schema.fields();
+    for (int i = 0; i < fields.size(); i++) {
+      Field field = fields.get(i);
+      nested.put(field.name(), switch (field.type()) {
+        case BINARY -> group.getBinaryValue(i).getBytes();

Review Comment:
   @sebr72 The problem is that we also need to return Optional for each values in a potentially very big geoparquet file. As for the List, I'm a bit worried about the overhead. It also means that we need to return Optional for required values.
   
   Here are some interesting arguments for and against the use of Optional:
   https://blogs.oracle.com/javamagazine/post/optional-class-null-pointer-drawbacks 



-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#issuecomment-2145590339

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [C Reliability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) (required ≥ A)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860)
   
   ##   
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png '') Catch issues before they fail your Quality Gate with our IDE extension ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png '') [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
   
   


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis commented on code in PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#discussion_r1623367027


##########
baremaps-core/src/main/java/org/apache/baremaps/storage/geoparquet/GeoParquetTypeConversion.java:
##########
@@ -69,9 +80,33 @@ public static List<Object> asRowValues(GeoParquetGroup group) {
         case DOUBLE -> values.add(group.getDoubleValue(i));
         case STRING -> values.add(group.getStringValue(i));
         case GEOMETRY -> values.add(group.getGeometryValue(i));
-        case GROUP -> values.add(null); // TODO: values.add(asDataRow(group.getGroupValue(i)));
+        case ENVELOPE -> values.add(group.getEnvelopeValue(i));
+        case GROUP -> values.add(asNested(group.getGroupValue(i)));
       }
     }
     return values;
   }
+
+  public static Map<String, Object> asNested(GeoParquetGroup group) {
+    Map<String, Object> nested = new HashMap<>();
+    Schema schema = group.getSchema();
+    List<Field> fields = schema.fields();
+    for (int i = 0; i < fields.size(); i++) {
+      Field field = fields.get(i);
+      nested.put(field.name(), switch (field.type()) {
+        case BINARY -> group.getBinaryValue(i).getBytes();

Review Comment:
   This is a good point. I'm not a fan of returning Optional everywhere, but this is something we could consider. Another option could be to return null and let the user check for it. 
   
   @Drabble @sebr72 Which variant do you prefer? 



-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#issuecomment-2143555418

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [5 New issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolutions=WONTFIX)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=860&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860)
   
   


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sebr72 (via GitHub)" <gi...@apache.org>.
sebr72 commented on code in PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#discussion_r1624743224


##########
baremaps-core/src/main/java/org/apache/baremaps/storage/geoparquet/GeoParquetTypeConversion.java:
##########
@@ -69,9 +80,33 @@ public static List<Object> asRowValues(GeoParquetGroup group) {
         case DOUBLE -> values.add(group.getDoubleValue(i));
         case STRING -> values.add(group.getStringValue(i));
         case GEOMETRY -> values.add(group.getGeometryValue(i));
-        case GROUP -> values.add(null); // TODO: values.add(asDataRow(group.getGroupValue(i)));
+        case ENVELOPE -> values.add(group.getEnvelopeValue(i));
+        case GROUP -> values.add(asNested(group.getGroupValue(i)));
       }
     }
     return values;
   }
+
+  public static Map<String, Object> asNested(GeoParquetGroup group) {
+    Map<String, Object> nested = new HashMap<>();
+    Schema schema = group.getSchema();
+    List<Field> fields = schema.fields();
+    for (int i = 0; i < fields.size(); i++) {
+      Field field = fields.get(i);
+      nested.put(field.name(), switch (field.type()) {
+        case BINARY -> group.getBinaryValue(i).getBytes();

Review Comment:
   @bchapuis based on the article and your concern about the overhead, I am happy if we don't use Optional.



-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#issuecomment-2143398066

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [2 New issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolutions=WONTFIX)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=860&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860)
   
   


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#issuecomment-2143378051

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [2 New issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolutions=WONTFIX)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=860&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860)
   
   


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sebr72 (via GitHub)" <gi...@apache.org>.
sebr72 commented on code in PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#discussion_r1624261828


##########
baremaps-core/src/main/java/org/apache/baremaps/storage/geoparquet/GeoParquetTypeConversion.java:
##########
@@ -69,9 +80,33 @@ public static List<Object> asRowValues(GeoParquetGroup group) {
         case DOUBLE -> values.add(group.getDoubleValue(i));
         case STRING -> values.add(group.getStringValue(i));
         case GEOMETRY -> values.add(group.getGeometryValue(i));
-        case GROUP -> values.add(null); // TODO: values.add(asDataRow(group.getGroupValue(i)));
+        case ENVELOPE -> values.add(group.getEnvelopeValue(i));
+        case GROUP -> values.add(asNested(group.getGroupValue(i)));
       }
     }
     return values;
   }
+
+  public static Map<String, Object> asNested(GeoParquetGroup group) {
+    Map<String, Object> nested = new HashMap<>();
+    Schema schema = group.getSchema();
+    List<Field> fields = schema.fields();
+    for (int i = 0; i < fields.size(); i++) {
+      Field field = fields.get(i);
+      nested.put(field.name(), switch (field.type()) {
+        case BINARY -> group.getBinaryValue(i).getBytes();

Review Comment:
   @bchapuis My instinct would say that when a field is optional we use Optional. This way we can make a difference with fields which are not optional with a null value (in the way our code explains 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.

To unsubscribe, e-mail: commits-unsubscribe@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "bchapuis (via GitHub)" <gi...@apache.org>.
bchapuis merged PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#issuecomment-2143572956

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [3 New issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolutions=WONTFIX)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=860&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860)
   
   


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#issuecomment-2143621318

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [3 New issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolutions=WONTFIX)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=860&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860)
   
   


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#issuecomment-2143545109

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [4 New issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_baremaps&pullRequest=860&resolutions=WONTFIX)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_baremaps&pullRequest=860&resolved=false&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/no-data-16px.png '') No data about Coverage  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_baremaps&pullRequest=860&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860)
   
   


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "Drabble (via GitHub)" <gi...@apache.org>.
Drabble commented on code in PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#discussion_r1623290051


##########
baremaps-geoparquet/src/main/java/org/apache/baremaps/geoparquet/data/GeoParquetGroupImpl.java:
##########
@@ -414,6 +427,22 @@ public List<Geometry> getGeometryValues(int fieldIndex) {
     return geometries;
   }
 
+  @Override
+  public Envelope getEnvelopeValue(int fieldIndex) {
+    return getEnvelopeValues(fieldIndex).get(0);
+  }
+
+  @Override
+  public List<Envelope> getEnvelopeValues(int fieldIndex) {
+    return getGroupValues(fieldIndex).stream().map(group -> {
+      var xMin = group.getDoubleValue(0);
+      var yMin = group.getDoubleValue(1);
+      var xMax = group.getDoubleValue(2);
+      var yMax = group.getDoubleValue(3);

Review Comment:
   With a Overture maps file this fails because the bbox uses float values.
   
   ```suggestion
         var xMin = group.getFloatValue(0);
         var yMin = group.getFloatValue(1);
         var xMax = group.getFloatValue(2);
         var yMax = group.getFloatValue(3);
   ```



##########
baremaps-core/src/main/java/org/apache/baremaps/storage/geoparquet/GeoParquetTypeConversion.java:
##########
@@ -69,9 +80,33 @@ public static List<Object> asRowValues(GeoParquetGroup group) {
         case DOUBLE -> values.add(group.getDoubleValue(i));
         case STRING -> values.add(group.getStringValue(i));
         case GEOMETRY -> values.add(group.getGeometryValue(i));
-        case GROUP -> values.add(null); // TODO: values.add(asDataRow(group.getGroupValue(i)));
+        case ENVELOPE -> values.add(group.getEnvelopeValue(i));
+        case GROUP -> values.add(asNested(group.getGroupValue(i)));
       }
     }
     return values;
   }
+
+  public static Map<String, Object> asNested(GeoParquetGroup group) {
+    Map<String, Object> nested = new HashMap<>();
+    Schema schema = group.getSchema();
+    List<Field> fields = schema.fields();
+    for (int i = 0; i < fields.size(); i++) {
+      Field field = fields.get(i);
+      nested.put(field.name(), switch (field.type()) {
+        case BINARY -> group.getBinaryValue(i).getBytes();

Review Comment:
   When an optional field or a group is null in the geoparquet file, this will raise an exception. The same thing happens in the `asRowValues` method. I tested it with real Overture data.



-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#issuecomment-2143772001

   ## [![Quality Gate Failed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-failed-20px.png 'Quality Gate Failed')](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) **Quality Gate failed**  
   Failed conditions  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/failed-16px.png '') [C Reliability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860) (required ≥ A)  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_baremaps&pullRequest=860)
   
   ##   
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/light_bulb-16px.png '') Catch issues before they fail your Quality Gate with our IDE extension ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/sonarlint-16px.png '') [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
   
   


-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "sebr72 (via GitHub)" <gi...@apache.org>.
sebr72 commented on code in PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#discussion_r1624261828


##########
baremaps-core/src/main/java/org/apache/baremaps/storage/geoparquet/GeoParquetTypeConversion.java:
##########
@@ -69,9 +80,33 @@ public static List<Object> asRowValues(GeoParquetGroup group) {
         case DOUBLE -> values.add(group.getDoubleValue(i));
         case STRING -> values.add(group.getStringValue(i));
         case GEOMETRY -> values.add(group.getGeometryValue(i));
-        case GROUP -> values.add(null); // TODO: values.add(asDataRow(group.getGroupValue(i)));
+        case ENVELOPE -> values.add(group.getEnvelopeValue(i));
+        case GROUP -> values.add(asNested(group.getGroupValue(i)));
       }
     }
     return values;
   }
+
+  public static Map<String, Object> asNested(GeoParquetGroup group) {
+    Map<String, Object> nested = new HashMap<>();
+    Schema schema = group.getSchema();
+    List<Field> fields = schema.fields();
+    for (int i = 0; i < fields.size(); i++) {
+      Field field = fields.get(i);
+      nested.put(field.name(), switch (field.type()) {
+        case BINARY -> group.getBinaryValue(i).getBytes();

Review Comment:
   @bchapuis My instinct would say that when a field is optional we use Optional. This way we can make a difference (in the way our code explains it) with fields which are not optional with a null value.



-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add support for nested types, geoparquet groups, and postgres jsonb in data table [incubator-baremaps]

Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #860:
URL: https://github.com/apache/incubator-baremaps/pull/860#discussion_r1623264626


##########
baremaps-geoparquet/src/main/java/org/apache/baremaps/geoparquet/data/GeoParquetGroup.java:
##########
@@ -230,6 +243,10 @@
 
   void setGeometryValues(String fieldName, List<Geometry> geometryValues);
 
+  void setEnvelopeValue(String fieldName, Envelope envelopeValue);
+
+  void setEnvelopeValues(String fieldName, List<Envelope> envelopeValues);

Review Comment:
   ## Useless parameter
   
   The parameter 'envelopeValues' is never used.
   
   [Show more details](https://github.com/apache/incubator-baremaps/security/code-scanning/1436)



##########
baremaps-geoparquet/src/main/java/org/apache/baremaps/geoparquet/data/GeoParquetGroup.java:
##########
@@ -230,6 +243,10 @@
 
   void setGeometryValues(String fieldName, List<Geometry> geometryValues);
 
+  void setEnvelopeValue(String fieldName, Envelope envelopeValue);

Review Comment:
   ## Useless parameter
   
   The parameter 'fieldName' is never used.
   
   [Show more details](https://github.com/apache/incubator-baremaps/security/code-scanning/1437)



##########
baremaps-geoparquet/src/main/java/org/apache/baremaps/geoparquet/data/GeoParquetGroup.java:
##########
@@ -230,6 +243,10 @@
 
   void setGeometryValues(String fieldName, List<Geometry> geometryValues);
 
+  void setEnvelopeValue(String fieldName, Envelope envelopeValue);

Review Comment:
   ## Useless parameter
   
   The parameter 'envelopeValue' is never used.
   
   [Show more details](https://github.com/apache/incubator-baremaps/security/code-scanning/1438)



##########
baremaps-geoparquet/src/main/java/org/apache/baremaps/geoparquet/data/GeoParquetGroup.java:
##########
@@ -230,6 +243,10 @@
 
   void setGeometryValues(String fieldName, List<Geometry> geometryValues);
 
+  void setEnvelopeValue(String fieldName, Envelope envelopeValue);
+
+  void setEnvelopeValues(String fieldName, List<Envelope> envelopeValues);

Review Comment:
   ## Useless parameter
   
   The parameter 'fieldName' is never used.
   
   [Show more details](https://github.com/apache/incubator-baremaps/security/code-scanning/1435)



-- 
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@baremaps.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org