Add Control file module files and validation #2445

inv-jishnu · 2025-01-02T05:03:15Z

Description

This PR adds files related to control file and its validation. A control file is used to map column names specified in import source file to actual table columns.

Related issues and/or PRs

Please review and merge this PR once the following PRs are merged

Changes made

I have added files related to control file and its validation

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)

Road map to merge remaining data loader core files. Current status

General
- Add ScalarDB Dao and related files - Add ScalarDB Dao and related files #2417
- TableMetadataService(partially replaced by ConsensusUtils): Add table metadata service #2434
Export
- Export options and validations: Add export options validator #2435
- ProducerTasks: 1 PR incoming
Import
- Dto classes and utilities: Add data chunk and task result enums and dtos #2442
- Import processor and task code: 2-3 PRs incoming
- Code for Import transaction batch and data chunk import: 1 PR Incoming
- ControlFile related Dtos: Add Control file module files and validation #2445
- Import logger: 1 PR incoming
- Import Manager: 1 PR incoming

Release notes

NA

Torch3333

LGTM, thank you!

komamitsu · 2025-02-05T02:09:01Z

...src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileValidator.java

+      throws ControlFileValidationException {
+    Set<String> mappedTargetColumns = new HashSet<>();
+    for (ControlFileTableFieldMapping mapping : controlFileTable.getMappings()) {
+      if (mappedTargetColumns.contains(mapping.getTargetColumn())) {


[minor] You can call mappedTargetColumns.add(columnName) instead as it returns false when the entry already exists.

I confused this line with https://github.com/scalar-labs/scalardb/pull/2445/files#diff-fe051a3fcbc578fb1d270a16e032dbb16af0284fbd34ef077783848fba783138R89...

komamitsu · 2025-02-05T02:12:58Z

...src/main/java/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileValidator.java

+   * @return Set of uniquely mapped target columns
+   * @throws ControlFileValidationException when a duplicate mapping is found
+   */
+  private static Set<String> checkDuplicateColumnMappings(ControlFileTable controlFileTable)


I think this function name should be getTargetColumnSet or something since it converts the data structure not only checking. What do you think?

inv-jishnu · 2025-02-05T10:11:16Z

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

komamitsu

LGTM! 👍

brfrn169

Left a couple of comments. Please take a look when you have time!

brfrn169 · 2025-02-05T03:39:21Z

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

@@ -765,6 +765,44 @@ public enum CoreError implements ScalarDbError {
  DATA_LOADER_TABLE_METADATA_RETRIEVAL_FAILED(
      Category.USER_ERROR, "0166", "Failed to retrieve table metadata. Details: %s", "", ""),



Suggested change

brfrn169 · 2025-02-05T03:39:56Z

...ava/com/scalar/db/dataloader/core/dataimport/controlfile/ControlFileValidationException.java

@@ -0,0 +1,14 @@
+package com.scalar.db.dataloader.core.dataimport.controlfile;
+
+/** Represents the control file */


This javadoc is wrong, right?

@brfrn169 san,

I have updated the javadoc with more details.
Please check it when you get a chance and if it needs further changes, I will update it.
Thank you.

brfrn169

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

brfrn169 · 2025-02-06T07:55:58Z

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

+      "0173",
+      "Duplicated data mappings found for column '%s' in table '%s'",
+      "",
+      ""),
  //


Could you please add an empty line here?

Suggested change

//

//

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

inv-jishnu and others added 30 commits December 4, 2024 15:59

Util classes for data loader

753618b

Fix spotbug issue

8d39d02

Removed error message and added core error

bf94c49

Applied spotless

47be388

Fixed unit test failures

913eb1c

Merge branch 'master' into feat/data-loader/utils

1f204b8

Basic data import enum and exception

6cfa83a

Removed exception class for now

d381b2b

Added DECIMAL_FORMAT

67f2474

Path util class updated

14e3593

Feedback changes

a096d51

Merge branch 'master' into feat/data-loader/utils

dbf1940

Merge branch 'master' into feat/data-loader/utils

cd8add9

Changes

52890c8

Merge branch 'master' into feat/data-loader/import-data-1

5114639

Merge branch 'feat/data-loader/utils' into feat/data-loader/scaladb-dao

4f9cd75

Added ScalarDB Dao

1997eb8

Merge branch 'master' into feat/data-loader/scaladb-dao

91e6310

Remove unnecessary files

8a7338b

Initial commit [skip ci]

2b52eeb

Changes

e206073

Changes

26d3144

spotbugs exclude

b86487d

spotbugs exclude -2

818a2b4

Added a file [skip ci]

90c4105

Added unit test files [skip ci]

3d5d3e0

Spotbug fixes

6495202

Removed use of List.of to fix CI error

90abd9e

Merged changes from master after resolving conflict

ba2b3dd

Merge branch 'master' into feat/data-loader/metadata-service

b1b811b

ypeckstadt marked this pull request as ready for review February 4, 2025 05:57

inv-jishnu requested a review from komamitsu February 4, 2025 06:07

Torch3333 approved these changes Feb 5, 2025

View reviewed changes

komamitsu reviewed Feb 5, 2025

View reviewed changes

Chages

c2e4a0c

inv-jishnu requested a review from komamitsu February 5, 2025 10:10

Merge branch 'master' into feat/data-loader/control-file

04b46a0

komamitsu approved these changes Feb 6, 2025

View reviewed changes

brfrn169 reviewed Feb 6, 2025

View reviewed changes

inv-jishnu and others added 2 commits February 6, 2025 12:06

Changes

d399489

Merge branch 'master' into feat/data-loader/control-file

a817581

inv-jishnu requested a review from brfrn169 February 6, 2025 07:45

brfrn169 approved these changes Feb 6, 2025

View reviewed changes

inv-jishnu and others added 2 commits February 6, 2025 14:00

Added newline to core error

160e3df

Merge branch 'master' into feat/data-loader/control-file

2bdba20

ypeckstadt merged commit 8164078 into master Feb 7, 2025
48 checks passed

ypeckstadt deleted the feat/data-loader/control-file branch February 7, 2025 07:55

feeblefakie pushed a commit that referenced this pull request Feb 7, 2025

Add Control file module files and validation (#2445)

4e556a0

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

feeblefakie mentioned this pull request Feb 7, 2025

Backport to branch(3.15) : Add Control file module files and validation #2523

Merged

feeblefakie pushed a commit that referenced this pull request Feb 7, 2025

Add Control file module files and validation (#2445)

610c118

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

feeblefakie mentioned this pull request Feb 7, 2025

Backport to branch(3) : Add Control file module files and validation #2524

Merged

feeblefakie pushed a commit that referenced this pull request Feb 7, 2025

Add Control file module files and validation (#2445)

68207ce

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

feeblefakie mentioned this pull request Feb 7, 2025

Backport to branch(3.14) : Add Control file module files and validation #2525

Merged

feeblefakie pushed a commit that referenced this pull request Feb 7, 2025

Add Control file module files and validation (#2445)

72c374b

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

feeblefakie mentioned this pull request Feb 7, 2025

Backport to branch(3.13) : Add Control file module files and validation #2526

Merged

feeblefakie pushed a commit that referenced this pull request Feb 7, 2025

Add Control file module files and validation (#2445)

54af182

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

This was referenced Feb 7, 2025

Backport to branch(3.12) : Add Control file module files and validation #2527

Merged

Backport to branch(3.11) : Add Control file module files and validation #2528

Merged

inv-jishnu added a commit that referenced this pull request Feb 7, 2025

Add Control file module files and validation (#2445)

5fcdf70

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Control file module files and validation #2445

Add Control file module files and validation #2445

inv-jishnu commented Jan 2, 2025 •

edited

Loading

Torch3333 left a comment

komamitsu Feb 5, 2025

komamitsu Feb 5, 2025

inv-jishnu commented Feb 5, 2025

komamitsu left a comment

brfrn169 left a comment

brfrn169 Feb 5, 2025

brfrn169 Feb 5, 2025

inv-jishnu Feb 6, 2025

brfrn169 left a comment

brfrn169 Feb 6, 2025

		@@ -765,6 +765,44 @@ public enum CoreError implements ScalarDbError {
		DATA_LOADER_TABLE_METADATA_RETRIEVAL_FAILED(
		Category.USER_ERROR, "0166", "Failed to retrieve table metadata. Details: %s", "", ""),

		@@ -0,0 +1,14 @@
		package com.scalar.db.dataloader.core.dataimport.controlfile;

		/** Represents the control file */

Add Control file module files and validation #2445

Add Control file module files and validation #2445

Conversation

inv-jishnu commented Jan 2, 2025 • edited Loading

Description

Related issues and/or PRs

Changes made

Checklist

Additional notes (optional)

Release notes

Torch3333 left a comment

Choose a reason for hiding this comment

komamitsu Feb 5, 2025

Choose a reason for hiding this comment

komamitsu Feb 5, 2025

Choose a reason for hiding this comment

inv-jishnu commented Feb 5, 2025

komamitsu left a comment

Choose a reason for hiding this comment

brfrn169 left a comment

Choose a reason for hiding this comment

brfrn169 Feb 5, 2025

Choose a reason for hiding this comment

brfrn169 Feb 5, 2025

Choose a reason for hiding this comment

inv-jishnu Feb 6, 2025

Choose a reason for hiding this comment

brfrn169 left a comment

Choose a reason for hiding this comment

brfrn169 Feb 6, 2025

Choose a reason for hiding this comment

inv-jishnu commented Jan 2, 2025 •

edited

Loading