Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CBRD-24620] Add new csql session command 'connect' #4134

Merged
merged 39 commits into from
Mar 21, 2023
Merged

[CBRD-24620] Add new csql session command 'connect' #4134

merged 39 commits into from
Mar 21, 2023

Conversation

eido5
Copy link
Contributor

@eido5 eido5 commented Feb 22, 2023

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

Description

The user typing that the below command to connect other users is not intuitive, while the CSQL session is connected;

CALL login ('user_name', 'password') ON CLASS db_user;
So, we should add the session command user to use more intuitive while the CSQL session is connected.

;connect user_name db_name@host_name

This can helps user to connect more intuitive while the CSQL session is connected

Implementation

  • Make new switch condition 'S_CMD_CONNECT:' In csql_do_session_cmd().
  • Parse the argument to connect user, db and host
  • Connect through db_restart_ex() function.
  • Print the 'Connected.' to ensure the connection.

Remarks

N/A

@eido5
Copy link
Contributor Author

eido5 commented Feb 22, 2023

I am fixing the process when the db_name, host_name, user_name are is NULL.
So, could you review except these case?

src/executables/csql.c Outdated Show resolved Hide resolved
msg/en_US/csql.msg Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
@kisoo-han
Copy link
Contributor

@eido5 ,
Please extend csql HELP for ';CONnect' session command.

char *user_name = NULL;
char *host_name = NULL;
char *p = NULL;
CSQL_ARGUMENT csql_new_arg;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about using 3 variables (db_name, user_name, passwd) instead of csql_new_arg?
How about directly assigning the three variables to csql_arg after login?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because csql_arg.user_name is const * char, so we must use same structure and then copy it.

I tried what you suggest but it didn't work.

src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
@kisoo-han
Copy link
Contributor

We need to completely hand over screen control to libedit for csql. If the database is offline, readline should be called by changing the prompt to '!csql> ' instead of outputting '!' and passing it to readline with prompt "csql> ".

Please refer following diff as an example:

diff --git a/src/executables/csql.c b/src/executables/csql.c
index 8dca6862f..a3e847013 100644
--- a/src/executables/csql.c
+++ b/src/executables/csql.c
@@ -161,6 +161,7 @@ char csql_Scratch_text[SCRATCH_TEXT_LEN];
 int csql_Error_code = NO_ERROR;

 static char csql_Prompt[100];
+static char csql_Prompt_offline[100];
 static char csql_Name[100];

 /*
@@ -575,12 +576,7 @@ start_csql (CSQL_ARGUMENT * csql_arg)

   for (line_no = 1;; line_no++)
     {
-      if (db_Connect_status != DB_CONNECTION_STATUS_CONNECTED)
-       {
-         csql_Database_connected = false;
-         fputs ("!", csql_Output_fp);
-       }
-
+      char *prompt = db_Connect_status == DB_CONNECTION_STATUS_CONNECTED ? csql_Prompt : csql_Prompt_offline;
       read_whole_line = false;

       memset (line_buf, 0, LINE_BUFFER_SIZE);
@@ -592,7 +588,7 @@ start_csql (CSQL_ARGUMENT * csql_arg)
          fputs (csql_Prompt, csql_Output_fp);  /* display prompt */
          line_read = fgets ((char *) line_buf, LINE_BUFFER_SIZE, csql_Input_fp);
 #else
-         if ((line_read = readline (csql_Prompt)) != NULL)
+         if ((line_read = readline (prompt)) != NULL)
            {
              if (line_read_alloced != NULL)
                {
@@ -2800,6 +2796,7 @@ csql (const char *argv0, CSQL_ARGUMENT * csql_arg)
     {
       strncat (csql_Prompt, " ", avail_size);
     }
+  snprintf (csql_Prompt_offline, sizeof (csql_Prompt_offline), "!%s", csql_Prompt);
   strncpy_bufsize (csql_Name, csql_get_message (CSQL_NAME));

   /* as we must use db_open_file_name() to open the input file, it is necessary to be opening csql_Input_fp at this

@eido5
Copy link
Contributor Author

eido5 commented Mar 20, 2023

SA mode handling.
Only using by username can login.

@eido5 eido5 requested a review from kisoo-han March 20, 2023 06:02
src/executables/csql.c Outdated Show resolved Hide resolved
src/executables/csql.c Outdated Show resolved Hide resolved
Copy link
Contributor

@kisoo-han kisoo-han left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now, everything is ok.
Thank you for your efforts!

@eido5 eido5 merged commit 84e9877 into CUBRID:develop Mar 21, 2023
eido5 added a commit that referenced this pull request May 1, 2023
)

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

Description

-The user typing that the below command to connect other users is not intuitive, while the CSQL session is connected;

-CALL login ('user_name', 'password') ON CLASS db_user;
So, we should add the session command user to use more intuitive while the CSQL session is connected.

-;connect user_name db_name@host_name
This can helps user to connect more intuitive while the CSQL session is connected.

Remarks

-This PR is to avoid possible memory leak of [CBRD-24620] Add new csql session command 'connect' #4134 pointed out by valgrind.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants