You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@carbondata.apache.org by GitBox <gi...@apache.org> on 2021/04/23 07:41:39 UTC

[GitHub] [carbondata] Indhumathi27 commented on a change in pull request #4118: [CARBONDATA-4167][CARBONDATA-4168] Fix case sensitive issues and input validation for Geo values.

Indhumathi27 commented on a change in pull request #4118:
URL: https://github.com/apache/carbondata/pull/4118#discussion_r618991229



##########
File path: geo/src/main/java/org/apache/carbondata/geo/GeoHashIndex.java
##########
@@ -136,17 +139,21 @@ public void init(String indexName, Map<String, String> properties) throws Except
     }
     String GRID_SIZE = commonKey + "gridsize";
     String gridSize = properties.get(GRID_SIZE);
-    if (StringUtils.isEmpty(gridSize)) {
+    if (StringUtils.isEmpty(gridSize) ||
+        !Pattern.compile(POSITIVE_INTEGER_REGEX).matcher(gridSize).find()) {
       throw new MalformedCarbonCommandException(
-              String.format("%s property is invalid. %s property must be specified.",
-                      CarbonCommonConstants.SPATIAL_INDEX, GRID_SIZE));
+          String.format("%s property is invalid. %s property must be specified, "
+                  + "and the value must be positive integer.",
+              CarbonCommonConstants.SPATIAL_INDEX, GRID_SIZE));
     }
     String CONVERSION_RATIO = commonKey + "conversionratio";
     String conversionRatio = properties.get(CONVERSION_RATIO);
-    if (StringUtils.isEmpty(conversionRatio)) {
+    if (StringUtils.isEmpty(conversionRatio) ||

Review comment:
       Can move this validation to method and reuse

##########
File path: geo/src/main/java/org/apache/carbondata/geo/scan/expression/PolygonRangeListExpression.java
##########
@@ -49,14 +49,18 @@ public PolygonRangeListExpression(String polygonRangeList, String opType, String
   public void processExpression() {
     // 1. parse the range list string
     List<String> rangeLists = new ArrayList<>();
-    Pattern pattern = Pattern.compile(GeoConstants.RANGELIST_REG_EXPRESSION);
+    Pattern pattern =
+        Pattern.compile(GeoConstants.RANGELIST_REG_EXPRESSION, Pattern.CASE_INSENSITIVE);
     Matcher matcher = pattern.matcher(polygon);
     while (matcher.find()) {
       String matchedStr = matcher.group();
       rangeLists.add(matchedStr);
     }
     // 2. process the range lists
     if (rangeLists.size() > 0) {
+      if (!opType.equals("OR") && !opType.equals("AND")) {

Review comment:
       We can use GeoOperationType.getEnum(opType) and validate.

##########
File path: geo/src/main/java/org/apache/carbondata/geo/GeoHashUtils.java
##########
@@ -83,6 +85,17 @@ public static long lonLat2GeoID(long longitude, long latitude, double oriLatitud
     return colRow2GeoID(ij[0], ij[1]);
   }
 
+  public static void validateUDFInputValue(Object input, String inputName, String datatype)
+      throws MalformedCarbonCommandException {
+    if (inputName.equalsIgnoreCase("gridSize") && (input == null
+        || Integer.parseInt(input.toString()) <= 0)) {

Review comment:
       can we use Pattern.compile(POSITIVE_INTEGER_REGEX) to validate here




-- 
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:
users@infra.apache.org