0

What's wrong with my code, i tried to combine two query into one. But the second query is not working, i already follow the answer of this link INSERT INTO two tables at one query but i think mine doesn't work, am i missing something in my code?

string sql = "INSERT INTO tbladdbook(fBookTitle,fAuthor,fBookYr,fEdition,fPublication,fAccNo,fCallNo,fCategory,fBarCodeNo,fCurrentCopies) VALUES('"
                                    + txtTITLE.Text + "','"
                                    + txTAUTHOR.Text + "','"
                                    + txtBOOKYR.Text + "','"
                                    + txtEDITION.Text + "','"
                                    + txtPUBLICATION.Text + "','"
                                    + txtACCESSNO.Text + "','"
                                    + txtCALLNO.Text + "','"
                                    + txtCATEGORY.SelectedItem + "','"
                                    + txtBARCODE.Text + "','"
                                    + txtCOPIES.Text + "'); INSERT INTO tbltruecopies(fBookTitle,fAuthor,fBarCodeNo,fTrueCopies) VALUES('"
                                    + txtTITLE.Text + "','"
                                    + txTAUTHOR.Text + "','"
                                    + txtBARCODE.Text + "','"
                                    + txtCOPIES.Text + "')";

                            cfgotcall.inputQ(sql);

Table definition: for tbladdbook

fBookTitle   varchar
fAuthor      varchar
fEdition     varchar
fBookYr      varchar
fPublication varchar
fAccNo       varchar
fCallNo      varchar
fCategory    varchar
fBarCodeNo   varchar
fCurrentCopies  float

Table definition: for tbltrue

fBookTitle  varchar
fAuthor     varchar
fBarCodeNo  bigint
fTrueCopies bigint

Old and working code:

string sql = "INSERT INTO tbladdbook(fBookTitle,fAuthor,fBookYr,fEdition,fPublication,fAccNo,fCallNo,fCategory,fBarCodeNo,fCurrentCopies) VALUES('"
                                        + txtTITLE.Text + "','"
                                        + txTAUTHOR.Text + "','"
                                        + txtBOOKYR.Text + "','"
                                        + txtEDITION.Text + "','"
                                        + txtPUBLICATION.Text + "','"
                                        + txtACCESSNO.Text + "','"
                                        + txtCALLNO.Text + "','"
                                        + txtCATEGORY.SelectedItem + "','"
                                        + txtBARCODE.Text + "','"
                                        + txtCOPIES.Text + "')";

                                cfgotcall.inputQ(sql);

                                sql = "INSERT INTO tbltruecopies(fBookTitle,fAuthor,fBarCodeNo,fTrueCopies) VALUES('"
                                        + txtTITLE.Text + "','"
                                        + txTAUTHOR.Text + "','"
                                        + txtBARCODE.Text + "','"
                                        + txtCOPIES.Text + "')";

                                cfgotcall.inputQ(sql);
