SQL Error 21 'Library used incorrectly'

[teal epoch]

Hi, I am using SQLite3 to interact with my SQLite database in C++. I have written this function to make select from queries into my database:

std::vector<std::map<std::string, DatabaseValue>> DatabaseManager::queryData(
    const std::string& tables,
    const std::string &selection,
    const std::string &where = "",
    const std::string &other = ""
) const {
    const std::string sql = "SELECT " + selection + " FROM " + tables + (where.empty() ? "" : " WHERE " + where) + " " + other + ";";
    sqlite3_stmt *stmt;
    std::cerr << "\n'" << sql << "'\n";
    std::vector<std::map<std::string, DatabaseValue>> results;

    // std::cerr << sqlite3_prepare_v2(database, sql.c_str(), -1, &stmt, nullptr);

    if (sqlite3_prepare_v2(database, sql.c_str(), -1, &stmt, nullptr) != SQLITE_OK) {
        std::cerr << "Error preparing statement" << std::endl;
        return results;
    }

    const int cols = sqlite3_column_count(stmt);
    while (sqlite3_step(stmt) == SQLITE_ROW) {
        std::map<std::string, DatabaseValue> row;

        for (int col = 0; col < cols; col++) {
            const char *colName = sqlite3_column_name(stmt, col);
            const auto colValue = new DatabaseValue(stmt, col);

            row[colName] = *colValue;
        }

        results.push_back(row);
    }

    sqlite3_finalize(stmt);
    return results;
}

This function works, for example here in this other class of mine:

LaunchManager::LaunchManager(DatabaseManager db) : db(&db) {
    for (const auto& file: std::filesystem::recursive_directory_iterator(std::filesystem::current_path() / "archive")) {
        if(file.is_directory()) continue;
        if(file.path().extension() != ".tsv") continue;
        const auto extractor = new FileExtractor(file.path().c_str());
        const auto launch = extractor->toRadiosondeLaunch();
        if(
            db.queryData(
                "LaunchSites",
                "WMONumber",
                "LaunchSites.WMONumber='" + launch->GetLaunchSite().GetWmoNumber() + "'",
                ""
                ).empty()
        ) {

        db.insertIntoTable(
            "LaunchSites",
            "WMONumber, StationName, Longitude, Latitude",
            "'" + launch->GetLaunchSite().GetWmoNumber()
            + "', '" + launch->GetLaunchSite().GetStationName()
            + "', " + std::to_string(launch->GetLaunchSite().GetLongitude())
            + ", " + std::to_string(launch->GetLaunchSite().GetLatitude())
            );
        }
    }
}

However, when I use this function here:

std::map<std::string, std::pair<time_t, std::string>> LaunchManager::fetchTopLaunches(const int limit) const {
    auto query = db->queryData(
        "RadiosondeLaunches",
        "LaunchSites.WMONumber, RadiosondeLaunches.LaunchDate, LaunchSites.StationName",
        "",
        "JOIN LaunchSites ON LaunchSites.WMONumber = RadiosondeLaunches.LaunchSite"
    );
    std::map<std::string, std::pair<time_t, std::string>> results = {};
    for (auto row : query) {
        results[row["LaunchSites.StationName"].asString() + " - " + std::to_string(row["RadiosondeLaunches.LaunchDate"].asTime())]
            = std::make_pair(row["RadiosondeLaunches.LaunchDate"].asTime(), row["LaunchSites.WMONumber"].asString());
    }
    return results;
}

it doesn't like that. It gives the error: Error preparing statement. This error comes from my check in the queryData function when preparing the statement, and when I output this, the prepare function returns 21, which according to the SQLite result codes is a SQLITE_MISUSE or 'Library used incorrectly'.

The statement produced from my function is is:

SELECT LaunchSites.WMONumber, RadiosondeLaunches.LaunchDate, LaunchSites.StationName FROM RadiosondeLaunches JOIN LaunchSites ON LaunchSites.WMONumber = RadiosondeLaunches.LaunchSite;

which when I run in my Jetbrains database tool, works fine. I can't figure out whats wrong so I'm reaching in case anyone might know.

[lime dragon]

When your question is answered use !solved to mark the question as resolved.

Remember to ask specific questions, provide necessary details, and reduce your question to its simplest form. For tips on how to ask a good question use !howto ask.

[teal epoch]

I have found this page on the error, which suggests that the error could because I'm running another statement on another thread
https://sqliteviewer.com/blog/how-to-fix-sqlite-error-code-21/

but I havn't done any specific multithreading in my codebase, and just checking through the procceses, all initial uses of the query function that work should have been finished well before this specific query is run

