Tiny refinement

facebookincubator · Nov 15, 2023 · ebab33c · ebab33c
1 parent 03efc3a
commit ebab33c
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 18 deletions.
diff --git a/velox/functions/sparksql/Register.cpp b/velox/functions/sparksql/Register.cpp
@@ -108,8 +108,6 @@ void registerFunctions(const std::string& prefix) {
   // Register size functions
   registerSize(prefix + "size");
 
-  //   registerFunction<JsonExtractScalarFunction, Varchar, Varchar, Varchar>(
-  //       {prefix + "get_json_object"});
   registerFunction<SIMDGetJsonObjectFunction, Varchar, Varchar, Varchar>(
       {prefix + "get_json_object"});
 

diff --git a/velox/functions/sparksql/SIMDJsonFunctions.h b/velox/functions/sparksql/SIMDJsonFunctions.h
@@ -134,24 +134,24 @@ struct SIMDGetJsonObjectFunction {
     }
   }
 
-  // This is simple validation by checking whether the obtained result is
+  // This is a simple validation by checking whether the obtained result is
   // followed by expected char. It is useful in ondemand kind of parsing which
-  // can ignore the validation of character following closing '"'. This functon
-  // is a simple checking. For many cases, even though it returns true, the raw
-  // json string can still be illegal possibly.
-  bool isValidEnding(const char* current_position, int check_index) {
-    char ending_char = current_position[check_index];
-    if (ending_char == ',') {
+  // ignores the json format validation for characters following the current
+  // parsing position. For many cases, even though this function returns true,
+  // the raw json string can still be illegal possibly.
+  bool isValidEnding(const char* currentPos) {
+    char endingChar = *currentPos;
+    if (endingChar == ',') {
       return true;
-    } else if (ending_char == '}') {
+    } else if (endingChar == '}') {
       return true;
-    } else if (ending_char == ']') {
+    } else if (endingChar == ']') {
       return true;
     } else if (
-        ending_char == ' ' || ending_char == '\r' || ending_char == '\n' ||
-        ending_char == '\t') {
+        endingChar == ' ' || endingChar == '\r' || endingChar == '\n' ||
+        endingChar == '\t') {
       // space, '\r', '\n' or '\t' can precede valid ending char.
-      return isValidEnding(current_position, check_index + 1);
+      return isValidEnding(currentPos++);
     } else {
       return false;
     }
@@ -192,9 +192,9 @@ struct SIMDGetJsonObjectFunction {
       return false;
     }
 
-    const char* current_location;
-    ctx.jsonDoc.current_location().get(current_location);
-    if (!isValidEnding(current_location, 0)) {
+    const char* currentPos;
+    ctx.jsonDoc.current_location().get(currentPos);
+    if (!isValidEnding(currentPos)) {
       return false;
     }
 

diff --git a/velox/functions/sparksql/tests/JsonTest.cpp b/velox/functions/sparksql/tests/JsonTest.cpp
@@ -102,4 +102,4 @@ TEST_F(JsonTest, getJsonObject) {
 }
 
 } // namespace
-} // namespace facebook::velox::functions::sparksql::test
+} // namespace facebook::velox::functions::sparksql::test