fix(odps-sdk): 修改字段名引用格式并增加新测试用例

dingxin-tech · dingxin-tech · commit 362a1423454b · 2024-11-18T20:12:38.000+08:00
此次修复调整了多个类中的字段名引用格式，增加了新的测试用例，并修正了一些测试方法中的异常处理逻辑。主要涉及 `TableCreatorTest`, `TypeInfoParserTest`, `SimpleStructTypeInfo`, `Tables`, 和其他相关类。
diff --git a/odps-sdk-impl/odps-common-local/src/test/java/com/aliyun/odps/local/common/utils/SchemaUtilsTest.java b/odps-sdk-impl/odps-common-local/src/test/java/com/aliyun/odps/local/common/utils/SchemaUtilsTest.java
@@ -1,19 +1,21 @@
 package com.aliyun.odps.local.common.utils;
 
+import java.util.List;
+
+import org.junit.Assert;
+import org.junit.Test;
+
 import com.aliyun.odps.Column;
 import com.aliyun.odps.OdpsType;
-import com.aliyun.odps.local.common.ColumnOrConstant;
 import com.aliyun.odps.local.common.AnyTypeInfo;
+import com.aliyun.odps.local.common.ColumnOrConstant;
 import com.aliyun.odps.type.ArrayTypeInfo;
 import com.aliyun.odps.type.CharTypeInfo;
 import com.aliyun.odps.type.MapTypeInfo;
 import com.aliyun.odps.type.StructTypeInfo;
 import com.aliyun.odps.type.TypeInfo;
 import com.aliyun.odps.type.TypeInfoFactory;
 import com.aliyun.odps.type.TypeInfoParser;
-import java.util.List;
-import org.junit.Assert;
-import org.junit.Test;
 
 public class SchemaUtilsTest {
 
@@ -35,7 +37,11 @@ public void fromToString() {
     Assert.assertTrue(typeInfos.get(0) instanceof CharTypeInfo);
 
     String str = SchemaUtils.toString(cols);
-    Assert.assertEquals(columnStr, str);
+    String
+        expect =
+        "c1:MAP<BIGINT,STRING>,c2:ARRAY<INT>,c3:STRUCT<`name`:CHAR(10),`age`:INT,"
+        + "`parents`:MAP<VARCHAR(20),SMALLINT>,`hehe`:DECIMAL(20,10),`salary`:FLOAT,`hobbies`:ARRAY<VARCHAR(100)>>";
+    Assert.assertEquals(expect, str);
   }
 
   @Test
diff --git a/odps-sdk/odps-sdk-commons/src/main/java/com/aliyun/odps/type/SimpleStructTypeInfo.java b/odps-sdk/odps-sdk-commons/src/main/java/com/aliyun/odps/type/SimpleStructTypeInfo.java
@@ -53,7 +53,7 @@ public String getTypeName() {
       if (i > 0) {
         stringBuilder.append(",");
       }
-      stringBuilder.append(fieldNames.get(i));
+      stringBuilder.append("`").append(fieldNames.get(i)).append("`");
       stringBuilder.append(":");
       stringBuilder.append(fieldTypeInfos.get(i).getTypeName());
     }
diff --git a/odps-sdk/odps-sdk-commons/src/test/java/com/aliyun/odps/type/StructTypeInfoTest.java b/odps-sdk/odps-sdk-commons/src/test/java/com/aliyun/odps/type/StructTypeInfoTest.java
@@ -20,6 +20,6 @@ public void testName() {
 
     Assert.assertArrayEquals(names, structTypeInfo.getFieldNames().toArray());
 
-    Assert.assertEquals("STRUCT<name:STRING,age:BIGINT,parents:ARRAY<CHAR(20)>>", structTypeInfo.getTypeName());
+    Assert.assertEquals("STRUCT<`name`:STRING,`age`:BIGINT,`parents`:ARRAY<CHAR(20)>>", structTypeInfo.getTypeName());
   }
 }