[teal epoch]

Does anybody have any ideas?

[teal epoch]

Solved, turns out my database variable in my class was being destroyed by the time I was running my code the second time, so I made each function open & close the database itself.

!solved

[lime dragon]

Thank you and let us know if you have any more questions!

This thread is now set to auto-hide after an hour of inactivity

[winter wyvern]

there are SQL injection vulnerabilities in DatabaseManager::queryData

And possibly loads of memory leaks / memory related ticking time bombs due to suspicious looking usages of new

const auto colValue = new DatabaseValue(stmt, col);

            row[colName] = *colValue;

here colValue is never deleted, this is a guaranteed memory leak. I don't know what kinds of hacks you run in the copy constructor and why you decided to do this, but this is bad

[teal epoch]

The constructor just attempts to converts the statement into text, int, float etc. and stores it as a variable

Perhaps you can help me fix it, but the reason I decided to do it this way was to try and make interacting with the database easier through abstraction

[winter wyvern]

Okay, sure, your constructor does stuff, but my point still stands

[winter wyvern]

mhm, I can help you

Abstractions are fine, as long as you don't insert horrible bugs

You here?

[teal epoch]

Yes, I'll add that delete code

[teal epoch]

Ping me when your next on I'll try and be here if I can

[winter wyvern]

I need to analyse you entire codebase to see if delete will not crash, this heavily depends on what the copy constructor of the class DatabaseValue does

send a giithub link or something

[teal epoch]

What's your gh username/email?

[winter wyvern]

I will read the code but you will be the one doing the commits, just make it public for a little bit

[teal epoch]

https://github.com/Nuckerr/computer-science-nea

I'm quite new to C++ coming from higher level languages like Java, so I would love to understand why and how I'm doing things wrong

[winter wyvern]

Basically, Java has a garbage collector, C++ doesn't, so when you keep using new everywhere there isn't anyone to clean up those objects, so this results in hundreds of memory leaks.

You are trying to use the build system cmake, the GUI library wxWidgets, the database sqlite3, but you don't know the basics of C++, this is like trying to compete in a formula 1 race without having a driver's license.

it may not crash but these memory leaks may accumulate fast and eat up all of your ram

you do close the database connections, but you aren't using the C++ equivalent to Java's try-with-resources to ensure the resource is destroyed even if exceptions are thrown

Use cat log.txt | less to see the coloured output of the comments I made

You can set your github repo back to private

[teal epoch]

Yeah, though I've always found the best way to learn is by doing aha

[teal epoch]

This is amazing, thank you so much. I will read through this and get back with any questions I have

[teal epoch]

• I was originally storing a pointer to the database (sqlite3*) as a variable in my class, however it was causing issues in my code that prompted this post. So by having every function that interacts with the database open/close it fixed the issue, but obviously there must be a way to store the database instance in memory.
• Where there is SQL injection, is it just a matter of sanatizing the inputted strings? I didn't see this as needed since it isn't taken from directly inputted text by the end user. But is there a risk of memory injection or something?
• What do you mean by making the regex code some singleton?

[winter wyvern]

I was originally storing a pointer to the database (sqlite3*) as a variable in my class, however it was causing issues in my code that prompted this post.

Oh you sweet summer child... Let me guess, double database closing leading to errors that you can't run anything with the conneciton afterwards?

[winter wyvern]

https://en.wikipedia.org/wiki/SQL_injection

To fix this, after you create the prepared statement with sqlite3_prepare_v2, you are required to use ? placeholders and use sqlite3_bind_text, or whatever other types you want to fill in, like this the sqlite3 library will safely insert the text even if it contains funky shit like quotes, slashes, backslashes, double dashes, etc...
next you run sqlite3_step

https://sqlite.org/c3ref/bind_blob.html
@teal epoch
📖 👀

What do you mean by making the regex code some singleton?
What I mean is

class RegexSingleton
{
  static const std::regex& GetInstance()
  {
    static std::regex instance("regex_pattern_here", 
    std::regex_constants::ECMAScript || std::regex_constants::optimize);
    
    return instance;
  }
};

void findMatch(const std::string& largeInput)
{
  if(std::regex_match(largeInput, RegexSingleton::GetInstance())
  {
      //do stuff
  }    
}

Here, the same std::regex object (which may be computationally expensive to create) is made only once, and not multiple times every single time

This works only when the regex to match is the same everywhere of course

(if you are in need of customization, well then unordered_map<std::string, std::regex> cachedRegexes; where the key is the string pattern and the value is the constructed regex from that same pattern

[winter wyvern]

A classic 👌

[winter wyvern]

@teal epoch