godot-sqlite icon indicating copy to clipboard operation
godot-sqlite copied to clipboard

query() should return query_result, not boolean

Open MrSmite opened this issue 4 years ago • 3 comments

Describe the solution you'd like

Since gdscript doesn't support pointers or passing by reference, it would be more efficient and possibly safer to have query() return the actual recordset instead of a boolean. In the current implementation, the caller has to duplicate the recordset into an empty dictionary in order to preserve it or it gets deleted on the next query().

Current:

var my_result = {}
 
if db.query(sql_string):
    my_result = db.query_result.duplicate()    # adds more overhead if query_result is complex

New:

var my_result = {}
 
my_result = db.query(sql_string)

if my_result.empty():
    # error handling
else
    # do stuff with my_result without having to duplicate it or worry it will be overwritten by another query()

MrSmite avatar Nov 05 '21 05:11 MrSmite

Hey @MrSmite

I've implemented it like that because duplicating an Array creates considerable overhead (both in C++ and GDScript). There are a lot of queries that do not require anything to be returned in which case this duplication would just be a duplication of an empty array.

I'll think about it though 🤔

2shady4u avatar Nov 07 '21 09:11 2shady4u

Why would you need to duplicate the array? The dictionary can be passed directly to sqlite and modified.

extends Node2D

func _ready():
	var d = {"zero" : 0, "one" : 1, "two" : 2}
	mod_dict(d)
	print(d["zero"])
	
func mod_dict(dict: Dictionary):
	dict["zero"] = 5

This indicates that variables are in fact passed by reference (I was mistaken above) so unless there's some strange marshalling problem, the dictionary can be part of the C++ implementation?

bool SQLite::query(String p_query, Dictionary &result)
{
    return query_with_bindings(p_query, Array(), result);
}

bool SQLite::query_with_bindings(String p_query, Array param_bindings, &result)

Then instead of using Dictionary column_dict; to store the query and copying it to a query_result, you could just iterate over the query rows and put them directly into result?

result[String(azColName)] = column_value;

MrSmite avatar Nov 09 '21 09:11 MrSmite