[CBRD-25771] Add -i option to DIAGDB for input file and update usage message #5743

InChiJun · 2024-12-27T06:20:09Z

http://jira.cubrid.org/browse/CBRD-25771

Purpose

해당 이슈는 -i 옵션을 인식할 수 있도록 구현한다.
-i 옵션은 다수의 클래스명을 file로 전달할 수 있게 한다.
-i 옵션에는 클래스명이 담긴 파일이 전달되어야 한다.
-i 옵션으로 전달된 파일을 인식할 수 있도록 한다.
-n 옵션과 -i 옵션을 동시에 실행시킬 수 없다.
usage message에 -i 옵션에 대한 설명도 추가한다.

Implementation

DIAGDB에서 -i 옵션을 사용할 수 있게 구현
- DIAG_INPUT_FILE_S = diagdb에서 -i 인식하는 변수
DIAGDB usage message에 -i 옵션 추가(영문, 한글)
diagdb -d 9 옵션 외에 다른 옵션에서 -i 기능 실행되지 않도록 구현
-n과 -i 옵션이 함께 실행되지 않도록 구현

Remarks

N/A

YeunjunLee · 2024-12-27T08:26:34Z

msg/en_US.utf8/utils.msg

@@ -839,7 +839,8 @@ valid options:\n\
                                6 dump disk bitmaps\n\
                                7 dump catalog\n\
                                8 dump log\n\
-                                9 dump heap\n
+                                9 dump heap\n\
+  -i, --input-class-file=FILE input FILE of table names\n


Suggested change

-i, --input-class-file=FILE input FILE of table names\n

-i, --input-class-file=FILE input FILE of table names\n

option에 대한 설명이 위에 -o와 -d의 설명과 indent가 맞지 않는 것 같습니다.

수정하였습니다.

YeunjunLee · 2024-12-27T08:26:48Z

msg/ko_KR.utf8/utils.msg

@@ -837,6 +837,7 @@ diagdb: 데이터베이스 정보 덤프\n\
                                7 카탈로그 덤프\n\
                                8 로그 덤프\n\
                                9 힙 덤프\n
+  -i, --input-class-file=FILE 테이블 이름이 기입된 파일\n\


Suggested change

-i, --input-class-file=FILE 테이블 이름이 기입된 파일\n\

-i, --input-class-file=FILE 테이블 이름이 기입된 파일\n\

option에 대한 설명이 위에 -o와 -d의 설명과 indent가 맞지 않는 것 같습니다.

수정하였습니다.

YeunjunLee · 2024-12-27T09:07:35Z

src/executables/utility.h

+#define DIAG_INPUT_FILE_S                      'i'
+#define DIAG_INPUT_FILE_L                      "input-file"


Suggested change

#define DIAG_INPUT_FILE_S 'i'

#define DIAG_INPUT_FILE_L "input-file"

#define DIAG_INPUT_FILE_S 'i'

#define DIAG_INPUT_FILE_L "input-file"

Indent

수정하였습니다.

YeunjunLee · 2024-12-27T09:26:14Z

src/executables/util_sa.c

if (diag != DIAGDUMP_HEAP && class_name != NULL) { goto print_diag_usage; }

해당 코드는 diag -d 9(Heap dump)외의 옵션에서 -n 옵션이 입력되었을 때, usage를 출력해주는 코드입니다.
아마, -i 옵션 또한 유사한 동작이 필요할 필요가 있다고 생각됩니다. 해당 상황이 고려되어야 한다면, 해당 위치의 코드 수정이 필요해보입니다.

수정하여 반영하였습니다.

… not specified

vimkim · 2024-12-31T02:10:20Z

src/executables/util_sa.c


-      if (class_name == NULL)
+      if (class_name == NULL && fname == NULL)
 	{
 	  fprintf (outfp, "\n*** DUMP OF ALL HEAPS ***\n");
 	  (void) file_tracker_dump_all_heap (thread_p, outfp, dump_records);
 	}
-      else
+      else if (class_name && fname)
+	{
+	  goto print_diag_usage;
+	}
+      else if (class_name != NULL)
 	{


잘 모르는 부분이라 리뷰의 의도는 아니고, 순수한 궁금증에 여쭤보고 싶은 게 있습니다.
혹시 class_name 은 NULL인데 fname 이 NULL 이 아닌 경우에, 이 로직을 타는 경우가 있을 수 있을까요?
만약 그렇다면, fname이 NULL이 아닌 경우는 에러 처리를 안해주어도 괜찮나요?
감사합니다.

말씀하신 의도를 제가 정확히 파악했는지 모르겠습니다만, class_name이 NULL이고 fname이 NULL이 아닌 경우는 input file에 대한 heap dump를 위한 로직이 실행됩니다. 해당 코드는 다른 브랜치로 PR 예정이기 때문에 현재 PR 코드에는 보이지 않습니다.
답변이 안 되셨다면 추가로 말씀해주시면 감사하겠습니다.
감사합니다.

아하 답변 감사드립니다. 이해되었습니다.

hornetmj · 2025-01-06T08:16:23Z

msg/en_US.utf8/utils.msg

@@ -839,7 +839,8 @@ valid options:\n\
                                6 dump disk bitmaps\n\
                                7 dump catalog\n\
                                8 dump log\n\
-                                9 dump heap\n
+                                9 dump heap\n\
+  -i, --input-class-file=FILE  input FILE of table names\n


내부 요구사항에 의해 추가하는 기능으로 히든 옵션으로 정리하시죠.

hornetmj · 2025-01-08T11:07:37Z

src/executables/util_sa.c

@@ -1556,6 +1556,7 @@ diagdb (UTIL_FUNCTION_ARG * arg)
  DIAGDUMP_TYPE diag;
  THREAD_ENTRY *thread_p;
  int error_code = NO_ERROR;
+  char *fname;


의미있는 변수명을 사용하는 것이 좋겠습니다.

ex) class_name_list_file

