Check PACKED_BYTE_ARRAY size before trying to bind to parameter

[cridenour]

Not sure if this is the right approach, but I have a nullable BLOB column in SQLite, and in GDScript I will pass a PackedByteArray() to avoid GDScript 2.0's stronger typing error. I figured an empty array would be OK, but it would throw an error on sqlite3_bind_blob64 complaining about trying to grab the 0 p_index when ->size() = 0.

This check avoids that error, but would fail on a not_null BLOB column when passed an empty array. I think that's OK because null is the likely the proper value to handle an empty variable being passed?

[2shady4u]

Hi @cridenour,

Just to be sure I understand the situation correctly, there are two cases:

1. The table has a nullable BLOB column

Old behaviour: Passing an empty PackedByteArray throws an error because the method tries to access the zeroth element of the array

New behaviour: Passing an empty PackedByteArray does not throw an error

2. The table has a non-nullable BLOB column

Old behaviour: Passing an empty PackedByteArray does not throw an error

New behaviour: Passing an empty PackedByteArray throws an error because the method binds a null

[cridenour]

I believe passing an empty PackedByteArray failed in both cases in the old behavior. I can do some more testing though.

[2shady4u]

I'm currently trying to test the issue in Godot 3.5.1 and I'm unable to replicate the observed behaviour 🤔 Here's my code:

extends Node

const SQLite = preload("res://addons/godot-sqlite/bin/gdsqlite.gdns")
var db

enum VerbosityLevel {
    QUIET = 0,
    NORMAL = 1,
    VERBOSE = 2,
    VERY_VERBOSE = 3
}
const verbosity_level : int = VerbosityLevel.VERBOSE

var db_name := "res://data/test"
var table_name := "company"

signal output_received(text)
signal texture_received(texture)

func _ready():
    # Enable/disable examples here:
    example_of_blob_io()

# The BLOB-datatype is useful when lots of raw data has to be stored.
# For example images fall into this category!
func example_of_blob_io():
    # Make a big table containing the variable types.
    var table_dict : Dictionary = Dictionary()
    table_dict["id"] = {"data_type":"int", "primary_key": true, "not_null": true}
    table_dict["data"] = {"data_type":"blob", "not_null": false}

    var texture := preload("res://icon.png")
    var tex_data : PoolByteArray = texture.get_data().save_png_to_buffer()

    db = SQLite.new()
    db.path = db_name
    db.verbosity_level = verbosity_level
    # Open the database using the db_name found in the path variable
    db.open_db()
    # Throw away any table that was already present
    db.drop_table(table_name)
    # Create a table with the structure found in table_dict and add it to the database
    db.create_table(table_name, table_dict)

    # Insert a new row in the table and bind the texture data to the data column.
    db.insert_row(table_name, {"id": 1, "data": tex_data})
    db.insert_row(table_name, {"id": 2, "data": PoolByteArray()})

    # Close the current database
    db.close_db()

This code doesn't generate any errors which is what I would expect. Also when I change the column to be non-nullable the SQLite binding returns an error due to the PoolByteArray being empty which is the expected/wanted behavior.

I'll check the exact same code with Godot 4.0 and I'll get back to you with that.

[2shady4u]

I can confirm that the code returns an error in BOTH CASES in Godot 4.0 I'll do some more investigation, because this actually shouldn't happen :/

ERROR: Index p_index = 0 is out of bounds (self->size() = 0).
   at: gdnative_packed_byte_array_operator_index_const (core/extension/gdnative_interface.cpp:691)

This might be an issue that has to be fixed in the godot-cpp-repository instead.

[cridenour]

You know, I never thought it would be godot-cpp's fault but that makes more sense since we're not ever trying to call [0].

[2shady4u]

You know, I never thought it would be godot-cpp's fault but that makes more sense since we're not ever trying to call [0].

I've opened an issue in the godot-cpp-repository which can be found here: https://github.com/godotengine/godot-cpp/issues/931

We'll see what they have to say about it 😄

[2shady4u]

Hi @cridenour

I have decided to merge this PR for following reasons:

• It is a good & elegant solution for the time being
• It will probably take a while for the godot-cpp repository to get a fix (if ever)

Thank you for your contribution :)