address review comments

Signed-off-by: Kenrick Yap <[email protected]>
Bit-Quill · Dec 19, 2024 · e037672 · e037672
1 parent 26ab5b0
commit e037672
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 22 deletions.
diff --git a/docs/ppl-lang/planning/ppl-geoip-command.md b/docs/ppl-lang/planning/ppl-geoip-command.md
@@ -57,18 +57,3 @@ geoip function to add information about the geographical location of an IPv4 or
     - API data sources - if users have their own geoip provided will create ability for users to configure and call said endpoints
     - OpenSearch geospatial client - once geospatial client is published we can leverage client to utilize OpenSearch geo2ip functionality.
 - Additional datasource connection params will be provided through spark config options.
-
-### New syntax definition in ANTLR
-
-```ANTLR
-  
-// functions
-evalFunctionCall
-   : evalFunctionName LT_PRTHS functionArgs RT_PRTHS
-   | geoipFunction
-   ;  
-  
-geoipFunction
-   : GEOIP LT_PRTHS (datasource = functionArg COMMA)? ipAddress = functionArg (COMMA properties = stringLiteral)? RT_PRTHS
-   ;
-```
diff --git a/ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl-spark-integration/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java
@@ -457,20 +457,22 @@ public UnresolvedExpression visitGeoIpPropertyList(OpenSearchPPLParser.GeoIpProp
         if (ctx != null) {
             for (OpenSearchPPLParser.GeoIpPropertyContext property : ctx.geoIpProperty()) {
                 String propertyName = property.getText().toUpperCase();
-
-                try {
-                    GeoIpCatalystLogicalPlanTranslator.GeoIpProperty.valueOf(propertyName);
-                } catch (NullPointerException | IllegalArgumentException e) {
-                    throw new IllegalArgumentException("Invalid properties used.");
-                }
-
+                validateGeoIpProperty(propertyName);
                 properties.add(new Literal(propertyName, DataType.STRING));
             }
         }
 
         return new AttributeList(properties.build());
     }
 
+    private static void validateGeoIpProperty(String propertyName) {
+        try {
+            GeoIpCatalystLogicalPlanTranslator.GeoIpProperty.valueOf(propertyName);
+        } catch (NullPointerException | IllegalArgumentException e) {
+            throw new IllegalArgumentException("Invalid properties used.");
+        }
+    }
+
     private List<UnresolvedExpression> timestampFunctionArguments(
             OpenSearchPPLParser.TimestampFunctionCallContext ctx) {
         List<UnresolvedExpression> args =