hornetmj · 2025-01-08T11:09:13Z

src/executables/util_sa.c

@@ -1599,6 +1603,11 @@ diagdb (UTIL_FUNCTION_ARG * arg)
      goto print_diag_usage;
    }

+  if (diag != DIAGDUMP_HEAP && fname != NULL)


class_name 검사와 합치는게 어떨까요?

if (diag != DIAGDUMP_HEAP && (class_name != NULL || fname != NULL))

hornetmj · 2025-01-08T11:13:35Z

src/executables/util_sa.c

 	{
 	  fprintf (outfp, "\n*** DUMP OF ALL HEAPS ***\n");
 	  (void) file_tracker_dump_all_heap (thread_p, outfp, dump_records);
 	}
-      else
+      else if (class_name && fname)


이 부분은 전달받은 class_name 또는 fname 정보를 통해 출력을 처리하는 부분입니다. 전달받은 인자에 대한 예외처리를 수행하는 코드가 들어가게 되면, 코드 가독성이 떨어지게 됩니다. 해당 검사는 위에서 if (diag != DIAGDUMP_HEAP && fname != NULL) 검사 다음에 바로 수행하는게 좋을 것 같습니다.

InChijun added 3 commits December 27, 2024 14:53

[CBRD-25771] Add usage message for the DIAGDB -i option

09bc405

[CBRD-25771] Feat: Implemented file recognition for the DIAGDB -i option

dda8dd9

[CBRD-25771] Handled conditional processing for the -n and -i options

e9c42e4

InChiJun self-assigned this Dec 27, 2024

InChiJun marked this pull request as draft December 27, 2024 06:20

InChiJun force-pushed the CBRD-25771 branch from c2f3acd to e9c42e4 Compare December 27, 2024 07:32

InChiJun requested review from hornetmj, H2SU and YeunjunLee December 27, 2024 07:49

InChiJun marked this pull request as ready for review December 27, 2024 07:50

InChiJun changed the title ~~[CBRD-25771] Add -i option to DIAGDB for file recognition and update usage message.~~ [CBRD-25771] Add -i option to DIAGDB for input file and update usage message Dec 27, 2024

YeunjunLee reviewed Dec 27, 2024

View reviewed changes

[CBRD-25571] Style: Adjust indentation of usage

981fd9f

YeunjunLee reviewed Dec 27, 2024

View reviewed changes

hornetmj marked this pull request as draft December 27, 2024 10:07

InChiJun marked this pull request as ready for review December 30, 2024 22:33

[CBRD-25771] Add usage message when fname exists but DIAGDUMP_HEAP is…

d335c27

… not specified

vimkim reviewed Dec 31, 2024

View reviewed changes

[CBRD-25771] Modify inednt

208d1b9

InChiJun requested review from YeunjunLee and vimkim January 3, 2025 11:13

hornetmj reviewed Jan 8, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CBRD-25771] Add -i option to DIAGDB for input file and update usage message #5743

[CBRD-25771] Add -i option to DIAGDB for input file and update usage message #5743

InChiJun commented Dec 27, 2024 •

edited

Loading

YeunjunLee Dec 27, 2024

InChiJun Dec 27, 2024

YeunjunLee Dec 27, 2024

InChiJun Dec 27, 2024

YeunjunLee Dec 27, 2024

InChiJun Dec 31, 2024

YeunjunLee Dec 27, 2024

InChiJun Dec 31, 2024

vimkim Dec 31, 2024

InChiJun Dec 31, 2024

vimkim Dec 31, 2024

hornetmj Jan 6, 2025

hornetmj Jan 8, 2025

hornetmj Jan 8, 2025

hornetmj Jan 8, 2025

	-i, --input-class-file=FILE input FILE of table names\n
	-i, --input-class-file=FILE input FILE of table names\n

	-i, --input-class-file=FILE 테이블 이름이 기입된 파일\n\
	-i, --input-class-file=FILE 테이블 이름이 기입된 파일\n\

		#define DIAG_INPUT_FILE_S 'i'
		#define DIAG_INPUT_FILE_L "input-file"

[CBRD-25771] Add -i option to DIAGDB for input file and update usage message #5743

Are you sure you want to change the base?

[CBRD-25771] Add -i option to DIAGDB for input file and update usage message #5743

Conversation

InChiJun commented Dec 27, 2024 • edited Loading

Purpose

Implementation

Remarks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InChiJun commented Dec 27, 2024 •

edited

Loading