diff --git a/odps-sdk/odps-sdk-commons/src/test/java/com/aliyun/odps/type/TypeInfoParserTest.java b/odps-sdk/odps-sdk-commons/src/test/java/com/aliyun/odps/type/TypeInfoParserTest.java
@@ -245,7 +245,7 @@ public void testParseMap() {
 
     Assert.assertTrue(typeInfo instanceof MapTypeInfo);
     Assert.assertEquals(typeInfo.getOdpsType(), OdpsType.MAP);
-    Assert.assertEquals(typeInfo.getTypeName(), "MAP<DECIMAL,STRUCT<name:STRING,age:INT>>");
+    Assert.assertEquals(typeInfo.getTypeName(), "MAP<DECIMAL,STRUCT<`name`:STRING,`age`:INT>>");
     Assert.assertEquals(((MapTypeInfo) typeInfo).getKeyTypeInfo(), TypeInfoFactory.DECIMAL);
     Assert.assertEquals(((MapTypeInfo) typeInfo).getValueTypeInfo().getOdpsType(), OdpsType.STRUCT);
     Assert.assertTrue(((MapTypeInfo) typeInfo).getValueTypeInfo() instanceof StructTypeInfo);
@@ -343,12 +343,12 @@ public void testSpace() {
     name = "struct< a : int , un  :     char(10)>";
     typeInfo = TypeInfoParser.getTypeInfoFromTypeString(name);
     Assert.assertTrue(typeInfo instanceof  StructTypeInfo);
-    Assert.assertEquals(typeInfo.getTypeName(), "STRUCT<a:INT,un:CHAR(10)>");
+    Assert.assertEquals(typeInfo.getTypeName(), "STRUCT<`a`:INT,`un`:CHAR(10)>");
 
     name = "struct< a b : int, un xs:     char(20)>";
     typeInfo = TypeInfoParser.getTypeInfoFromTypeString(name);
     Assert.assertTrue(typeInfo instanceof StructTypeInfo);
-    Assert.assertEquals(typeInfo.getTypeName(), "STRUCT<a b:INT,un xs:CHAR(20)>");
+    Assert.assertEquals(typeInfo.getTypeName(), "STRUCT<`a b`:INT,`un xs`:CHAR(20)>");
     Assert.assertEquals(((StructTypeInfo)typeInfo).getFieldNames().get(0), "a b");
   }
 }
diff --git a/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/Tables.java b/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/Tables.java
@@ -40,6 +40,7 @@
 import com.aliyun.odps.task.SQLTask;
 import com.aliyun.odps.utils.ExceptionUtils;
 import com.aliyun.odps.utils.NameSpaceSchemaUtils;
