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

value exception throws when using batch-insert to insert one row #347

Open
x-awaken opened this issue Oct 11, 2022 · 3 comments
Open

value exception throws when using batch-insert to insert one row #347

x-awaken opened this issue Oct 11, 2022 · 3 comments

Comments

@x-awaken
Copy link

x-awaken commented Oct 11, 2022

Environment

  • nanodbc version:2.14
  • DBMS name/version:KingBaseES
  • ODBC connection string:
  • OS and Compiler: WINDOWS vs2017
  • CMake settings:

Actual behavior

when insert one row in batch mode, exception throws:
data/time value out of range: "0000-00-00 00:00:00"
but it work well when insert one more rows.

Expected behavior

insert one row into database with null value success

Minimal Working Example


int main(int argc, char* argv[])
{


    try
    {
        nanodbc::connection connection("KingbaseES", "SYSTEM", "11111", 10, true);
        nanodbc::statement statement(connection);
        execute(connection, "drop table if exists public.simple_test;");
        execute(connection, "create table public.simple_test (a int, b timestamp);");
        prepare(statement, "insert into public.simple_test (a, b) values (?, ?);");

        const int elements = 1;
        int a_data[elements] = {0};
        char b_data[elements][20] = {""};
        bool nulls[elements] = {true};

        statement.bind(0, a_data, elements, nulls);
        statement.bind_strings(1, b_data, nulls);

        execute(statement, elements);
    }
    catch (const exception& e)
    {
        cerr << e.what() << endl;
        return 1;
    }
}
  
@HGeorge-Adapdix
Copy link

I came across this when looking at reporting a similar issue.

Environment

nanodbc version:2.14
DBMS name/version:Postgres
ODBC connection string:Driver={PostgreSQL ANSI};Uid=postgres;Server=localhost;Port=36947
OS and Compiler: Ubuntu 22.04
CMake settings:
TEST(Example, TestStringBindSingleNullRowFails)
{
	const std::string pgConnectionString{
		"Driver={PostgreSQL ANSI};Uid=postgres;Server=localhost;Port=36947"};
	nanodbc::connection conn(pgConnectionString);

	nanodbc::just_execute(conn,
						  "create temporary table test_table (id integer, name varchar(255))");
	nanodbc::just_execute(conn, "truncate test_table");

	nanodbc::transaction transaction(conn);

	nanodbc::statement statement(conn, "insert into test_table (id, name) values (?, ?)");

	std::vector<std::string> ids;
	ids.emplace_back("1");
	const std::array<bool, 1> notNull{{false}};

	statement.bind_strings(0, ids, notNull.data());
	bool pass(false);
	if (pass) {
		// Works if bind null
		statement.bind_null(1, 1);
	} else {
		// Fails if bind string with all nulls
		const std::array<bool, 1> null{{true}};
		statement.bind_strings(1, ids, null.data());
	}

	nanodbc::just_execute(statement, static_cast<int>(ids.size()));

	transaction.commit();

	nanodbc::result result(nanodbc::execute(conn, "select * from test_table"));
	ASSERT_TRUE(result.next());
	ASSERT_FALSE(result.is_null(0));

	// This fails
	ASSERT_TRUE(result.is_null(1));
}

If you use the bind_null interface it works, but bind strings does not insert a null and instead will insert the string ignoring the "null" aspect.

@HGeorge-Adapdix
Copy link

I believe the issue is in bind_parameter

diff --git a/nanodbc/nanodbc.cpp b/nanodbc/nanodbc.cpp
index 9ece354..ed5ad3c 100644
--- a/nanodbc/nanodbc.cpp
+++ b/nanodbc/nanodbc.cpp
@@ -2239,7 +2239,7 @@ public:
             param.scale_,        // decimal digits
             (SQLPOINTER)buffer.values_, // parameter value
             buffer_size,                // buffer length
-            (buffer.size_ <= 1 ? nullptr : bind_len_or_null_[param.index_].data()));
+            (buffer_size <= 1 ? nullptr : bind_len_or_null_[param.index_].data()));
 
         if (!success(rc))
             NANODBC_THROW_DATABASE_ERROR(stmt_, SQL_HANDLE_STMT);

In the one row example the buffer.size_ is 1 and the buffer_size is 2.

Changing it fixes it, so that code is what is causing the issue but I am not 100% sure what the code is supposed to be doing (hence no PR). I am unsure as to why you wouldn't just always pass in the bind_len_or_null_ value but looking through the history that appears to have been there for many years (tracked it back as far as 2bd4e2e).

Not sure if anyone has any insight here.

@mloskot
Copy link
Member

mloskot commented Dec 21, 2023

@x-awaken & @HGeorge-Adapdix Thank you for your reports.
Hmm, this is interesting as it looks like an overlooked place that miss the change from buffer.size_ to local variable buffer_size:

(buffer.size_ <= 1 ? nullptr : bind_len_or_null_[param.index_].data()));

(buffer.size_ <= 1 ? nullptr : bind_len_or_null_[param.index_].data()));

I do not recall anything specific on the use of buffer.size_ in those places, instead of buffer_size_.

What I'd suggest is to submit PR with

and let's see what the CI builds think about it.
I will also try to run local tests of such PR and perhaps add more test cases.

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

No branches or pull requests

3 participants