yuqi1129 commented on code in PR #10637: URL: https://github.com/apache/gravitino/pull/10637#discussion_r3201981399
########## docs/lance-rest-integration.md: ########## @@ -158,6 +158,7 @@ pip install lance-ray :::info - Ray will be automatically installed if not already present - lance-ray is currently tested with Ray versions 2.41.0 to 2.50.0 Review Comment: The version of Ray used by lance-ray may have changed. Can you confirm it? ########## docs/lance-rest-service.md: ########## @@ -273,17 +274,26 @@ curl -X POST http://localhost:9101/lance/v1/table/lance_catalog%24schema%24table -d '{ "id": ["lance_catalog", "schema", "table01"], "location": "/tmp/lance_catalog/schema/table01", - "mode": "CREATE" + "mode": "create" Review Comment: So the value must be `create`, and `CREATE` will run into errors? ########## lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableAlterHandler.java: ########## @@ -115,21 +115,20 @@ public AlterTableAlterColumnsResponse handle( } private TableChange[] buildAlterColumnChanges(AlterTableAlterColumnsRequest request) { - List<ColumnAlteration> columns = request.getAlterations(); + List<AlterColumnsEntry> columns = request.getAlterations(); List<TableChange> changes = new ArrayList<>(); - for (ColumnAlteration column : columns) { + for (AlterColumnsEntry column : columns) { // Column name will not be null according to LanceDB spec. - String columnName = column.getColumn(); + String columnName = column.getPath(); String newName = column.getRename(); if (StringUtils.isNotBlank(newName)) { changes.add(TableChange.renameColumn(new String[] {columnName}, newName)); } - // The format of ColumnAlteration#castTo is unclear, so we will skip it now - // for more, please refer to: https://shorturl.at/bYI0Z (short url for - // github.com/lance-format/lance-namespace) - if (StringUtils.isNotBlank(column.getCastTo())) { + if (column.getDataType() != null + || column.getNullable() != null + || column.getVirtualColumn() != null) { Review Comment: So an alter operation will contain those three informations? ########## lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java: ########## @@ -366,4 +377,37 @@ private JsonArrowSchema toJsonArrowSchema(Column[] columns) { return JsonArrowSchemaConverter.convertToJsonArrowSchema( new org.apache.arrow.vector.types.pojo.Schema(fields)); } + + private static String normalizeCreateMode(String mode, String tableId) { + if (mode == null) { + return "CREATE"; + } + String normalized = CommonUtil.normalizeToken(mode); + if ("CREATE".equals(normalized)) { + return "CREATE"; + } + if ("EXISTOK".equals(normalized)) { + return "EXIST_OK"; + } + if ("OVERWRITE".equals(normalized)) { + return "OVERWRITE"; + } Review Comment: That code looks tedious, can you use a set or enum to replace them? ########## lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceNameSpaceOperations.java: ########## @@ -414,6 +408,56 @@ private <T> T[] buildChanges( return Stream.concat(setPropertiesStream, removePropertiesStream).toArray(arrayCreator); } + private static CreateMode parseCreateMode(String instance, String mode) { + if (mode == null) { + return CreateMode.CREATE; + } + String normalized = CommonUtil.normalizeToken(mode); Review Comment: `normalizeToken` will remove all sepcical character, why do we need it? Is true that it's a valid mode if the value looks like `#CREA!TE$`? ########## docs/lance-rest-integration.md: ########## @@ -22,7 +22,7 @@ The following table outlines the tested compatibility between Gravitino versions | Gravitino Version (Lance REST) | Supported lance-spark Versions | Supported lance-ray Versions | |--------------------------------|--------------------------------|------------------------------| -| 1.1.1 | 0.0.10 – 0.0.15 | 0.0.6 – 0.0.8 | +| 1.3.0 | 0.1.0 – 0.2.0 | 0.0.6 – 0.2.0 | Review Comment: Format the table. Besides, can you try to change it to the format | Gravitino Version (Lance REST) | Supported lance-spark Versions | Supported lance-ray Versions | |--------------------------------|--------------------------------|------------------------------| | 1.1.1 - 1.2.1 | 0.0.10 – 0.0.15 | 0.0.6 – 0.0.8 | | 1.3.0 | 0.1.0 – 0.2.0 | 0.0.6 – 0.2.0 | Another point is that -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
