0

I tried so many things but nothing fixes it, as any of the INSERT Command or DELETE Command in MySQL Database are working fine, only UPDATE Command swaps the first and second column unnecessarily, when it's displayed using the SELECT Command. So, I need to close my app. and then re-Run it, then the columns are then displayed correctly, only until I execute UPDATE Command again, which messes with the first and second column.

Any idea why this is happening!

Here is my code

Sub DB_CMD_FUNC(SENDER As String)
        'Database realted script
        Dim con As New MySqlConnection("server=localhost; user=root; password=****; database=****;")
        Dim cmd As New MySqlCommand
        Dim dt As New DataTable
        Dim da As New MySqlDataAdapter
        Dim sql As String
        Dim DR As MySqlDataReader
        Dim SQL_CMD_TXT = "UPDATE `employees` SET `NAME`= '" & EMPLOYEE_EDIT_FRM.EDIT_NAME_TXT.Text & "', _
`FATHER_NAME`='" & EMPLOYEE_EDIT_FRM.EDIT_FATHER_NAME_TXT.Text & "', `DOB`='" &
        EMPLOYEE_EDIT_FRM.EDIT_DOB_TXT.Text & "',`DOJ`='" &
        EMPLOYEE_EDIT_FRM.EDIT_DOJ_TXT.Text & "',`POSITION`='" &
        EMPLOYEE_EDIT_FRM.EDIT_POSITION_TXT.Text &
        "',`EMP_STATUS`='" & EMPLOYEE_EDIT_FRM.EDIT_EMP_STATUS_TXT.Text & "', _ 
`SALARY`='" & EMPLOYEE_EDIT_FRM.EDIT_SALARY_TXT.Text & "',`EOS`='" &
    EMPLOYEE_EDIT_FRM.EDIT_EOS_TXT.Text & "', _
`MOBILE`='" & EMPLOYEE_EDIT_FRM.EDIT_REMARKS_TXT.Text & "' WHERE 
    `EMPLOYEE_ID` = '" & EMPLOYEE_EDIT_FRM.EDIT_ID_TXT.Text & "'; 
Select `NAME` FROM `employees` WHERE 
    `EMPLOYEE_ID`='" & EMPLOYEE_EDIT_FRM.EDIT_ID_TXT.Text & "';"
        Try
            'DB CMD EXECUTION
            con.Open()
            With cmd
                sql = SQL_CMD_TXT
                .Connection = con
                .CommandText = sql
            End With
            dt.Clear()
            da.SelectCommand = cmd
            da.Fill(dt)
            'Command for datagridview object
            With OBJECT_DATAGRIDVIEW
                .DataSource = dt
                'Scroll to the last row.
                .Name = "MYDATAGRIDVIEW"
                .FirstDisplayedScrollingRowIndex = .RowCount - 1
            End With
            con.Close()
        Catch ex As Exception
            MessageBox.Show("ERROR FOR SQL CMD EXECUTION SECTION-" & ex.Message,
                                      "SQL CMD EXECUTION", MessageBoxButtons.OK, MessageBoxIcon.Error)
            Exit Sub
        End Try
    End Sub
jmcilhinney
  • 50,448
  • 5
  • 26
  • 46
oliva nikita
  • 76
  • 1
  • 10
  • 1
    The question appears shows two problems, one explicit and the other implied. First, *never* concatenate parameter values into the string - use SQL parameters for the parameter values. Second, do not use `*` to select data - always specify the column names in the order that you want them to be returned to force it to do what you want. – Andrew Morton May 31 '20 at 16:01
  • i also tried instead of '*' specifying all fields one by one but still, the same thing happens first and second column swaps with each other even I tried assigning them name in datagridview with each column but still the same thing happens – oliva nikita May 31 '20 at 16:54

1 Answers1

3

