1

I'm trying to loop through multiple sql queries that are executed. I want to first get all the question information for a certain task and then get the keywords for that question. I have three records in my Questions table, but when the while loop at the end of list.add(keyword); is done, it jumps to the SELECT Questions.Question loop (as it should) and then just jumps out and gives me only one record and not the other 2.

What am I doing wrong? Can someone maybe help me fix my code? I've thought of doing batch sql executes (maybe that is the solution), but within each while loop, I need information from the previous sql statement, so I can't just do it all at the end of the batch.

SQL Code:

String TaskTopic = eElement.getElementsByTagName("TaskTopic").item(0).getTextContent();

// perform query on database and retrieve results
String sql = "SELECT Tasks.TaskNo FROM Tasks WHERE Tasks.TaskTopic = '" + TaskTopic + "';";
System.out.println("   Performing query, sql = " + sql);
result = stmt.executeQuery(sql);
Document doc2 = x.createDoc();
Element feedback = doc2.createElement("Results");

while (result.next())
{
    String TaskNo = result.getString("TaskNo");
    // perform query on database and retrieve results

    String sqlquery = "SELECT Questions.Question, Questions.Answer, Questions.AverageRating, Questions.AverageRating\n" +
              "FROM Questions\n" +
             "INNER JOIN TaskQuestions ON TaskQuestions.QuestionID = Questions.QuestionID \n" +
             "INNER JOIN Tasks ON Tasks.TaskNo = '" + TaskNo + "';";                                  
    result = stmt.executeQuery(sqlquery);

    while (result.next())
    {
        String Question = result.getString("Question");
        String Answer = result.getString("Answer");
        String AverageRating = result.getString("AverageRating");                                 
        String sqlID = "SELECT QuestionID FROM Questions WHERE Question = '" + Question + "';";
        result = stmt.executeQuery(sqlID);      

        while (result.next())
        {
            String ID = result.getString("QuestionID");
            String sqlKeywords = "SELECT Keyword FROM LinkedTo WHERE QuestionID = '" + ID + "';";
            result = stmt.executeQuery(sqlKeywords);

            while (result.next())
            {
                String keyword = result.getString("Keyword");
                list.add(keyword);
            }
       }
       feedback.appendChild(x.CreateQuestionKeyword(doc2, Question, Answer, AverageRating, list));
    }
}
4
  • 1
    Seems like it'd be much more efficient to build out a long query rather than loop in order to reduce round trips. Commented Jul 9, 2014 at 20:47
  • And the code you posted is wide open to sql injection. You should not take user input and then directly execute that against your database. I don't think you need a long query here, you just need to know what you want. Get all your data in one query and then loop through those results instead. Commented Jul 9, 2014 at 20:49
  • The only problem with that is that I need the results from the previous SQL statement to get the following results. If I combine the SQL statement into one, I'm going to get as many questions as I have keywords and I want one question and it's subsequent keywords as a list. Commented Jul 9, 2014 at 20:52
  • If I put it all into one query, I'm getting multiples of my question (for each of their respective keywords) and not a single question and their respective multiple keywords as I would like it to be. Do you have a suggestion as to how I can structure my SQL statement to get this? Commented Jul 9, 2014 at 20:57

1 Answer 1

1

Why this should be done in SQL

Creating loops is exponentially less efficient than writing a sql query. Sql is built to pull back this type of data and can plan out how it is going to get this data from the database (called an execution plan).

Allowing Sql to do its job and determine the best way to pull back the data instead of explicitly determining what tables you are going to use first and then calling them one at a time is better in terms of the amount of resources you will use, how much time it will take to get the results, code readability, and maintainability in the future.

What information you are looking for

In the psuedocode you provided, you are using the Keyword, Question, Answer, and AnswerRating values. Finding these values should be the focus of the sql query. Based on the code you have written, Question, Answer, and AnswerRating are coming from the Questions table and Keyword is coming from the LinkedTo table, so both of these tables should be available to have data pulled from them.

You can note at this point that we have essentially just mapped out what the Select and From portions of your query should look like.

It also looks like you have a parameter called TaskTopic so we need to include the table Tasks to make sure the correct data is returned. Lastly, the TaskQuestions table is the link between the tasks and the questions. Now that we know what the query should look like, let's see what the results are using sql syntax.

The Code

You did not include the declaration of stmt, but I assume that it is a PreparedStatement. You can add parameters to a prepared statement. Notice the ? in the sql code? The parameters you provide will be added in place of the ?. To do this, you should use stmt.setString(1, TaskTopic);. Note that if there were more than one parameter, you would need to add them in the order that they exists in the sql query (using 1, 2, ...)

SELECT  l.Keyword,
        q.Question,
        q.Answer,
        q.AverageRating
FROM LinkedTo l Inner Join
     Questions q
  on l.questionID = q.QuestionID
Where exists (  Select  1
                From TaskQuestions tq INNER JOIN 
                     Tasks t
                  on tq.TaskNo = t.TaskNo
                Where t.TaskTopic = ?
                and tq.QuestionID = q.QuestionID)

This is one way that you can write the query to return the same results. There are other ways to write this to get what you are looking for.

What's Going On?

There are a few things in this query you may not be familiar with. First are table aliases. Instead of writing the table name over and over again, you can alias your tables. I used the letter q to represent the Questions table. Any time you see q. you should recognize that I am referring to a column from Questions. The q after Questions is what gives the table its alias.

Exists Instead of doing a bunch of inner joins with tables that you are not selecting information from, you can use an exists to check if what you are looking for is in those tables. You can continue to do inner joins if you need data from the tables, but if you don't, Exists is more efficient.

I suspect you had issues with the query before (and probably the one you provided) because you did not provide any information to join TaskQuestions and Tasks together. That most likely resulted in the duplicates. I joined on TaskNo but this may not be the correct column depending on how the tables are set up.

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.