2shady4u / godot-sqlite

GDExtension wrapper for SQLite (Godot 4.x+)
MIT License
850 stars 76 forks source link

query_result gets me a reference, not a duplicated value #134

Closed ndecizion closed 1 year ago

ndecizion commented 1 year ago

Environment:

Issue description: When querying using query() function and fetching value via query_result, the docs indicate that this is a value:

query_result (Array, default=[])

Contains the results from the latest query by value; meaning that this property is safe to use when looping successive queries as it does not get overwritten by any future queries.

query_result_by_reference (Array, default=[])

Contains the results from the latest query by reference and is, as a direct result, cleared and repopulated after every new query.

In my testing, this is not the case, and you have to duplicate query_result to prevent it being overwritten.

Steps to reproduce: Create a table & add some rows Run a query to select row 1 and store the result in a variable Run a second query to select row 2 and store the result in a variable compare the first variable to the second

Minimal reproduction project: I'll work to slap together a small project & gut test demonstrating this.

Additional context

fireworx commented 1 year ago

you have to range over it -> loop/range x db.query_result[x]["field"]

2shady4u commented 1 year ago

I've just tested this on my end and everything works as intended:

extends Node

const SQLite = preload("res://addons/godot-sqlite/godot-sqlite-wrapper.gd")
var db

var table_name := "company"

signal output_received(text)
signal texture_received(texture)

func _ready():
    var table_dict : Dictionary = Dictionary()
    table_dict["id"] = {"data_type":"int", "primary_key": true, "not_null": true}
    table_dict["name"] = {"data_type":"text", "not_null": true}
    table_dict["age"] = {"data_type":"int", "not_null": true}

    db = SQLite.new()
    db.open_db()
    db.drop_table(table_name)
    db.create_table(table_name, table_dict)

    db.insert_row(table_name, {"id": 1, "name": "Bob", "age": 29})
    db.insert_row(table_name, {"id": 2, "name": "Amber", "age": 31})

    db.query("SELECT name FROM {table} WHERE age > 30;".format({"table": table_name}))
    var query_result1: Array = db.query_result
    var query_result_by_ref1: Array = db.query_result_by_reference

    db.query("SELECT name FROM {table} WHERE age < 30;".format({"table": table_name}))
    var query_result2: Array = db.query_result
    var query_result_by_ref2: Array = db.query_result_by_reference

    print(str(query_result1)) # Prints [{name:Amber}] <-- Is NOT overwritten because it is passed by value
    print(str(query_result2)) # Prints [{name:Bob}]

    print(str(query_result_by_ref1)) # Prints [{name:Bob}] <-- Was overwritten because it is passed by reference
    print(str(query_result_by_ref2)) # Prints [{name:Bob}]

    db.close_db()

Please tell me what you is different in your case?

godot-sqlite version: 7.4.1

There is no version godot-sqlite version 7.4.1 😄

2shady4u commented 1 year ago

I have found a potential issue with the passing by reference/value issue if you use the select_rows-method. I'll have to investigate it a bit more...

ndecizion commented 1 year ago

Very sorry for posting this, then vanishing for the past couple of weeks. I have spent some time troubleshooting my code and figured out that my original issue isn't what I thought it was. I didn't read my test code very carefully. I've also updated the Godot SQLite version in my original post. Reading comprehension was low when I originally posted this.

What I was actually seeing is apparent in the following sequence:

  1. use select_rows and save that value as query_result_1
  2. then perform an update to that row in the DB using query
  3. use select_rows again and save that value as query_result_2

In my testing I found that performing an update query actually updated the value in query_result_1, which I found quite surprising.

extends Node

const SQLite = preload("res://addons/godot-sqlite/godot-sqlite-wrapper.gd")
var db

var table_name := "company"

signal output_received(text)
signal texture_received(texture)

func _ready():
    var table_dict : Dictionary = Dictionary()
    table_dict["id"] = {"data_type":"int", "primary_key": true, "not_null": true}
    table_dict["name"] = {"data_type":"text", "not_null": true}
    table_dict["age"] = {"data_type":"int", "not_null": true}

    db = SQLite.new()
    db.open_db()
    db.drop_table(table_name)
    db.create_table(table_name, table_dict)

    db.insert_row(table_name, {"id": 1, "name": "Bob", "age": 29})
    db.insert_row(table_name, {"id": 2, "name": "Amber", "age": 31})

    var query_result1: Array = db.select_rows(table_name, "age < 30", ["name"])
    print(str(query_result1)) # Expect [{name:Bob}] 

    db.query("UPDATE {table} SET name = 'Steve' WHERE id = 1;".format({"table": table_name}))

    var query_result2: Array = db.select_rows(table_name, "age < 30", ["name"])

    print(str(query_result1)) # Expect [{name:Bob}] actually [{name:Steve}]
    print(str(query_result2)) # Expect [{name:Steve}]

    db.close_db()

Which yields:

Opened database successfully (default.db)
[{name:Bob}]
[{name:Steve}]
[{name:Steve}]
Closed database (default.db)

Looking back at the documentation & reviewing the issue, this might just be a misunderstanding on my part. The docs don't explicitly state that select_rows returns a value or a reference.

2shady4u commented 1 year ago

Hi @ndecizion!

The intended behavior is to return the query_result by value. The actual behavior was wrong and has now been fixed in 7110d30

I have also updated the relevant documentation 😄

ndecizion commented 1 year ago

Neat. Thanks!