Community
  • 1
  • 1
  • Do you really have ellipsis `...` in your `INSERT` statements? Those are just placeholders. You need to specify actual column names there. Or, you could omit that completely if you provide values for all columns. – Tim Biegeleisen May 05 '17 at 03:32
  • it actually has actual column, i just put ellipsis so it will be shorter to and easy to read – Captain Teemo May 05 '17 at 03:34
  • 1
    I suspect the column type is not all `varchar`, seeing there is input like `CallNo` or `Copies` and yet you use aposthrope for all. – Ian May 05 '17 at 03:36
  • I'm not a .NET person, but you should try to catch the exception, if there be one. This would be way more informative than someone on SO trying to take shots in the dark. – Tim Biegeleisen May 05 '17 at 03:36
  • @Ian But he's calling `.Text` on those variables. Presumably that would return a string/text type. – Tim Biegeleisen May 05 '17 at 03:37
  • 1
    @TimBiegeleisen the `Text` comes from `.Net` GUI I suppose, not on the database table. Thus I called the data type `varchar` since I am referring to the database (not `string`). It's ok for the OP to use `string` (`.Text`) in this case since it will be used as query string, not to be use as numerical calculation in the `.Net` program. – Ian May 05 '17 at 03:37
  • Yes, perfectly possible, and wouldn't surprise me. Too bad we don't have the stack trace :-( – Tim Biegeleisen May 05 '17 at 03:38
  • @CaptainTeemo Can you post your table definitions? – Tim Biegeleisen May 05 '17 at 03:40
  • Can you capture the sql string and run directly against the DB with SSMS? – Kevin Raffay May 05 '17 at 03:40
  • @TimBiegeleisen there you go i just edit it – Captain Teemo May 05 '17 at 03:47
  • Ian appears to be correct, you're trying to insert string data into a number column in MySQL. Stop doing that. – Tim Biegeleisen May 05 '17 at 03:47
  • @TimBiegeleisen they are just working fine, i join them because if the first sql is correct and the second sql has error in it, the first sql will not be inserted in the database – Captain Teemo May 05 '17 at 03:48
  • If everything is working fine, then why did you ask this question in the first place? Can you share a stack trace with us, assuming an exception happened? – Tim Biegeleisen May 05 '17 at 03:50
  • Your query is prone to SQL injection if your text input is not filtered anyway, try using named parameters instead. Also you're trying to insert `fBarCodeNo` & `fTrueCopies` (and potentially other non-string fields) with string value directly from textboxes, convert them to respective formats or remove single quotes between them. – Tetsuya Yamamoto May 05 '17 at 03:51
  • @Ian i saw a post, http://stackoverflow.com/questions/9282988/insert-into-two-tables-at-one-query, running the two queries as one statement, i converted my old code into one statement, look at my old code at post that old code is working smoothly – Captain Teemo May 05 '17 at 03:54
  • stack trace `ERROR [42000] [MySQL][ODBC 5.3(w) Driver][mysqld-5.5.5-10.0.17-MariaDB]You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'INSERT INTO tbltruecopies(fBookTitle,fAuthor,fBarCodeNo,fTrueCopies) VALUES('4',' at line 1` – Captain Teemo May 05 '17 at 03:58
  • I really despise all of these inline variables that should be parameters – Mad Myche May 05 '17 at 03:59
  • @MadMyche yeah need to work on that, im reading now `parameterized queries`, can you produce sql injection on my code? – Captain Teemo May 05 '17 at 04:01
  • Where `tbltruecopies` syntax belongs to? I only see `tbltrue` there, provide the insert statement for it if exists. I strongly recommend you to wrap all insert queries into a stored procedure at once with named parameters (parameterized queries) to eliminate such syntax error hassles. – Tetsuya Yamamoto May 05 '17 at 04:04
  • @TetsuyaYamamoto - I have added an answer per se for the OP comment asking for this to be parameterized, and have added in a SP to wrap this as you have suggested be done – Mad Myche May 05 '17 at 15:10

1 Answers1

0

Captain Teemo asked if I was able to rewrite using parameters.

This is a relatively easy operation; however, I write for SQL Server and there may be slight differences with the MySql commands and I do not the cfgotcall methods (is this Cold Fusion?) that are being used for the Data Layer, so I will just write this in ADO.

In this case I simply replaced all of the values in the VALUES clause with SQL Variables, and basically reused the column names with a preceding @, so the column fBookTitle is assigned the value @fBookTitle. Then we can assign those parameters to the command object via the Parameters.AddWithValue() method. For the above @fBookTitle value the call would be cmd.Parameters.AddWithValue("@Title", txtTITLE.Text); I noticed that the variables being used in the second query were all in the first query, but not vice versa; so I am going to build up execute Qry2 first, then we can simply change the CommandText and add in the additional parameters. One of the things with using parameters is that you will need to add the values with the correct type, so adding in a value that is a BigInt in the database will need to be added in as the corresponding C# type of Int64.

What I can do is show how this would be done via ADO with SQL Server, and you can modify what needs to be done. If you can't find a cfgotcall that works with parameters then you could just change this for use with MySql which has nearly identical syntax to the SQL Server syntax.

string Qry1 = "INSERT INTO tbladdbook(fBookTitle,fAuthor,fBookYr,fEdition,fPublication,fAccNo,fCallNo,fCategory,fBarCodeNo,fCurrentCopies) VALUES (@Title, @Author, @BookYr, @Edition, @Publication, @AccNo, @CallNo, @Category, @BarCode, @Copies)";
string Qry2 = "INSERT INTO tbltruecopies(fBookTitle, fAuthor, fBarCodeNo, fTrueCopies) VALUES (@Title, @Author, @Barcode, @Copies)";

using (SqlConnection conn = new SqlConnection(connectionstring)) {
    conn.Open();
    using (SqlCommand cmd = new SqlCommand()) {
        cmd.Connection = conn;
        cmd.CommandType = CommandType.Text;

        cmd.CommandText = Qry2;
        cmd.Parameters.AddWithValue("@Title", txtTITLE.Text);
        cmd.Parameters.AddWithValue("@Author", txTAUTHOR.Text);
        cmd.Parameters.AddWithValue("@Barcode", Int64.parse(txtBARCODE.Text));
        cmd.Parameters.AddWithValue("@Copies", Int64.parse(txtCOPIES.Text));
        try { cmd.ExecuteNonQuery(); }
        catch (Exception) { /* your error handling */ }

        cmd.CommandText = Qry1;
        cmd.Parameters.AddWithValue("@BookYr", txtBOOKYR.Text);
        cmd.Parameters.AddWithValue("@Edition", txtEDITION.Text);
        cmd.Parameters.AddWithValue("@Publication", txtPUBLICATION.Text);
        cmd.Parameters.AddWithValue("@AccNo", txtACCESSNO.Text);
        cmd.Parameters.AddWithValue("@CallNo", txtCALLNO.Text);
        cmd.Parameters.AddWithValue("@Category", txtCATEGORY.SelectedItem);
        try { cmd.ExecuteNonQuery(); }
        catch (Exception) { /* your error handling */ }

    }
    conn.Close();
}

Tetsuya Yamamoto suggests that this be converted over to a Stored Procedure

This is an easy enough task on SQL Server, but I do not know the MySql implementation; sorry you are going to get what I would be typing in Query Analyzer or SSMS, and this would most likely to be translated for MySql as well.

The syntax for this procedure is going to be rather simple, as all we are going to do is wrap the 2 queries within it.

CREATE PROCEDURE usp_addBookAndCopies (
    @Title       VARCHAR(100),
    @Author      VARCHAR(100),
    @BookYr      VARCHAR(100),
    @Edition     VARCHAR(100),
    @Publication VARCHAR(100),
    @AccNo       VARCHAR(100),
    @CallNo      VARCHAR(100),
    @Category    VARCHAR(100),
    @BarCode     BIGINT,
    @Copies      BIGINT
) AS
BEGIN

    INSERT tbladdbook ( fBookTitle,  fAuthor,  fBookYr,  fEdition,  fPublication, 
                       fAccNo,  fCallNo,  fCategory,  fBarCodeNo,  fCurrentCopies  )
    VALUES            (@Title, @Author, @BookYr, @Edition, @Publication,
                      @AccNo, @CallNo, @Category, @BarCode, @Copies  )

    INSERT tbltruecopies ( fBookTitle, fAuthor, fBarCodeNo, fTrueCopies)
    VALUES               (@Title, @Author, @Barcode, @Copies)

END
GO

Once we have the Stored Procedure created, we would need to modify the original code, removing the 2 INSERT queries and replace them with the one command calling the procedure. We would also change the command type to reflect that we are running a procedure instead of a text command.

// Not Needed: string Qry1 = "INSERT INTO tbladdbook..." 
// Not Needed: string Qry2 = "INSERT INTO tbltruecopies..." 
using (SqlConnection conn = new SqlConnection(connectionstring)) {
    conn.Open();
    using (SqlCommand cmd = new SqlCommand()) {
        cmd.Connection = conn;
        cmd.CommandType = CommandType.StoredProcedure; //  Changed
        cmd.CommandText = "usp_addBookAndCopies";      //  Changed

        cmd.Parameters.AddWithValue("@Title", txtTITLE.Text);
        cmd.Parameters.AddWithValue("@Author", txTAUTHOR.Text);
        cmd.Parameters.AddWithValue("@Barcode", Int64.parse(txtBARCODE.Text));
        cmd.Parameters.AddWithValue("@Copies", Int64.parse(txtCOPIES.Text));
        cmd.Parameters.AddWithValue("@BookYr", txtBOOKYR.Text);
        cmd.Parameters.AddWithValue("@Edition", txtEDITION.Text);
        cmd.Parameters.AddWithValue("@Publication", txtPUBLICATION.Text);
        cmd.Parameters.AddWithValue("@AccNo", txtACCESSNO.Text);
        cmd.Parameters.AddWithValue("@CallNo", txtCALLNO.Text);
        cmd.Parameters.AddWithValue("@Category", txtCATEGORY.SelectedItem);

        try { cmd.ExecuteNonQuery(); }
        catch (Exception) { /* your error handling */ }
    }
    conn.Close();
}

My Comments

Looking at the actual statements and the code this appears to be adding books to a library of sorts. There is a table of Books (tbladdbook) and another for Book Copies (tbltruecopies), and the only thing different between the two tables is that Copies would reflect how many copies are currently on hand, but the Count is of dissimilar types; one being Float and the other as BigInt. My opinion would be that these two should be of the same type, and I really don't think it is realistic that these values would ever exceed the capacity of a 32 bit integer, if not 16 bit. Not too mention that Float and Double are only approximations.

This a rather long answer, and my aging eyes may have a syntax error or two. Please forgive me and let me know of any errors or suggestions, I will be happy to update this for you.

Mad Myche
  • 1,075
  • 1
  • 7
  • 15