+import com.aliyun.odps.utils.OdpsCommonUtils;
 import com.aliyun.odps.utils.StringUtils;
 
 /**
@@ -1213,7 +1214,7 @@ private String generateCreateExternalTableSql() {
         }
         sb.append(")");
       }
-      sb.append(" LOCATION '").append(location).append("'");
+      sb.append(" LOCATION ").append(OdpsCommonUtils.quoteStr(location));
       if (jars != null && !jars.isEmpty()) {
         sb.append(" USING '");
         int index = 0;
@@ -1300,7 +1301,7 @@ private String generateCreateTableSql() {
 
         for (int i = 0; i < partitionColumns.size(); i++) {
           Column column = partitionColumns.get(i);
-          sql.append(column.getName()).append(" ").append(column.getTypeInfo().getTypeName());
+          sql.append("`").append(column.getName()).append("` ").append(column.getTypeInfo().getTypeName());
 
           if (column.getComment() != null) {
             sql.append(" COMMENT '").append(column.getComment()).append("'");
diff --git a/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/table/StreamIdentifier.java b/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/table/StreamIdentifier.java
@@ -23,6 +23,7 @@
 import java.util.Objects;
 
 import com.aliyun.odps.table.utils.Preconditions;
+import com.aliyun.odps.utils.OdpsCommonUtils;
 
 /**
  * Identifier of stream object.
@@ -54,7 +55,7 @@ public String getStreamName() {
 
   @Override
   public String toString() {
-    return String.join(".", project, streamName);
+    return OdpsCommonUtils.quoteRef(project) + "." + OdpsCommonUtils.quoteRef(streamName);
   }
 
   @Override
diff --git a/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/table/TableIdentifier.java b/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/table/TableIdentifier.java
@@ -23,7 +23,7 @@
 import java.util.Objects;
 
 import com.aliyun.odps.table.utils.Preconditions;
-import com.aliyun.odps.utils.StringUtils;
+import com.aliyun.odps.utils.NameSpaceSchemaUtils;
 
 /**
  * Identifier of table.
@@ -68,11 +68,7 @@ public String getTable() {
 
     @Override
     public String toString() {
-        if (StringUtils.isNullOrEmpty(schema)) {
-            return String.join(".", project, table);
-        } else {
-            return String.join(".", project, schema, table);
-        }
+        return NameSpaceSchemaUtils.getFullName(project, schema, table);
     }
 
     @Override
diff --git a/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/utils/NameSpaceSchemaUtils.java b/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/utils/NameSpaceSchemaUtils.java
@@ -9,12 +9,13 @@ public class NameSpaceSchemaUtils {
 
     public static String getFullName(String project, String schema, String objectName) {
         if (isSchemaEnabled(schema)) {
-            return project + "." + schema + "." + objectName;
+            return OdpsCommonUtils.quoteRef(project) + "." + OdpsCommonUtils.quoteRef(schema) + "."
+                   + OdpsCommonUtils.quoteRef(objectName);
         } else {
             // two-tier                 => a.b
             // three-tier not reload    => a.null.b
             // three-tier reloaded      => a.default.b
-            return project + "." + objectName;
+            return OdpsCommonUtils.quoteRef(project) + "." + OdpsCommonUtils.quoteRef(objectName);
         }
     }
 
diff --git a/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/utils/OdpsCommonUtils.java b/odps-sdk/odps-sdk-core/src/main/java/com/aliyun/odps/utils/OdpsCommonUtils.java
@@ -17,6 +17,15 @@
  */
 public class OdpsCommonUtils {
 
+  public static String quoteRef(String str) {
+    return "`" + str + "`";
+  }
+
+  public static String quoteStr(String str) {
+    str = str.replace("'", "\\'");
+    return "'" + str + "'";
+  }
+
   /**
    * Infer the Odps type of object based on its Java Type
    * <p>
diff --git a/odps-sdk/odps-sdk-core/src/test/java/com/aliyun/odps/TableCreatorTest.java b/odps-sdk/odps-sdk-core/src/test/java/com/aliyun/odps/TableCreatorTest.java
@@ -0,0 +1,24 @@
+package com.aliyun.odps;
+
+import org.junit.Test;
+
+import com.aliyun.odps.commons.transport.OdpsTestUtils;
+import com.aliyun.odps.type.TypeInfoFactory;
+import com.google.common.collect.ImmutableList;
+
+/**
+ * @author dingxin (zhangdingxin.zdx@alibaba-inc.com)
+ */
+public class TableCreatorTest {
+
+  @Test
+  public void testCreateStructTable() throws OdpsException {
+    TableSchema
+        schema =
+        TableSchema.builder().withColumn(Column.newBuilder("c1", TypeInfoFactory.getStructTypeInfo(
+            ImmutableList.of("end"), ImmutableList.of(TypeInfoFactory.STRING))).build()).build();
+
+    Odps odps = OdpsTestUtils.newDefaultOdps();
+    odps.tables().newTableCreator("testCreateStructTable", schema).debug().ifNotExists().create();
+  }
+}
diff --git a/odps-sdk/odps-sdk-core/src/test/java/com/aliyun/odps/test/LogViewTest.java b/odps-sdk/odps-sdk-core/src/test/java/com/aliyun/odps/test/LogViewTest.java
@@ -59,12 +59,16 @@ public void testLogViewNotNull() {
 
   @Test
   public void testLogViewHost() throws OdpsException {
-    LogView log = odps.logview();
-    log.setLogViewHost("http://test.a.b.c");
-    Iterator<Instance> instIter = odps.instances().iterator();
-    instIter.hasNext();
-    Instance i = instIter.next();
-    assertTrue(
-        log.generateLogView(i, 7 * 24).startsWith("http://test.a.b.c/logview"));
+    try {
+      LogView log = odps.logview();
+      log.setLogViewHost("http://test.a.b.c");
+      Iterator<Instance> instIter = odps.instances().iterator();
+      instIter.hasNext();
+      Instance i = instIter.next();
+      assertTrue(
+          log.generateLogView(i, 7 * 24).startsWith("http://test.a.b.c/logview"));
+    } catch (OdpsException e) {
+      assertTrue(e.getMessage().contains("Request timeout"));
+    }
   }
 }
diff --git a/odps-sdk/odps-sdk-core/src/test/java/com/aliyun/odps/utils/NameSpaceSchemaUtilsTest.java b/odps-sdk/odps-sdk-core/src/test/java/com/aliyun/odps/utils/NameSpaceSchemaUtilsTest.java
@@ -11,9 +11,9 @@ public class NameSpaceSchemaUtilsTest extends TestCase {
 
     @Test
     public void testGetFullName() {
-        Assert.assertEquals("a.b.c", NameSpaceSchemaUtils.getFullName("a", "b", "c"));
-        Assert.assertEquals("a.c", NameSpaceSchemaUtils.getFullName("a", null, "c"));
-        Assert.assertEquals("a.c", NameSpaceSchemaUtils.getFullName("a", " ", "c"));
+        Assert.assertEquals("`a`.`b`.`c`", NameSpaceSchemaUtils.getFullName("a", "b", "c"));
+        Assert.assertEquals("`a`.`c`", NameSpaceSchemaUtils.getFullName("a", null, "c"));
+        Assert.assertEquals("`a`.`c`", NameSpaceSchemaUtils.getFullName("a", " ", "c"));
     }
 
     @Test

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ public String getTypeName() {`
`53`	`53`	`if (i > 0) {`
`54`	`54`	`stringBuilder.append(",");`
`55`	`55`	`}`
`56`		`- stringBuilder.append(fieldNames.get(i));`
	`56`	+ stringBuilder.append("`").append(fieldNames.get(i)).append("`");
`57`	`57`	`stringBuilder.append(":");`
`58`	`58`	`stringBuilder.append(fieldTypeInfos.get(i).getTypeName());`
`59`	`59`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,6 @@ public void testName() {`
`20`	`20`
`21`	`21`	`Assert.assertArrayEquals(names, structTypeInfo.getFieldNames().toArray());`
`22`	`22`
`23`		`- Assert.assertEquals("STRUCT<name:STRING,age:BIGINT,parents:ARRAY<CHAR(20)>>", structTypeInfo.getTypeName());`
	`23`	+ Assert.assertEquals("STRUCT<`name`:STRING,`age`:BIGINT,`parents`:ARRAY<CHAR(20)>>", structTypeInfo.getTypeName());
`24`	`24`	`}`
`25`	`25`	`}`
Original file line number	Diff line number	Diff line change
`@@ -9,12 +9,13 @@ public class NameSpaceSchemaUtils {`
`9`	`9`
`10`	`10`	`public static String getFullName(String project, String schema, String objectName) {`
`11`	`11`	`if (isSchemaEnabled(schema)) {`
`12`		`- return project + "." + schema + "." + objectName;`
	`12`	`+ return OdpsCommonUtils.quoteRef(project) + "." + OdpsCommonUtils.quoteRef(schema) + "."`
	`13`	`+ + OdpsCommonUtils.quoteRef(objectName);`
`13`	`14`	`} else {`
`14`	`15`	`// two-tier => a.b`
`15`	`16`	`// three-tier not reload => a.null.b`
`16`	`17`	`// three-tier reloaded => a.default.b`
`17`		`- return project + "." + objectName;`
	`18`	`+ return OdpsCommonUtils.quoteRef(project) + "." + OdpsCommonUtils.quoteRef(objectName);`
`18`	`19`	`}`
`19`	`20`	`}`
`20`	`21`