Add export command for data loader CLI #2617

inv-jishnu · 2025-04-30T12:11:27Z

Description

In this PR I am adding data loader export command and command options for data export.

Related issues and/or PRs

Please review this PR once the below PRs is merged and latest changes from master are merged to this branch

Add util classes for data loader CLI #2616

Changes made

I have added the export command options and export command as a CLI command for data export

Checklist

The following is a best-effort checklist. If any items in this checklist are not applicable to this PR or are dependent on other, unmerged PRs, please still mark the checkboxes after you have read and understood each item.

I have commented my code, particularly in hard-to-understand areas.
I have updated the documentation to reflect the changes.
Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
Tests (unit, integration, etc.) have been added for the changes.
My changes generate no new warnings.
Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

This is the 2nd PR for dataloader CLI

Release notes

NA

Copilot

Pull Request Overview

This PR adds a new export command to the data-loader CLI, implements related parsing utilities and converters, and updates tests and error definitions.

Introduces parseMultipleKeyValues in KeyUtils and related tests for multi-column key parsing.
Adds CLI parsing utilities (CommandLineInputUtils) and converters (SingleColumnKeyValueConverter, MultiColumnKeyValueConverter, ScanOrderingConverter).
Implements ExportCommand and ExportCommandOptions to handle export logic, file/path validation, and output formatting; updates tests and error codes.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/KeyUtils.java	Added `parseMultipleKeyValues` to build a `Key` from multiple values
data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/KeyUtilsTest.java	Added test for `parseMultipleKeyValues`
data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/util/CommandLineInputUtils.java	Added `parseKeyValue` and `splitByDelimiter` with error handling
data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/SingleColumnKeyValueConverter.java	New converter for single key-value CLI inputs
data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/MultiColumnKeyValueConverter.java	New converter for multi key-value CLI inputs
data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ScanOrderingConverter.java	New converter for scan ordering
data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandOptions.java	Defined CLI options for export command
data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommand.java	Implemented export logic, path validation, manager creation
data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandTest.java	Updated tests for blank and invalid config paths
core/src/main/java/com/scalar/db/common/error/CoreError.java	Added new error codes for data loader CLI

Comments suppressed due to low confidence (2)

data-loader/core/src/test/java/com/scalar/db/dataloader/core/util/KeyUtilsTest.java:249

The test only asserts the column name; consider also verifying the parsed column value and the total number of columns to improve coverage for parseMultipleKeyValues.

assertEquals(c1, key.getColumnName(0));

data-loader/cli/src/test/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommandTest.java:16

This cleanup method is not annotated (e.g., @AfterEach), so it won't run. Add the appropriate JUnit annotation to ensure test artifacts are removed.

void removeFileIfCreated() {

Copilot · 2025-05-20T06:08:03Z

data-loader/core/src/main/java/com/scalar/db/dataloader/core/util/KeyUtils.java

+   * Convert a keyValue, in the format of <key>=<value>, to a ScalarDB Key instance.
+   *
+   * @param keyValues A list of key values in the format of <key>=<value>


The @param description is misleading: keyValues is a list of ColumnKeyValue objects, not raw strings. Update the documentation to reflect that.

Suggested change

* Convert a keyValue, in the format of <key>=<value>, to a ScalarDB Key instance.

*

* @param keyValues A list of key values in the format of <key>=<value>

* Convert a list of ColumnKeyValue objects to a ScalarDB Key instance.

*

* @param keyValues A list of ColumnKeyValue objects, where each object contains a column name

* and its corresponding value

Copilot · 2025-05-20T06:08:03Z

...-loader/cli/src/main/java/com/scalar/db/dataloader/cli/command/dataexport/ExportCommand.java

-    validateOutputDirectory(outputFilePath);
-    return 0;
-  }
+    String scalarDbPropertiesFilePath = getScalarDbPropertiesFilePath();


Tests expect an IllegalArgumentException when configFilePath is blank, but the current implementation catches all invalid file paths and returns an exit code. Add an explicit check for blank configFilePath and throw IllegalArgumentException with CoreError.DATA_LOADER_FILE_PATH_IS_BLANK to match test expectations.

