From e9bb00481a4b37bbd70acbf84118d31e88a4b3ce Mon Sep 17 00:00:00 2001 From: Jishnu J Date: Thu, 19 Dec 2024 17:21:13 +0530 Subject: [PATCH 1/5] Added export options validator --- .../com/scalar/db/common/error/CoreError.java | 7 + .../ExportOptionsValidationException.java | 14 ++ .../validation/ExportOptionsValidator.java | 107 +++++++++++++ .../ExportOptionsValidatorTest.java | 149 ++++++++++++++++++ 4 files changed, 277 insertions(+) create mode 100644 data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidationException.java create mode 100644 data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java create mode 100644 data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidatorTest.java diff --git a/core/src/main/java/com/scalar/db/common/error/CoreError.java b/core/src/main/java/com/scalar/db/common/error/CoreError.java index b02b3c45a6..b4d43a73c0 100644 --- a/core/src/main/java/com/scalar/db/common/error/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/error/CoreError.java @@ -690,6 +690,13 @@ public enum CoreError implements ScalarDbError { ""), DATA_LOADER_ERROR_METHOD_NULL_ARGUMENT( Category.USER_ERROR, "0151", "Method null argument not allowed", "", ""), + DATA_LOADER_CLUSTERING_KEY_NOT_FOUND( + Category.USER_ERROR, "0152", "The provided clustering key %s was not found", "", "" + ), + DATA_LOADER_INVALID_PROJECTION( + Category.USER_ERROR, "0153", "The column '%s' was not found", "", "" + ), + // // Errors for the concurrency error category diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidationException.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidationException.java new file mode 100644 index 0000000000..42e342dec7 --- /dev/null +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidationException.java @@ -0,0 +1,14 @@ +package com.scalar.db.dataloader.core.dataexport.validation; + +/** A custom exception for export options validation errors */ +public class ExportOptionsValidationException extends Exception { + + /** + * Class constructor + * + * @param message error message + */ + public ExportOptionsValidationException(String message) { + super(message); + } +} diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java new file mode 100644 index 0000000000..3aefe1ce31 --- /dev/null +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java @@ -0,0 +1,107 @@ +package com.scalar.db.dataloader.core.dataexport.validation; + +import com.scalar.db.api.Scan; +import com.scalar.db.api.TableMetadata; +import com.scalar.db.common.error.CoreError; +import com.scalar.db.dataloader.core.ScanRange; +import com.scalar.db.dataloader.core.dataexport.ExportOptions; +import com.scalar.db.io.Key; +import java.util.LinkedHashSet; +import java.util.List; + +/** Validation for Export options */ +public class ExportOptionsValidator { + + /** + * Validate export request + * + * @param exportOptions Export options + * @param tableMetadata Metadata for a single ScalarDB table + * @throws ExportOptionsValidationException when the options are invalid + */ + public static void validate(ExportOptions exportOptions, TableMetadata tableMetadata) + throws ExportOptionsValidationException { + LinkedHashSet clusteringKeyNames = tableMetadata.getClusteringKeyNames(); + ScanRange scanRange = exportOptions.getScanRange(); + + // validate projections + validateProjectionColumns(tableMetadata.getColumnNames(), exportOptions.getProjectionColumns()); + + // validate sorts + if (!exportOptions.getSortOrders().isEmpty()) { + for (Scan.Ordering sort : exportOptions.getSortOrders()) { + validateClusteringKey(clusteringKeyNames, sort.getColumnName()); + } + } + + // Validate scan start key + if (scanRange.getScanStartKey() != null) { + validateClusteringKey(clusteringKeyNames, scanRange.getScanStartKey()); + } + + // Validate scan end key + if (scanRange.getScanEndKey() != null) { + validateClusteringKey(clusteringKeyNames, scanRange.getScanEndKey()); + } + } + + /** + * Check if the provided clustering key is available in the ScalarDB table + * + * @param clusteringKeyNames List of clustering key names available in a + * @param key To be validated ScalarDB key + * @throws ExportOptionsValidationException if the key could not be found or is not a clustering + * key + */ + private static void validateClusteringKey(LinkedHashSet clusteringKeyNames, Key key) + throws ExportOptionsValidationException { + if (clusteringKeyNames == null) { + return; + } + String columnName = key.getColumnName(0); + validateClusteringKey(clusteringKeyNames, columnName); + } + + /** + * Check if the provided clustering key is available in the ScalarDB table + * + * @param clusteringKeyNames List of clustering key names available in a + * @param columnName Column name of the to be validated clustering key + * @throws ExportOptionsValidationException if the key could not be found or is not a clustering + * key + */ + private static void validateClusteringKey( + LinkedHashSet clusteringKeyNames, String columnName) + throws ExportOptionsValidationException { + if (clusteringKeyNames == null) { + return; + } + + if (!clusteringKeyNames.contains(columnName)) { + throw new ExportOptionsValidationException( + CoreError.DATA_LOADER_CLUSTERING_KEY_NOT_FOUND.buildMessage(columnName)); + } + } + + /** + * Check if the provided projection column names are available in the ScalarDB table + * + * @param columnNames List of ScalarDB table column names + * @param columns List of to be validated column names + * @throws ExportOptionsValidationException if the column name was not found in the table + */ + private static void validateProjectionColumns( + LinkedHashSet columnNames, List columns) + throws ExportOptionsValidationException { + if (columns == null || columns.isEmpty()) { + return; + } + for (String column : columns) { + // O(n) but list is always going to be very small, so it's ok + if (!columnNames.contains(column)) { + throw new ExportOptionsValidationException( + CoreError.DATA_LOADER_INVALID_PROJECTION.buildMessage(column)); + } + } + } +} diff --git a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidatorTest.java b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidatorTest.java new file mode 100644 index 0000000000..b34daaba2d --- /dev/null +++ b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidatorTest.java @@ -0,0 +1,149 @@ +package com.scalar.db.dataloader.core.dataexport.validation; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import com.scalar.db.api.Scan; +import com.scalar.db.api.TableMetadata; +import com.scalar.db.common.error.CoreError; +import com.scalar.db.dataloader.core.FileFormat; +import com.scalar.db.dataloader.core.ScanRange; +import com.scalar.db.dataloader.core.dataexport.ExportOptions; +import com.scalar.db.io.DataType; +import com.scalar.db.io.IntColumn; +import com.scalar.db.io.Key; +import com.scalar.db.io.TextColumn; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class ExportOptionsValidatorTest { + + TableMetadata mockMetadata; + List projectedColumns; + + @BeforeEach + void setup() { + mockMetadata = + TableMetadata.newBuilder() + .addColumn("id", DataType.INT) + .addColumn("name", DataType.TEXT) + .addColumn("email", DataType.TEXT) + .addColumn("department", DataType.TEXT) + .addPartitionKey("id") + .addClusteringKey("department") + .build(); + projectedColumns = new ArrayList<>(); + projectedColumns.add("id"); + projectedColumns.add("name"); + projectedColumns.add("email"); + projectedColumns.add("department"); + } + + @Test + void validate_withValidExportOptions_ShouldNotThrowException() + throws ExportOptionsValidationException { + ExportOptions exportOptions = + ExportOptions.builder( + "test", + "sample", + Key.newBuilder().add(IntColumn.of("id", 1)).build(), + FileFormat.JSON) + .sortOrders(Collections.emptyList()) + .scanRange(new ScanRange(null, null, false, false)) + .projectionColumns(projectedColumns) + .build(); + ExportOptionsValidator.validate(exportOptions, mockMetadata); + } + + @Test + void validate_withInValidColumnInProjectionColumnList_ShouldThrowException() { + ExportOptions exportOptions = + ExportOptions.builder( + "test", + "sample", + Key.newBuilder().add(IntColumn.of("id", 1)).build(), + FileFormat.JSON) + .sortOrders(Collections.emptyList()) + .scanRange(new ScanRange(null, null, false, false)) + .projectionColumns(Collections.singletonList("sample")) + .build(); + assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, mockMetadata)) + .isInstanceOf(ExportOptionsValidationException.class) + .hasMessage(CoreError.DATA_LOADER_INVALID_PROJECTION.buildMessage("sample")); + } + + @Test + void validate_withInValidSortOrderWithMultipleClusteringKeys_ShouldThrowException() { + mockMetadata = + TableMetadata.newBuilder() + .addColumn("id", DataType.INT) + .addColumn("name", DataType.TEXT) + .addColumn("email", DataType.TEXT) + .addColumn("department", DataType.TEXT) + .addColumn("building", DataType.TEXT) + .addPartitionKey("id") + .addClusteringKey("department") + .addClusteringKey("building") + .build(); + ExportOptions exportOptions = + ExportOptions.builder( + "test", + "sample", + Key.newBuilder().add(IntColumn.of("id", 1)).build(), + FileFormat.JSON) + .sortOrders( + Collections.singletonList(new Scan.Ordering("name", Scan.Ordering.Order.ASC))) + .scanRange(new ScanRange(null, null, false, false)) + .projectionColumns(Collections.singletonList("id")) + .build(); + assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, mockMetadata)) + .isInstanceOf(ExportOptionsValidationException.class) + .hasMessage(CoreError.DATA_LOADER_CLUSTERING_KEY_NOT_FOUND.buildMessage("name")); + } + + @Test + void validate_withInValidKeyInSortRange_ShouldThrowException() { + ExportOptions exportOptions = + ExportOptions.builder( + "test", + "sample", + Key.newBuilder().add(IntColumn.of("id", 1)).build(), + FileFormat.JSON) + .sortOrders(Collections.emptyList()) + .scanRange( + new ScanRange( + Key.newBuilder().add(IntColumn.of("id", 1)).build(), + Key.newBuilder().add(IntColumn.of("id", 100)).build(), + false, + false)) + .projectionColumns(Collections.singletonList("id")) + .build(); + assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, mockMetadata)) + .isInstanceOf(ExportOptionsValidationException.class) + .hasMessage(CoreError.DATA_LOADER_CLUSTERING_KEY_NOT_FOUND.buildMessage("id")); + } + + @Test + void validate_withInValidEndKeyInSortRange_ShouldThrowException() { + ExportOptions exportOptions = + ExportOptions.builder( + "test", + "sample", + Key.newBuilder().add(IntColumn.of("id", 1)).build(), + FileFormat.JSON) + .sortOrders(Collections.emptyList()) + .scanRange( + new ScanRange( + Key.newBuilder().add(TextColumn.of("department", "sample")).build(), + Key.newBuilder().add(IntColumn.of("name", 100)).build(), + false, + false)) + .projectionColumns(projectedColumns) + .build(); + assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, mockMetadata)) + .isInstanceOf(ExportOptionsValidationException.class) + .hasMessage(CoreError.DATA_LOADER_CLUSTERING_KEY_NOT_FOUND.buildMessage("name")); + } +} From d6aaf85c89cd88e4345dc57287fc997de51fd227 Mon Sep 17 00:00:00 2001 From: Jishnu J Date: Thu, 19 Dec 2024 17:52:04 +0530 Subject: [PATCH 2/5] Applied spotless on CoreError --- .../main/java/com/scalar/db/common/error/CoreError.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/com/scalar/db/common/error/CoreError.java b/core/src/main/java/com/scalar/db/common/error/CoreError.java index b4d43a73c0..3a41f7b20b 100644 --- a/core/src/main/java/com/scalar/db/common/error/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/error/CoreError.java @@ -691,12 +691,9 @@ public enum CoreError implements ScalarDbError { DATA_LOADER_ERROR_METHOD_NULL_ARGUMENT( Category.USER_ERROR, "0151", "Method null argument not allowed", "", ""), DATA_LOADER_CLUSTERING_KEY_NOT_FOUND( - Category.USER_ERROR, "0152", "The provided clustering key %s was not found", "", "" - ), + Category.USER_ERROR, "0152", "The provided clustering key %s was not found", "", ""), DATA_LOADER_INVALID_PROJECTION( - Category.USER_ERROR, "0153", "The column '%s' was not found", "", "" - ), - + Category.USER_ERROR, "0153", "The column '%s' was not found", "", ""), // // Errors for the concurrency error category From 4439dea7097f592de5f419ef52798c623f49a047 Mon Sep 17 00:00:00 2001 From: Peckstadt Yves Date: Fri, 20 Dec 2024 08:55:18 +0900 Subject: [PATCH 3/5] Make constructor private and improve javadocs --- .../validation/ExportOptionsValidator.java | 48 ++++++++++--------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java index 3aefe1ce31..143fabfd33 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java @@ -8,26 +8,32 @@ import com.scalar.db.io.Key; import java.util.LinkedHashSet; import java.util.List; +import lombok.AccessLevel; +import lombok.NoArgsConstructor; -/** Validation for Export options */ +/** + * A validator for ensuring that export options are consistent with the ScalarDB table metadata and + * follow the defined constraints. + */ +@NoArgsConstructor(access = AccessLevel.PRIVATE) public class ExportOptionsValidator { /** - * Validate export request + * Validates the export request. * - * @param exportOptions Export options - * @param tableMetadata Metadata for a single ScalarDB table - * @throws ExportOptionsValidationException when the options are invalid + * @param exportOptions The export options provided by the user. + * @param tableMetadata The metadata of the ScalarDB table to validate against. + * @throws ExportOptionsValidationException If the export options are invalid. */ public static void validate(ExportOptions exportOptions, TableMetadata tableMetadata) throws ExportOptionsValidationException { LinkedHashSet clusteringKeyNames = tableMetadata.getClusteringKeyNames(); ScanRange scanRange = exportOptions.getScanRange(); - // validate projections + // Validate projection columns validateProjectionColumns(tableMetadata.getColumnNames(), exportOptions.getProjectionColumns()); - // validate sorts + // Validate sort orders if (!exportOptions.getSortOrders().isEmpty()) { for (Scan.Ordering sort : exportOptions.getSortOrders()) { validateClusteringKey(clusteringKeyNames, sort.getColumnName()); @@ -46,12 +52,11 @@ public static void validate(ExportOptions exportOptions, TableMetadata tableMeta } /** - * Check if the provided clustering key is available in the ScalarDB table + * Checks if the provided clustering key is valid for the ScalarDB table. * - * @param clusteringKeyNames List of clustering key names available in a - * @param key To be validated ScalarDB key - * @throws ExportOptionsValidationException if the key could not be found or is not a clustering - * key + * @param clusteringKeyNames The set of valid clustering key names for the table. + * @param key The ScalarDB key to validate. + * @throws ExportOptionsValidationException If the key is invalid or not a clustering key. */ private static void validateClusteringKey(LinkedHashSet clusteringKeyNames, Key key) throws ExportOptionsValidationException { @@ -63,12 +68,11 @@ private static void validateClusteringKey(LinkedHashSet clusteringKeyNam } /** - * Check if the provided clustering key is available in the ScalarDB table + * Checks if the provided clustering key column name is valid for the ScalarDB table. * - * @param clusteringKeyNames List of clustering key names available in a - * @param columnName Column name of the to be validated clustering key - * @throws ExportOptionsValidationException if the key could not be found or is not a clustering - * key + * @param clusteringKeyNames The set of valid clustering key names for the table. + * @param columnName The column name of the clustering key to validate. + * @throws ExportOptionsValidationException If the column name is not a valid clustering key. */ private static void validateClusteringKey( LinkedHashSet clusteringKeyNames, String columnName) @@ -84,11 +88,11 @@ private static void validateClusteringKey( } /** - * Check if the provided projection column names are available in the ScalarDB table + * Checks if the provided projection column names are valid for the ScalarDB table. * - * @param columnNames List of ScalarDB table column names - * @param columns List of to be validated column names - * @throws ExportOptionsValidationException if the column name was not found in the table + * @param columnNames The set of valid column names for the table. + * @param columns The list of projection column names to validate. + * @throws ExportOptionsValidationException If any of the column names are invalid. */ private static void validateProjectionColumns( LinkedHashSet columnNames, List columns) @@ -97,7 +101,7 @@ private static void validateProjectionColumns( return; } for (String column : columns) { - // O(n) but list is always going to be very small, so it's ok + // O(n) lookup, but acceptable given the typically small list size if (!columnNames.contains(column)) { throw new ExportOptionsValidationException( CoreError.DATA_LOADER_INVALID_PROJECTION.buildMessage(column)); From b3279baa7b08e7753e97a6028b3ece6fa0282dad Mon Sep 17 00:00:00 2001 From: Peckstadt Yves Date: Mon, 23 Dec 2024 17:22:49 +0900 Subject: [PATCH 4/5] Fix the validation for partition and clustering keys --- .../com/scalar/db/common/error/CoreError.java | 14 ++ .../validation/ExportOptionsValidator.java | 115 +++++++--- .../ExportOptionsValidatorTest.java | 206 ++++++++++-------- 3 files changed, 214 insertions(+), 121 deletions(-) diff --git a/core/src/main/java/com/scalar/db/common/error/CoreError.java b/core/src/main/java/com/scalar/db/common/error/CoreError.java index 3a41f7b20b..7535c3553d 100644 --- a/core/src/main/java/com/scalar/db/common/error/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/error/CoreError.java @@ -694,6 +694,20 @@ public enum CoreError implements ScalarDbError { Category.USER_ERROR, "0152", "The provided clustering key %s was not found", "", ""), DATA_LOADER_INVALID_PROJECTION( Category.USER_ERROR, "0153", "The column '%s' was not found", "", ""), + DATA_LOADER_INCOMPLETE_PARTITION_KEY( + Category.USER_ERROR, "0154", "The provided partition key is incomplete. Required key: %s", "", ""), + DATA_LOADER_CLUSTERING_KEY_ORDER_MISMATCH( + Category.USER_ERROR, + "0155", + "The provided clustering key order does not match the table schema. Required order: %s", + "", + ""), + DATA_LOADER_PARTITION_KEY_ORDER_MISMATCH( + Category.USER_ERROR, + "0156", + "The provided partition key order does not match the table schema. Required order: %s", + "", + ""), // // Errors for the concurrency error category diff --git a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java index 143fabfd33..7bf7645b0e 100644 --- a/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java +++ b/data-loader/core/src/main/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidator.java @@ -5,7 +5,9 @@ import com.scalar.db.common.error.CoreError; import com.scalar.db.dataloader.core.ScanRange; import com.scalar.db.dataloader.core.dataexport.ExportOptions; +import com.scalar.db.io.Column; import com.scalar.db.io.Key; +import java.util.Iterator; import java.util.LinkedHashSet; import java.util.List; import lombok.AccessLevel; @@ -27,57 +29,107 @@ public class ExportOptionsValidator { */ public static void validate(ExportOptions exportOptions, TableMetadata tableMetadata) throws ExportOptionsValidationException { + LinkedHashSet partitionKeyNames = tableMetadata.getPartitionKeyNames(); LinkedHashSet clusteringKeyNames = tableMetadata.getClusteringKeyNames(); ScanRange scanRange = exportOptions.getScanRange(); - // Validate projection columns + validatePartitionKey(partitionKeyNames, exportOptions.getScanPartitionKey()); validateProjectionColumns(tableMetadata.getColumnNames(), exportOptions.getProjectionColumns()); + validateSortOrders(clusteringKeyNames, exportOptions.getSortOrders()); - // Validate sort orders - if (!exportOptions.getSortOrders().isEmpty()) { - for (Scan.Ordering sort : exportOptions.getSortOrders()) { - validateClusteringKey(clusteringKeyNames, sort.getColumnName()); - } - } - - // Validate scan start key if (scanRange.getScanStartKey() != null) { validateClusteringKey(clusteringKeyNames, scanRange.getScanStartKey()); } - - // Validate scan end key if (scanRange.getScanEndKey() != null) { validateClusteringKey(clusteringKeyNames, scanRange.getScanEndKey()); } } - /** - * Checks if the provided clustering key is valid for the ScalarDB table. - * - * @param clusteringKeyNames The set of valid clustering key names for the table. - * @param key The ScalarDB key to validate. - * @throws ExportOptionsValidationException If the key is invalid or not a clustering key. + /* + * Check if the provided partition key is available in the ScalarDB table + * @param partitionKeyNames List of partition key names available in a + * @param key To be validated ScalarDB key + * @throws ExportOptionsValidationException if the key could not be found or is not a partition */ - private static void validateClusteringKey(LinkedHashSet clusteringKeyNames, Key key) + private static void validatePartitionKey(LinkedHashSet partitionKeyNames, Key key) + throws ExportOptionsValidationException { + if (partitionKeyNames == null || key == null) { + return; + } + + // Make sure that all partition key columns are provided + if (partitionKeyNames.size() != key.getColumns().size()) { + throw new ExportOptionsValidationException( + CoreError.DATA_LOADER_INCOMPLETE_PARTITION_KEY.buildMessage(partitionKeyNames)); + } + + // Check if the order of columns in key.getColumns() matches the order in partitionKeyNames + Iterator partitionKeyIterator = partitionKeyNames.iterator(); + for (Column column : key.getColumns()) { + // Check if the column names match in order + if (!partitionKeyIterator.hasNext() + || !partitionKeyIterator.next().equals(column.getName())) { + throw new ExportOptionsValidationException( + CoreError.DATA_LOADER_PARTITION_KEY_ORDER_MISMATCH.buildMessage(partitionKeyNames)); + } + } + } + + private static void validateSortOrders( + LinkedHashSet clusteringKeyNames, List sortOrders) throws ExportOptionsValidationException { - if (clusteringKeyNames == null) { + if (sortOrders == null || sortOrders.isEmpty()) { return; } - String columnName = key.getColumnName(0); - validateClusteringKey(clusteringKeyNames, columnName); + + for (Scan.Ordering sortOrder : sortOrders) { + checkIfColumnExistsAsClusteringKey(clusteringKeyNames, sortOrder.getColumnName()); + } } /** - * Checks if the provided clustering key column name is valid for the ScalarDB table. + * Validates that the clustering key columns in the given Key object match the expected order + * defined in the clusteringKeyNames. The Key can be a prefix of the clusteringKeyNames, but the + * order must remain consistent. * - * @param clusteringKeyNames The set of valid clustering key names for the table. - * @param columnName The column name of the clustering key to validate. - * @throws ExportOptionsValidationException If the column name is not a valid clustering key. + * @param clusteringKeyNames the expected ordered set of clustering key names + * @param key the Key object containing the actual clustering key columns + * @throws ExportOptionsValidationException if the order or names of clustering keys do not match */ - private static void validateClusteringKey( + private static void validateClusteringKey(LinkedHashSet clusteringKeyNames, Key key) + throws ExportOptionsValidationException { + // If either clusteringKeyNames or key is null, no validation is needed + if (clusteringKeyNames == null || key == null) { + return; + } + + // Create an iterator to traverse the clusteringKeyNames in order + Iterator clusteringKeyIterator = clusteringKeyNames.iterator(); + + // Iterate through the columns in the given Key + for (Column column : key.getColumns()) { + // If clusteringKeyNames have been exhausted but columns still exist in the Key, + // it indicates a mismatch + if (!clusteringKeyIterator.hasNext()) { + throw new ExportOptionsValidationException( + CoreError.DATA_LOADER_CLUSTERING_KEY_ORDER_MISMATCH.buildMessage(clusteringKeyNames)); + } + + // Get the next expected clustering key name + String expectedKey = clusteringKeyIterator.next(); + + // Check if the current column name matches the expected clustering key name + if (!column.getName().equals(expectedKey)) { + throw new ExportOptionsValidationException( + CoreError.DATA_LOADER_CLUSTERING_KEY_ORDER_MISMATCH.buildMessage(clusteringKeyNames)); + } + } + } + + private static void checkIfColumnExistsAsClusteringKey( LinkedHashSet clusteringKeyNames, String columnName) throws ExportOptionsValidationException { - if (clusteringKeyNames == null) { + if (clusteringKeyNames == null || columnName == null) { return; } @@ -87,21 +139,14 @@ private static void validateClusteringKey( } } - /** - * Checks if the provided projection column names are valid for the ScalarDB table. - * - * @param columnNames The set of valid column names for the table. - * @param columns The list of projection column names to validate. - * @throws ExportOptionsValidationException If any of the column names are invalid. - */ private static void validateProjectionColumns( LinkedHashSet columnNames, List columns) throws ExportOptionsValidationException { if (columns == null || columns.isEmpty()) { return; } + for (String column : columns) { - // O(n) lookup, but acceptable given the typically small list size if (!columnNames.contains(column)) { throw new ExportOptionsValidationException( CoreError.DATA_LOADER_INVALID_PROJECTION.buildMessage(column)); diff --git a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidatorTest.java b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidatorTest.java index b34daaba2d..b36522a0fc 100644 --- a/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidatorTest.java +++ b/data-loader/core/src/test/java/com/scalar/db/dataloader/core/dataexport/validation/ExportOptionsValidatorTest.java @@ -2,7 +2,6 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; -import com.scalar.db.api.Scan; import com.scalar.db.api.TableMetadata; import com.scalar.db.common.error.CoreError; import com.scalar.db.dataloader.core.FileFormat; @@ -14,136 +13,171 @@ import com.scalar.db.io.TextColumn; import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; class ExportOptionsValidatorTest { - TableMetadata mockMetadata; - List projectedColumns; + private TableMetadata singlePkCkMetadata; + private TableMetadata multiplePkCkMetadata; + private List projectedColumns; @BeforeEach void setup() { - mockMetadata = - TableMetadata.newBuilder() - .addColumn("id", DataType.INT) - .addColumn("name", DataType.TEXT) - .addColumn("email", DataType.TEXT) - .addColumn("department", DataType.TEXT) - .addPartitionKey("id") - .addClusteringKey("department") - .build(); - projectedColumns = new ArrayList<>(); - projectedColumns.add("id"); - projectedColumns.add("name"); - projectedColumns.add("email"); - projectedColumns.add("department"); + singlePkCkMetadata = createMockMetadata(1, 1); + multiplePkCkMetadata = createMockMetadata(2, 2); + projectedColumns = createProjectedColumns(); + } + + private TableMetadata createMockMetadata(int pkCount, int ckCount) { + TableMetadata.Builder builder = TableMetadata.newBuilder(); + + // Add partition keys + for (int i = 1; i <= pkCount; i++) { + builder.addColumn("pk" + i, DataType.INT); + builder.addPartitionKey("pk" + i); + } + + // Add clustering keys + for (int i = 1; i <= ckCount; i++) { + builder.addColumn("ck" + i, DataType.TEXT); + builder.addClusteringKey("ck" + i); + } + + return builder.build(); + } + + private List createProjectedColumns() { + List columns = new ArrayList<>(); + columns.add("pk1"); + columns.add("ck1"); + return columns; } @Test - void validate_withValidExportOptions_ShouldNotThrowException() + void validate_withValidExportOptionsForSinglePkCk_ShouldNotThrowException() throws ExportOptionsValidationException { + + Key partitionKey = Key.newBuilder().add(IntColumn.of("pk1", 1)).build(); + ExportOptions exportOptions = - ExportOptions.builder( - "test", - "sample", - Key.newBuilder().add(IntColumn.of("id", 1)).build(), - FileFormat.JSON) - .sortOrders(Collections.emptyList()) - .scanRange(new ScanRange(null, null, false, false)) + ExportOptions.builder("test", "sample", partitionKey, FileFormat.JSON) .projectionColumns(projectedColumns) + .scanRange(new ScanRange(null, null, false, false)) .build(); - ExportOptionsValidator.validate(exportOptions, mockMetadata); + + ExportOptionsValidator.validate(exportOptions, singlePkCkMetadata); } @Test - void validate_withInValidColumnInProjectionColumnList_ShouldThrowException() { + void validate_withValidExportOptionsForMultiplePkCk_ShouldNotThrowException() + throws ExportOptionsValidationException { + + Key partitionKey = + Key.newBuilder().add(IntColumn.of("pk1", 1)).add(IntColumn.of("pk2", 2)).build(); + ExportOptions exportOptions = - ExportOptions.builder( - "test", - "sample", - Key.newBuilder().add(IntColumn.of("id", 1)).build(), - FileFormat.JSON) - .sortOrders(Collections.emptyList()) + ExportOptions.builder("test", "sample", partitionKey, FileFormat.JSON) + .projectionColumns(projectedColumns) .scanRange(new ScanRange(null, null, false, false)) - .projectionColumns(Collections.singletonList("sample")) .build(); - assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, mockMetadata)) + + ExportOptionsValidator.validate(exportOptions, multiplePkCkMetadata); + } + + @Test + void validate_withIncompletePartitionKeyForSinglePk_ShouldThrowException() { + Key incompletePartitionKey = Key.newBuilder().build(); + + ExportOptions exportOptions = + ExportOptions.builder("test", "sample", incompletePartitionKey, FileFormat.JSON).build(); + + assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, singlePkCkMetadata)) .isInstanceOf(ExportOptionsValidationException.class) - .hasMessage(CoreError.DATA_LOADER_INVALID_PROJECTION.buildMessage("sample")); + .hasMessage( + CoreError.DATA_LOADER_INCOMPLETE_PARTITION_KEY.buildMessage( + singlePkCkMetadata.getPartitionKeyNames())); } @Test - void validate_withInValidSortOrderWithMultipleClusteringKeys_ShouldThrowException() { - mockMetadata = - TableMetadata.newBuilder() - .addColumn("id", DataType.INT) - .addColumn("name", DataType.TEXT) - .addColumn("email", DataType.TEXT) - .addColumn("department", DataType.TEXT) - .addColumn("building", DataType.TEXT) - .addPartitionKey("id") - .addClusteringKey("department") - .addClusteringKey("building") - .build(); + void validate_withIncompletePartitionKeyForMultiplePks_ShouldThrowException() { + Key incompletePartitionKey = Key.newBuilder().add(IntColumn.of("pk1", 1)).build(); + + ExportOptions exportOptions = + ExportOptions.builder("test", "sample", incompletePartitionKey, FileFormat.JSON).build(); + + assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, multiplePkCkMetadata)) + .isInstanceOf(ExportOptionsValidationException.class) + .hasMessage( + CoreError.DATA_LOADER_INCOMPLETE_PARTITION_KEY.buildMessage( + multiplePkCkMetadata.getPartitionKeyNames())); + } + + @Test + void validate_withInvalidProjectionColumn_ShouldThrowException() { ExportOptions exportOptions = ExportOptions.builder( "test", "sample", - Key.newBuilder().add(IntColumn.of("id", 1)).build(), + Key.newBuilder().add(IntColumn.of("pk1", 1)).build(), FileFormat.JSON) - .sortOrders( - Collections.singletonList(new Scan.Ordering("name", Scan.Ordering.Order.ASC))) - .scanRange(new ScanRange(null, null, false, false)) - .projectionColumns(Collections.singletonList("id")) + .projectionColumns(Collections.singletonList("invalid_column")) .build(); - assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, mockMetadata)) + + assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, singlePkCkMetadata)) .isInstanceOf(ExportOptionsValidationException.class) - .hasMessage(CoreError.DATA_LOADER_CLUSTERING_KEY_NOT_FOUND.buildMessage("name")); + .hasMessage(CoreError.DATA_LOADER_INVALID_PROJECTION.buildMessage("invalid_column")); } @Test - void validate_withInValidKeyInSortRange_ShouldThrowException() { + void validate_withInvalidClusteringKeyInScanRange_ShouldThrowException() { + ScanRange scanRange = + new ScanRange( + Key.newBuilder().add(TextColumn.of("invalid_ck", "value")).build(), + Key.newBuilder().add(TextColumn.of("ck1", "value")).build(), + false, + false); + ExportOptions exportOptions = - ExportOptions.builder( - "test", - "sample", - Key.newBuilder().add(IntColumn.of("id", 1)).build(), - FileFormat.JSON) - .sortOrders(Collections.emptyList()) - .scanRange( - new ScanRange( - Key.newBuilder().add(IntColumn.of("id", 1)).build(), - Key.newBuilder().add(IntColumn.of("id", 100)).build(), - false, - false)) - .projectionColumns(Collections.singletonList("id")) + ExportOptions.builder("test", "sample", createValidPartitionKey(), FileFormat.JSON) + .scanRange(scanRange) .build(); - assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, mockMetadata)) + + assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, singlePkCkMetadata)) .isInstanceOf(ExportOptionsValidationException.class) - .hasMessage(CoreError.DATA_LOADER_CLUSTERING_KEY_NOT_FOUND.buildMessage("id")); + .hasMessage(CoreError.DATA_LOADER_CLUSTERING_KEY_ORDER_MISMATCH.buildMessage("[ck1]")); } @Test - void validate_withInValidEndKeyInSortRange_ShouldThrowException() { + void validate_withInvalidPartitionKeyOrder_ShouldThrowException() { + // Partition key names are expected to be "pk1", "pk2" + LinkedHashSet partitionKeyNames = new LinkedHashSet<>(); + partitionKeyNames.add("pk1"); + partitionKeyNames.add("pk2"); + + // Create a partition key with reversed order, expecting an error + Key invalidPartitionKey = + Key.newBuilder() + .add(IntColumn.of("pk2", 2)) // Incorrect order + .add(IntColumn.of("pk1", 1)) // Incorrect order + .build(); + ExportOptions exportOptions = - ExportOptions.builder( - "test", - "sample", - Key.newBuilder().add(IntColumn.of("id", 1)).build(), - FileFormat.JSON) - .sortOrders(Collections.emptyList()) - .scanRange( - new ScanRange( - Key.newBuilder().add(TextColumn.of("department", "sample")).build(), - Key.newBuilder().add(IntColumn.of("name", 100)).build(), - false, - false)) + ExportOptions.builder("test", "sample", invalidPartitionKey, FileFormat.JSON) .projectionColumns(projectedColumns) + .scanRange(new ScanRange(null, null, false, false)) .build(); - assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, mockMetadata)) + + // Verify that the validator throws the correct exception + assertThatThrownBy(() -> ExportOptionsValidator.validate(exportOptions, multiplePkCkMetadata)) .isInstanceOf(ExportOptionsValidationException.class) - .hasMessage(CoreError.DATA_LOADER_CLUSTERING_KEY_NOT_FOUND.buildMessage("name")); + .hasMessage( + CoreError.DATA_LOADER_PARTITION_KEY_ORDER_MISMATCH.buildMessage(partitionKeyNames)); + } + + private Key createValidPartitionKey() { + return Key.newBuilder().add(IntColumn.of("pk1", 1)).build(); } } From 78a817067a710673d6671cbab4af4f1050a4e8bf Mon Sep 17 00:00:00 2001 From: Peckstadt Yves Date: Mon, 23 Dec 2024 17:34:37 +0900 Subject: [PATCH 5/5] Fix spotless format --- .../com/scalar/db/common/error/CoreError.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/com/scalar/db/common/error/CoreError.java b/core/src/main/java/com/scalar/db/common/error/CoreError.java index 7535c3553d..7a8d518320 100644 --- a/core/src/main/java/com/scalar/db/common/error/CoreError.java +++ b/core/src/main/java/com/scalar/db/common/error/CoreError.java @@ -695,7 +695,11 @@ public enum CoreError implements ScalarDbError { DATA_LOADER_INVALID_PROJECTION( Category.USER_ERROR, "0153", "The column '%s' was not found", "", ""), DATA_LOADER_INCOMPLETE_PARTITION_KEY( - Category.USER_ERROR, "0154", "The provided partition key is incomplete. Required key: %s", "", ""), + Category.USER_ERROR, + "0154", + "The provided partition key is incomplete. Required key: %s", + "", + ""), DATA_LOADER_CLUSTERING_KEY_ORDER_MISMATCH( Category.USER_ERROR, "0155", @@ -703,11 +707,11 @@ public enum CoreError implements ScalarDbError { "", ""), DATA_LOADER_PARTITION_KEY_ORDER_MISMATCH( - Category.USER_ERROR, - "0156", - "The provided partition key order does not match the table schema. Required order: %s", - "", - ""), + Category.USER_ERROR, + "0156", + "The provided partition key order does not match the table schema. Required order: %s", + "", + ""), // // Errors for the concurrency error category