Your UPDATE-query seems to be structurally fine. But please do use SQL parameters, because if you now include an apostrophe (') within your entered textbox values, the query will most likely crash. (Or worse: some malicious user might apply SQL injection to manipulate your database!)

But after the UPDATE-query, you execute a small SELECT query. This SELECT query returns only the NAME of only the updated record. This single row with a single column is captured in a DataTable object, and that DataTable object is set as the new data source for your DataGridView.

Is that your real intention? In your place, I would probably want to select all relevant fields from the database table and include all records, so that everything will be shown/refreshed in the DataGridView...

You probably already have some SELECT query in your application's startup code to initially fill the DataGridView. I would personally put that code in a separate subroutine (called something like FillDataGridView) and call that subroutine in the application's (or the form's) startup code and also call it after executing the UPDATE query. So there would not be any need to include a separate SELECT query after your UPDATE query.

You could check out the following code for some inspiration. Note that I haven't tested the code (because I do not have MySQL installed on my machine), so it might quite certainly require some additional tweaking and bugfixing inside your application's code.

Private Sub DB_CMD_FUNC(SENDER As String)
    Dim SQL_CMD_TXT As New StringBuilder()

    SQL_CMD_TXT.AppendLine("UPDATE `employees`")
    SQL_CMD_TXT.AppendLine("SET")
    SQL_CMD_TXT.AppendLine("  `NAME` = @name,")
    SQL_CMD_TXT.AppendLine("  `FATHER_NAME` = @father_name,")
    SQL_CMD_TXT.AppendLine("  `DOB` = @dob,")
    SQL_CMD_TXT.AppendLine("  `DOJ` = @doj,")
    SQL_CMD_TXT.AppendLine("  `POSITION` = @position,")
    SQL_CMD_TXT.AppendLine("  `EMP_STATUS` = @status,")
    SQL_CMD_TXT.AppendLine("  `SALARY` = @salary,")
    SQL_CMD_TXT.AppendLine("  `EOS` = @eos,")
    SQL_CMD_TXT.AppendLine("  `MOBILE` = @mobile'")
    SQL_CMD_TXT.AppendLine("WHERE")
    SQL_CMD_TXT.AppendLine("  `EMPLOYEE_ID` = @ID;")

    Try
        Dim ID As String = EMPLOYEE_EDIT_FRM.EDIT_ID_TXT.Text

        If String.IsNullOrWhiteSpace(ID) Then
            Throw New Exception("The ID is required.")
        End If

        Dim NAME As String = EMPLOYEE_EDIT_FRM.EDIT_NAME_TXT.Text

        If String.IsNullOrWhiteSpace(NAME) Then
            NAME = Nothing
        End If

        Dim FATHER_NAME As String = EMPLOYEE_EDIT_FRM.EDIT_FATHER_NAME_TXT.Text

        If String.IsNullOrWhiteSpace(FATHER_NAME) Then
            FATHER_NAME = Nothing
        End If

        Dim DD As Date
        Dim DOB As Date?
        Dim DOJ As Date?

        If Date.TryParse(EMPLOYEE_EDIT_FRM.EDIT_DOB_TXT.Text, DD) Then
            DOB = DD
        Else
            DOB = Nothing
        End If

        If Not Date.TryParse(EMPLOYEE_EDIT_FRM.EDIT_DOJ_TXT.Text, DD) Then
            DOJ = DD
        Else
            DOJ = Nothing
        End If

        Dim POSITION As String = EMPLOYEE_EDIT_FRM.EDIT_POSITION_TXT.Text

        If String.IsNullOrWhiteSpace(POSITION) Then
            POSITION = Nothing
        End If

        Dim STATUS As String = EMPLOYEE_EDIT_FRM.EDIT_EMP_STATUS_TXT.Text

        If String.IsNullOrWhiteSpace(STATUS) Then
            STATUS = Nothing
        End If

        Dim DEC As Decimal
        Dim SALARY As Decimal?

        If Decimal.TryParse(EMPLOYEE_EDIT_FRM.EDIT_SALARY_TXT.Text, DEC) Then
            SALARY = DEC
        Else
            SALARY = Nothing
        End If

        Dim EOS As Date?

        If Date.TryParse(EMPLOYEE_EDIT_FRM.EDIT_EOS_TXT.Text, DD) Then
            EOS = DD
        Else
            EOS = Nothing
        End If

        Dim MOBILE As String = EMPLOYEE_EDIT_FRM.EDIT_REMARKS_TXT.Text

        If String.IsNullOrWhiteSpace(MOBILE) Then
            MOBILE = Nothing
        End If

        Using con As New MySqlConnection("server=localhost; user=root; password=****; database=****;")
            ''Do not open the database connection just yet...
            'con.Open()

            Using cmd As New MySqlCommand(SQL_CMD_TXT.ToString(), con)
                cmd.Parameters.Add("@id", MySqlDbType.VarChar).Value = ID
                cmd.Parameters.Add("@name", MySqlDbType.VarChar).Value = NAME
                cmd.Parameters.Add("@father_name", MySqlDbType.VarChar).Value = FATHER_NAME
                cmd.Parameters.Add("@dob", MySqlDbType.Date).Value = DOB
                cmd.Parameters.Add("@doj", MySqlDbType.Date).Value = DOJ
                cmd.Parameters.Add("@position", MySqlDbType.VarChar).Value = POSITION
                cmd.Parameters.Add("@status", MySqlDbType.VarChar).Value = STATUS
                cmd.Parameters.Add("@salary", MySqlDbType.Decimal).Value = SALARY
                cmd.Parameters.Add("@eos", MySqlDbType.Date).Value = EOS
                cmd.Parameters.Add("@mobile", MySqlDbType.VarChar).Value = MOBILE

                con.Open() 'Delayed opening the database connection until the very last moment.
                cmd.ExecuteNonQuery()
            End Using

            ''Explicitly closing the database connection is not necessary, because the disposal (triggered when the Using-block is ended) takes care of that.
            'con.Close()
        End Using
    Catch ex As Exception
        MessageBox.Show($"ERROR FOR SQL CMD EXECUTION SECTION - {ex.Message}",
                        "SQL CMD EXECUTION",
                        MessageBoxButtons.OK,
                        MessageBoxIcon.Error)
        Exit Sub
    End Try

    FillDataGridView()
End Sub

Private Sub FillDataGridView()
    Dim SQL_CMD_TXT As New StringBuilder()

    SQL_CMD_TXT.AppendLine("SELECT")
    SQL_CMD_TXT.AppendLine("  `ID`,")
    SQL_CMD_TXT.AppendLine("  `NAME`,")
    SQL_CMD_TXT.AppendLine("  `FATHER_NAME`,")
    SQL_CMD_TXT.AppendLine("  `DOB`,")
    SQL_CMD_TXT.AppendLine("  `DOJ`,")
    SQL_CMD_TXT.AppendLine("  `POSITION`,")
    SQL_CMD_TXT.AppendLine("  `EMP_STATUS`,")
    SQL_CMD_TXT.AppendLine("  `SALARY`,")
    SQL_CMD_TXT.AppendLine("  `EOS`,")
    SQL_CMD_TXT.AppendLine("  `MOBILE`")
    SQL_CMD_TXT.AppendLine("FROM")
    SQL_CMD_TXT.AppendLine("  `employees`;")

    Try
        Dim dt As New DataTable()

        Using con As New MySqlConnection("server=localhost; user=root; password=****; database=****;")
            ''Do not open the database connection just yet...
            'con.Open()

            Using cmd As New MySqlCommand(SQL_CMD_TXT.ToString(), con)
                Using da As New MySqlDataAdapter(cmd)
                    con.Open() 'Delayed opening the database connection until the very last moment.
                    da.Fill(dt)
                End Using
            End Using

            ''Explicitly closing the database connection is not necessary, because the disposal (triggered when the Using-block is ended) takes care of that.
            'con.Close()
        End Using

        With OBJECT_DATAGRIDVIEW
            .DataSource = dt
            .FirstDisplayedScrollingRowIndex = .RowCount - 1 'Scroll to the last row.
        End With
    Catch ex As Exception
        MessageBox.Show($"ERROR FOR SQL CMD EXECUTION SECTION - {ex.Message}",
                        "SQL CMD EXECUTION",
                        MessageBoxButtons.OK,
                        MessageBoxIcon.Error)
    End Try
End Sub
Bart Hofland
  • 3,700
  • 1
  • 13
  • 22
  • What is the point of the StringBuilder? Why not just create the whole string at once? – Mary Jun 02 '20 at 18:21
  • There is no need to open the connection until directly before the .Execute.... It is not necessary to close the connection when it is included in the Using block. – Mary Jun 02 '20 at 18:25
  • Hint: You can combine the connection and command in the same using block by putting a comma at the end of the first line of the Using and on the next line `cmd As New MySqlCommand(SQL_CMD_TXT.ToString(), con)`. It just saves some indenting and makes the code at bit easier to read. – Mary Jun 02 '20 at 18:36
  • @Mary Ah yes. I know I could do that. But IMHO its less easier to read. It would make the lines longer, and I don't like long lines of code. It would be possible to split it over multiple lines, but then the transition between the opening line of the Using-block and the first line of the code inside the Using-block would be more awkward. Splitting statements over multiple lines in VB is generally not so nice. So altogether (mainly for these kind of code formatting quirks and the difference in code compactness) I actually prefer C# over VB.NET, even after years of programming in VB.NET. – Bart Hofland Jun 03 '20 at 05:11
  • @Mary By the way, thanks for the compliments regarding the parameters and the Using-blocks. Regarding the explicit opening and closing of the connection, opinions generally differ. Some would say it's better to explicitly close everything before disposal, but because disposal typically takes care of closing, it's not really necessary here. I personally would omit the closing statement, like you suggest, but I included it for more clarity here. – Bart Hofland Jun 03 '20 at 05:17
  • @Mary Regarding the StringBuilder... You are right. This query is just a static string and could be assigned as such. But I like formatted queries (with newlines and indenting) in case I need to use an SQL profiler or if I want to copy and paste the resulting query in a SQL client application for further analysis and tweaking. So again here, using concatenated strings with would make the code less readable IMHO. And using a StringBuilder would also make my code structure generally somewhat more consistent (for example in case I would need to dynamically create a SQL query in the next routine). – Bart Hofland Jun 03 '20 at 05:23
  • @Mary Delaying opening the database connection until just the moment where it is needed is generally a very good idea. I will fix that in my answer. – Bart Hofland Jun 03 '20 at 05:39
  • Re the StringBuilder. You can put new lines in strings, tab or space to format as your wish. See my recent answer of an Update sql string. https://stackoverflow.com/a/62144299/8367626 – Mary Jun 03 '20 at 08:21
  • 1
    @Mary Yes, all string literals in VB.NET are also implicitly *verbatim string literals*. But when I would use that, I still would have to make indenting concessions: either nice indenting in code and terrible indenting in the string values, or nice indenting in the string values and terrible indenting in the code. So I personally still prefer the StringBuilder, despite the heavier code and the small performance overhead. Thank you very much, by the way. I really appreciate your thoroughness in your comments and I do value these kinds of discussions. :) – Bart Hofland Jun 03 '20 at 09:24