This reverts commit 210f62d.

komamitsu · 2025-05-21T02:19:05Z

data-loader/cli/src/main/java/com/scalar/db/dataloader/cli/util/CommandLineInputUtils.java

+   * @return a {@link ColumnKeyValue} object representing the parsed key-value pair
+   * @throws IllegalArgumentException if the input is null, empty, or not in the expected format
+   */
+  public static ColumnKeyValue parseKeyValue(String keyValue) {


This class is CommandLineInputUtils and the method name is parseKeyValue(). So, it doesn't seem to return the specific type ColumnKeyValue. How about returning more generic type like Pair<K, V> or Tuple<K, V> (or Map.Entry<K, V>?) ?

See also #2617 (comment)

@komamitsu san,
I have updated the util method implementation and also made changes to use the function to parse the scan order.
Thank you.

inv-jishnu · 2025-05-21T09:05:19Z

@komamitsu san,
I have made changes based on feedback.
Please take a look at this again when you get a chance.
Thank you.

komamitsu

LGTM! 👍

brfrn169

Left a minor comment. Other than that, LGTM! Thank you!

brfrn169 · 2025-05-21T13:56:32Z

core/src/main/java/com/scalar/db/common/error/CoreError.java

+  DATA_LOADER_INVALID_KEY_VALUE_INPUT(
+      Category.USER_ERROR, "0201", "Invalid key-value format: %s", "", ""),
+  DATA_LOADER_SPLIT_INPUT_VALUE_NULL(Category.USER_ERROR, "0202", "Value must not be null", "", ""),
+


Please remove this empty line.

Suggested change

@brfrn169 san,
I have removed the empty line.
Thank you.

ypeckstadt

LGTM. Thank you.

Torch3333

LGTM, thank you!

Co-authored-by: Peckstadt Yves <[email protected]> Co-authored-by: Toshihiro Suzuki <[email protected]>

inv-jishnu and others added 20 commits April 10, 2025 15:10

Initial commit

be9ee23

Spotless applied again

89b9f05

Removed unused code

49c83b6

Merge branch 'master' into feat/data-loader/import-log-2

b2871fb

Removed unused classes and references

c5c9c0a

Merge branch 'master' into feat/data-loader/import-log-2

4964e8d

Merge branch 'master' into feat/data-loader/import-log-2

ff81f5f

Improve Javadocs

3934c2a

Changes

9958f95

Renamed parameters

1afbc21

logging changes

8c5114d

removed repeated code

ffab395

Merge branch 'master' into feat/data-loader/import-log-2

79df1ed

Merge branch 'master' into feat/data-loader/import-log-2

cf31672

Added excetpion throw

6dd213e

Synchronisation changes

6542177

Added volatile back to fix spotbugs issue

603e46e

Removed unused variable

eaf9d88

Changes

0a2518a

Initial commit

378effb

inv-jishnu added the enhancement New feature or request label Apr 30, 2025

inv-jishnu self-assigned this Apr 30, 2025

inv-jishnu changed the title ~~Feat/data loader/cli export 2~~ Add export command for data loader CLI Apr 30, 2025

This was referenced Apr 30, 2025

Add export command for data loader CLI #2490

Closed

Add import command for data loader CLI #2618

Open

inv-jishnu marked this pull request as draft April 30, 2025 12:16

Base automatically changed from feat/data-loader/import-log-2 to master May 8, 2025 04:09

inv-jishnu added 3 commits May 15, 2025 09:42

Merged changes from master after resolving conflicts

573cfae

Removed unused file

7f7d34b

Minor name changes

a842dc6

Copilot AI reviewed May 20, 2025

View reviewed changes

inv-jishnu added 4 commits May 20, 2025 14:51

Merged latest changes from master after resolving conflicts

105367c

Javadoc change

145b239

Changed exception thrown based on feedback

210f62d

Revert "Changed exception thrown based on feedback"

676c12d

This reverts commit 210f62d.

komamitsu reviewed May 21, 2025

View reviewed changes

inv-jishnu and others added 5 commits May 21, 2025 10:27

Changes

d05ec66

Merge branch 'master' into feat/data-loader/cli-export-2

07243a4

Additional check added

e0ecb75

Minor changes

f595434

Scan order input read updated

8366167

inv-jishnu requested a review from komamitsu May 21, 2025 09:05

komamitsu approved these changes May 22, 2025

View reviewed changes

brfrn169 approved these changes May 22, 2025

View reviewed changes

Empty line removed

8992201

ypeckstadt approved these changes May 22, 2025

View reviewed changes

Torch3333 approved these changes May 22, 2025

View reviewed changes

ypeckstadt merged commit 32179a6 into master May 22, 2025
55 checks passed

ypeckstadt deleted the feat/data-loader/cli-export-2 branch May 22, 2025 05:58

feeblefakie pushed a commit that referenced this pull request May 22, 2025

Add export command for data loader CLI (#2617)

fd17840

Co-authored-by: Peckstadt Yves <[email protected]> Co-authored-by: Toshihiro Suzuki <[email protected]>

feeblefakie pushed a commit that referenced this pull request May 22, 2025

Add export command for data loader CLI (#2617)

21a362a

Co-authored-by: Peckstadt Yves <[email protected]> Co-authored-by: Toshihiro Suzuki <[email protected]>

feeblefakie mentioned this pull request May 22, 2025

Backport to branch(3) : Add export command for data loader CLI #2688

Merged

inv-jishnu added a commit that referenced this pull request May 22, 2025

Add export command for data loader CLI (#2617)

f0550fb

Co-authored-by: Peckstadt Yves <[email protected]> Co-authored-by: Toshihiro Suzuki <[email protected]>

inv-jishnu added a commit that referenced this pull request May 22, 2025

Add export command for data loader CLI (#2617)

6ae9001

Co-authored-by: Peckstadt Yves <[email protected]> Co-authored-by: Toshihiro Suzuki <[email protected]>

inv-jishnu added a commit that referenced this pull request May 22, 2025

Add export command for data loader CLI (#2617)

28fd900

Co-authored-by: Peckstadt Yves <[email protected]> Co-authored-by: Toshihiro Suzuki <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add export command for data loader CLI #2617

Add export command for data loader CLI #2617

Uh oh!

inv-jishnu commented Apr 30, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI May 20, 2025

Uh oh!

Copilot AI May 20, 2025

Uh oh!

komamitsu May 21, 2025 •

edited

Loading

Uh oh!

inv-jishnu May 21, 2025

Uh oh!

inv-jishnu commented May 21, 2025

Uh oh!

komamitsu left a comment

Uh oh!

brfrn169 left a comment

Uh oh!

brfrn169 May 21, 2025

Uh oh!

inv-jishnu May 22, 2025

Uh oh!

ypeckstadt left a comment

Uh oh!

Torch3333 left a comment

Uh oh!

Uh oh!

Uh oh!

-   * Convert a keyValue, in the format of <key>=<value>, to a ScalarDB Key instance.
-   *
-   * @param keyValues A list of key values in the format of <key>=<value>
+   * Convert a list of ColumnKeyValue objects to a ScalarDB Key instance.
+   *
+   * @param keyValues A list of ColumnKeyValue objects, where each object contains a column name
+   *     and its corresponding value

Add export command for data loader CLI #2617

Add export command for data loader CLI #2617

Uh oh!

Conversation

inv-jishnu commented Apr 30, 2025

Description

Related issues and/or PRs

Changes made

Checklist

Additional notes (optional)

Release notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI May 20, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI May 20, 2025

Choose a reason for hiding this comment

Uh oh!

komamitsu May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inv-jishnu May 21, 2025

Choose a reason for hiding this comment

Uh oh!

inv-jishnu commented May 21, 2025

Uh oh!

komamitsu left a comment

Choose a reason for hiding this comment

Uh oh!

brfrn169 left a comment

Choose a reason for hiding this comment

Uh oh!

brfrn169 May 21, 2025

Choose a reason for hiding this comment

Uh oh!

inv-jishnu May 22, 2025

Choose a reason for hiding this comment

Uh oh!

ypeckstadt left a comment

Choose a reason for hiding this comment

Uh oh!

Torch3333 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

komamitsu May 21, 2025 •

edited

Loading