Delete sql query not doing its job even though everything is correct

help
1

Hey, guys. Sorry to bother you all again, but I'm having an issue with my rust code at the moment.

I am making a wishlist program for myself at the moment, which can be used to turn a wishlist into something like this:

<!DOCTYPE html>
<html>
<head>
<title>Wishlist</title>
</head>
<body>
<h1>Wishlist</h1>
<hr>
<ol>
<li>Item 1</li>
<li>Item 2 <sup>[1]</sup></li>
<li>Item 3 <sup>[1]</sup><sup>[2]</sup></li>
</ol>
<h2>Notes</h2>
<hr>
<ol>
<li id="note1">Note 1</li>
<li id="note2">Note 2</li>
</ol>
</body>
</html>

Right now, I am having issues with dealing with note linking, in that I cannot say remove either note 1 or note 2 from either items two or three, even though my program creates the links just fine in the database backend, using rusqlite.

The code utilized to remove the linkis as follows:

pub fn remove_note_from_item(p: &str, item: &Item, note_id: u32) {
    if let Ok(db) = Connection::open(&real_path(p)) {
        let remove_link_statement = "DELETE FROM item_notes WHERE item_id = ?1 AND note_id = ?2";

        if let Ok(mut statement) = db.prepare(remove_link_statement) {
            if let Err(error) = statement.execute(params![item.id.clone(), note_id]) {
                println!("{}", error);
            }
        }
    }
}

If I were to run the query in SQLite directly, with the correct values, things work as they should, but not in the program itself.

As for where it the program this is called, it can be found here.

When I use the println! macro to step my way through things, it gets past the point where the statement is getting prepared and then is supposed to execute the query, but DB Browser shows no changes and I do not get any error reports.

Any ideas on how to fix this?

2

I don’t know rusqlite or your code base, but at a glance, I would suggest restructuring this code. It appears that you’re silently ignoring two errors (and one success). Since you’re stumped, it’s plausible that handling these error cases will surface whatever problem you’re encountering. Here’s my first impression of how I would restructure it:

pub fn remove_note_from_item(p: &str, item: &Item, note_id: u32) {
    let db = match Connection::open(&real_path(p)) {
        Ok(db) => db,
        Err(e) => {
            // Handle error and return early.
        }
    };

    let remove_link_statement = "DELETE FROM item_notes WHERE item_id = ?1 AND note_id = ?2";

    let mut statement = match db.prepare(remove_link_statement) {
        Ok(s) => s,
        Err(e) => {
            // Handle error and return early.
        }
    };

    // You might also restructure this to include logging success.
    if let Err(error) = statement.execute(params![item.id.clone(), note_id]) {
        println!("{}", error);
    }
}

There are a ton of ways to structure the code, and I’m quite tired at the moment, so I welcome someone else to improve it. If you fill in the error branches, though, you’ll at least have a lot more confidence about whether your program thinks it’s succeeding.

Also, I did fib above. I would really use either something like anyhow or an appropriate error type and just handle all those error cases with ?. That gets into a bigger discussion about how you handle want to errors in the scope of your particular project, though.

Good luck!

3

Thanks for the suggestions. I’ll try playing around with things for a bit and check on the documentation on things.

4

Alright, I looked at the documentation for the open function for Connection and that cannot fail, as a new database will be created if one does not exist at the path, so there is no reason to check for an error there, except for the appropriate permissions on the database.

However, when I use match to handle both errors and a success for executing, it will only return an unsigned integer type, as stated in the Statement, upon success, and the program claims to have succeeded by saying that one row was affected, thereby making error the only possible thing that would useful, though that does acknowledge that it ran.

With this, it might be useful for me to see about how it formed the query.

5

.expect or .unwrap is appropriate in such cases.

1 Like
6

Given that the DB is reporting one row affected, the SQL statement seems reasonable, and you’re finding that the DB isn’t actually changing, I have to wonder whether your code is opening and modifying a different database than the one you’re expecting. If SQLite says it deleted one row, I’m certainly inclined to believe it.

I’m signing off for the day, FYI. Best of luck!

7

That's a good line of thinking to check that, though I doubt it because this is an application using the clap crate and I did specify a default value and that value is specified in each subcommand, which all work except for this instance.

In cases where I had troubles before, it would end up creating databases in the wrong location until I used a custom function to translate it correctly, and that is still the case here it looks like, since I told the function to print out the directory, using that function, which is real_path.

8

Looks like I found my issue.

For whatever reason, even though it does not make sense, the program did not like having the ability to append or remove existing notes when updating data, as I moved the code to a block that only allowed the item id, note id, and a flag to determine if the note is to be added or removed were allowed and everything worked as expected.

Thanks for the tips you all provided.