[GitHub] [gobblin] jack-moseley commented on a change in pull request #3246: [GOBBLIN-1412] Escape single backslash in avro ORC schema conversion

[GitBox <gi...@apache.org>]

jack-moseley commented on a change in pull request #3246:
URL: https://github.com/apache/gobblin/pull/3246#discussion_r595558876



##########
File path: gobblin-utility/src/main/java/org/apache/gobblin/util/AvroUtils.java
##########
@@ -888,13 +888,13 @@ public static Path serializeAsPath(GenericRecord record, boolean includeFieldNam
   }
 
   /**
-   * Escaping ";" and "'" character in the schema string when it is being used in DDL.
+   * Escaping "\", """, ";" and "'" character in the schema string when it is being used in DDL.
    * These characters are not allowed to show as part of column name but could possibly appear in documentation field.
    * Therefore the escaping behavior won't cause correctness issues.
    */
   public static String sanitizeSchemaString(String schemaString) {
-    return schemaString.replace("\\\"", "\\\\\\\"").replace(";",  "\\;")
-        .replace("'", "\\'");
+    return schemaString.replace("\\\\", "\\\\\\\\").replace("\\\"", "\\\\\\\"")

Review comment:
       This is a bit confusing, but it's not really a single backslash when in the schema file, it's a double backslash.
   
   Basically if you are looking at the schema file directly (without java escaping), the original is replacing `\"` with "\\\"`, and the part I added is replacing `\\` with `\\\\`. So no, the quote case is not covered by this.




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