-1

I pass the following string to a method to perform an Insert, However each time I try to do this, I get an error.

PropInsert = "INSERT INTO Image_has_Props (Image_ImageID, Props_PropID), SELECT "+IntImageID+", PropID FROM Props WHERE PropDescription = '"+StrPropDescription+"';";

Error

You can see in the error, there appears to be an extra ' After where it says Blood Pressure Monitor. I have no idea where this extra ' is coming from. I get the StrPropDescription from a JComboBox.

StrPropDescription = propChoice.getSelectedItem().toString();
Peddler
  • 6,045
  • 4
  • 18
  • 22

5 Answers5

4

First, generating sql statements directly from user input is a dangerous thing!

Second, you might want to dump out the value of StrPropDescription right before the sql statement to ensure it has the value you think it should.

Likely, it will have an apostrophe at the end of it. Then it is a matter of tracing backwards from the source to see where/how that variable was changed.

Tommy Hui
  • 1,306
  • 6
  • 9
  • The user is actually selecting the value to input from a JComboBox so it's not too dangerous! Checked what was being printed and it is correct, no apostrophe at the end, – Peddler Mar 09 '12 at 21:44
  • Still worth sanitising your input, for that future case that always turns up where you've got "O'Malley Corporation Blood Pressure Monitor" in the list :) – Matt Gibson Mar 09 '12 at 21:48
  • @MattGibson Can't argue with that, this way just suits how I need to do it at the moment! – Peddler Mar 09 '12 at 23:31
4

I think you just have an extra comma. Try

PropInsert = "INSERT INTO Image_has_Props (Image_ImageID, Props_PropID) SELECT "+IntImageID+", PropID FROM Props WHERE PropDescription = '"+StrPropDescription+"';";

Note the lack of a comma between the INSERT and the SELECT.

Matt Gibson
  • 37,886
  • 9
  • 99
  • 128
1

The extra ' at the end is just the quote around the whole select syntax, matching the one before the SELECT. I think there is an error after SELECT, SELECT 1, PropID" Is 1 a column name?

FloatingCoder
  • 1,724
  • 1
  • 12
  • 18
1

A) That's not valid SQL. You have a , following (Image_ImageID, Props_PropID) - You also don't end the statement with a ; when using the JDBC.

B) You should be using prepared statements with placeholders rather than injecting raw user input into your SQL statement.

Brian Roach
  • 76,169
  • 12
  • 136
  • 161
  • The users are selecting value from a JComboBox which displays a list of values from another colum, no raw user input here. See posts below, and last line of my original question. – Peddler Mar 09 '12 at 21:57
  • 1
    Sounds like a great reason to ignore a best practice ;) Honestly, there's really no reason not to use them. – Brian Roach Mar 09 '12 at 22:55
  • Downvoter, care to comment? Or did I make someone all internet mad again? – Brian Roach Mar 13 '12 at 12:41
  • @Peddler - I didn't think you were ;) It happens ... I hurt people's widdle feewings sometimes and they go find a random post to downvote. – Brian Roach Mar 13 '12 at 13:07
1

I think the problem with this statement is that you have a SELECT 1 in your sql statement, when it's not a column name in your table, and also the , in between the INSERT and SELECT statements as other people on here have mentioned.

I'm not entirely sure about the purpose of your IntImageID variable in this context, but I'm guessing that you're trying to do one of two things.

1: You're trying to get the ImageID from the table, which is a column, in which case, you'd be wanting something like:

PropInsert = "INSERT INTO Image_has_Props (Image_ImageID, Props_PropID)
             SELECT ImageID, PropID FROM Props 
             WHERE PropDescription = '"+StrPropDescription+"'";

OR

2: You're trying to put IntImageID as the first insert value, and the second value is pulled from the database, in which case, it'd be something like the following:

PropInsert = "INSERT INTO Image_has_Props (Image_ImageID, Props_PropID)
             ("+IntImageID+", SELECT ImageID, PropID FROM Props 
             WHERE PropDescription = '"+StrPropDescription+"')";

I'm not really entirely sure if I wrote the second one correctly since I can't test it, but basically, it involves having your IntImageID variable separate from your SELECT statement, if it's not in the database table.

MysticXG
  • 1,437
  • 10
  • 10