Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

query() should return query_result, not boolean #60

Open
MrSmite opened this issue Nov 5, 2021 · 2 comments
Open

query() should return query_result, not boolean #60

MrSmite opened this issue Nov 5, 2021 · 2 comments
Labels
enhancement New feature or request

Comments

@MrSmite
Copy link

MrSmite commented Nov 5, 2021

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 MrSmite added the enhancement New feature or request label Nov 5, 2021
@2shady4u
Copy link
Owner

2shady4u commented Nov 7, 2021

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 🤔

@MrSmite
Copy link
Author

MrSmite commented Nov 9, 2021

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;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants