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

Error type with CommandBuilder #1591

Closed
lawrencelebowsky opened this Issue Jun 6, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@lawrencelebowsky

lawrencelebowsky commented Jun 6, 2017

Using CommandBuilder there are some problems with type errors such as Npgsql.PostgresException: 42883: operator does not exist: boolean = integer
The problem is solved by setting the property SetAllValues = True.

Note: In previous version (2.0.11) Parameters of CommandBuilder have the DbType derived from database. Now are always Object.

Table:

CREATE TABLE Test (
Cod varchar(5) NOT NULL,
Descr varchar(40),
Data date,
DataOra timestamp,
Intero SmallInt NOT NULL,
Decimale Currency,
Singolo float,
Booleano bit,
Nota varchar(255),
CONSTRAINT PK_test_Cod PRIMARY KEY (Cod)
);

Code:

Dim daDataAdapter As New Npgsql.NpgsqlDataAdapter("SELECT Cod, Descr, Data, DataOra, Intero, Decimale, Singolo, Booleano, Nota FROM Test", cntDb)
Dim cbCommandBuilder As New Npgsql.NpgsqlCommandBuilder(daDataAdapter)
Dim dtTable As New DataTable

With daDataAdapter
    .InsertCommand = cbCommandBuilder.GetInsertCommand
    .UpdateCommand = cbCommandBuilder.GetUpdateCommand
    .DeleteCommand = cbCommandBuilder.GetDeleteCommand
End With

daDataAdapter.Fill(dtTable)

dtTable.Rows(0).Item("Singolo") = 1.1D

Try
    daDataAdapter.Update(dtTable)
Catch ex As Exception
    MsgBox(ex.Message)
End Try

Exception message:
Npgsql.PostgresException (0x80004005): 42883: operator does not exist: boolean = integer
in System.Data.Common.DbDataAdapter.UpdatedRowStatusErrors(RowUpdatedEventArgs rowUpdatedEvent, BatchCommandInfo[] batchCommands, Int32 commandCount)
in System.Data.Common.DbDataAdapter.UpdatedRowStatus(RowUpdatedEventArgs rowUpdatedEvent, BatchCommandInfo[] batchCommands, Int32 commandCount)
in System.Data.Common.DbDataAdapter.Update(DataRow[] dataRows, DataTableMapping tableMapping)
in System.Data.Common.DbDataAdapter.UpdateFromDataTable(DataTable dataTable, DataTableMapping tableMapping)
in System.Data.Common.DbDataAdapter.Update(DataTable dataTable)

Npgsql version: 3.2.2
PostgreSQL version: 9.2.15 - 9.5.7

@roji roji added this to the Backlog milestone Jun 9, 2018

@Brar

This comment has been minimized.

Show comment
Hide comment
@Brar

Brar Jul 3, 2018

Member

@roji I'm investigating this one. You can assign it to me if you want.

Member

Brar commented Jul 3, 2018

@roji I'm investigating this one. You can assign it to me if you want.

@Brar

This comment has been minimized.

Show comment
Hide comment
@Brar

Brar Jul 3, 2018

Member

@lawrencelebowsky I'm failing to reproduce this issue (Npgsql 4.0.1, Postgresql 9.5.13) with the following test:

[Test, IssueLink("https://github.com/npgsql/npgsql/issues/1591")]
public void TemporaryTestToNarrowDownIssue1591()
{
    using (var conn = OpenConnection())
    {
        conn.ExecuteNonQuery(@"
            CREATE TABLE pg_temp.test (
                Cod varchar(5) NOT NULL,
                Descr varchar(40),
                Data date,
                DataOra timestamp,
                Intero smallInt NOT NULL,
                Decimale money,
                Singolo float,
                Booleano bit,
                Nota varchar(255),
                CONSTRAINT PK_test_Cod PRIMARY KEY (Cod)
            );
            INSERT INTO test VALUES('key1', 'description', '2018-07-03', '2018-07-03 07:02:00', 123, 123.4, 1234.5, B'1', 'note');
        ");

        var daDataAdapter =
            new NpgsqlDataAdapter(
                "SELECT Cod, Descr, Data, DataOra, Intero, Decimale, Singolo, Booleano, Nota FROM test", conn);
        var cbCommandBuilder = new NpgsqlCommandBuilder(daDataAdapter);
        var dtTable = new DataTable();

        daDataAdapter.InsertCommand = cbCommandBuilder.GetInsertCommand();
        daDataAdapter.UpdateCommand = cbCommandBuilder.GetUpdateCommand();
        daDataAdapter.DeleteCommand = cbCommandBuilder.GetDeleteCommand();

        daDataAdapter.Fill(dtTable);

        var row = dtTable.Rows[0];

        Assert.That(row[0], Is.EqualTo("key1"));
        Assert.That(row[1], Is.EqualTo("description"));
        Assert.That(row[2], Is.EqualTo(new DateTime(2018, 7, 3)));
        Assert.That(row[3], Is.EqualTo(new DateTime(2018, 7, 3, 7, 2, 0)));
        Assert.That(row[4], Is.EqualTo(123));
        Assert.That(row[5], Is.EqualTo(123.4));
        Assert.That(row[6], Is.EqualTo(1234.5));
        Assert.That(row[7], Is.EqualTo(true));
        Assert.That(row[8], Is.EqualTo("note"));

        dtTable.Rows[0]["Singolo"] = 1.1D;

        Assert.That(daDataAdapter.Update(dtTable), Is.EqualTo(1));
    }
}

Is there something you didn't tell us?
Can you please verify that this issue is still present?

Member

Brar commented Jul 3, 2018

@lawrencelebowsky I'm failing to reproduce this issue (Npgsql 4.0.1, Postgresql 9.5.13) with the following test:

[Test, IssueLink("https://github.com/npgsql/npgsql/issues/1591")]
public void TemporaryTestToNarrowDownIssue1591()
{
    using (var conn = OpenConnection())
    {
        conn.ExecuteNonQuery(@"
            CREATE TABLE pg_temp.test (
                Cod varchar(5) NOT NULL,
                Descr varchar(40),
                Data date,
                DataOra timestamp,
                Intero smallInt NOT NULL,
                Decimale money,
                Singolo float,
                Booleano bit,
                Nota varchar(255),
                CONSTRAINT PK_test_Cod PRIMARY KEY (Cod)
            );
            INSERT INTO test VALUES('key1', 'description', '2018-07-03', '2018-07-03 07:02:00', 123, 123.4, 1234.5, B'1', 'note');
        ");

        var daDataAdapter =
            new NpgsqlDataAdapter(
                "SELECT Cod, Descr, Data, DataOra, Intero, Decimale, Singolo, Booleano, Nota FROM test", conn);
        var cbCommandBuilder = new NpgsqlCommandBuilder(daDataAdapter);
        var dtTable = new DataTable();

        daDataAdapter.InsertCommand = cbCommandBuilder.GetInsertCommand();
        daDataAdapter.UpdateCommand = cbCommandBuilder.GetUpdateCommand();
        daDataAdapter.DeleteCommand = cbCommandBuilder.GetDeleteCommand();

        daDataAdapter.Fill(dtTable);

        var row = dtTable.Rows[0];

        Assert.That(row[0], Is.EqualTo("key1"));
        Assert.That(row[1], Is.EqualTo("description"));
        Assert.That(row[2], Is.EqualTo(new DateTime(2018, 7, 3)));
        Assert.That(row[3], Is.EqualTo(new DateTime(2018, 7, 3, 7, 2, 0)));
        Assert.That(row[4], Is.EqualTo(123));
        Assert.That(row[5], Is.EqualTo(123.4));
        Assert.That(row[6], Is.EqualTo(1234.5));
        Assert.That(row[7], Is.EqualTo(true));
        Assert.That(row[8], Is.EqualTo("note"));

        dtTable.Rows[0]["Singolo"] = 1.1D;

        Assert.That(daDataAdapter.Update(dtTable), Is.EqualTo(1));
    }
}

Is there something you didn't tell us?
Can you please verify that this issue is still present?

@roji roji removed this from the Backlog milestone Jul 3, 2018

@Brar

This comment has been minimized.

Show comment
Hide comment
@Brar

Brar Jul 4, 2018

Member

Ok, in the meantime I've verified that this is a regression that is still present in hotfix/3.2.8 but not in v4.0.0.

As we probably don't want to backport the commit that resolved it I'll try to find a minimal fix for this for v3.2.8.

@roji you might want to assign this to the 3.2.8 Milestone unless you have pressure to release it (it currently has no open issue).

Member

Brar commented Jul 4, 2018

Ok, in the meantime I've verified that this is a regression that is still present in hotfix/3.2.8 but not in v4.0.0.

As we probably don't want to backport the commit that resolved it I'll try to find a minimal fix for this for v3.2.8.

@roji you might want to assign this to the 3.2.8 Milestone unless you have pressure to release it (it currently has no open issue).

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Jul 4, 2018

Member

No pressure to release really, we can wait for this.

However, annoyingly github only allows one milestone per issue, so there's no way to say "fixed in both 3.2.8 and 4.0.2". So I'm setting it to milestone 4.0.2 although we can definitely backport to 3.2.8.

Member

roji commented Jul 4, 2018

No pressure to release really, we can wait for this.

However, annoyingly github only allows one milestone per issue, so there's no way to say "fixed in both 3.2.8 and 4.0.2". So I'm setting it to milestone 4.0.2 although we can definitely backport to 3.2.8.

@roji roji added this to the 4.0.2 milestone Jul 4, 2018

@Brar

This comment has been minimized.

Show comment
Hide comment
@Brar

Brar Jul 4, 2018

Member

I'm setting it to milestone 4.0.1 although we can definitely backport to 3.2.8.

@roji did you realize that this is not an issue in 4.0.0 and above?
I'll file a pull request against hotfix/3.2.8 once I've fixed it and not against dev (though we might want to cherry-pick the commit containing the test for 4.0 as it was/is obviously missing).

Member

Brar commented Jul 4, 2018

I'm setting it to milestone 4.0.1 although we can definitely backport to 3.2.8.

@roji did you realize that this is not an issue in 4.0.0 and above?
I'll file a pull request against hotfix/3.2.8 once I've fixed it and not against dev (though we might want to cherry-pick the commit containing the test for 4.0 as it was/is obviously missing).

@roji roji modified the milestones: 4.0.2, 3.2.8 Jul 4, 2018

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Jul 4, 2018

Member

Oh sorry, I read this too quickly.

Setting milestone to 3.2.8.

Member

roji commented Jul 4, 2018

Oh sorry, I read this too quickly.

Setting milestone to 3.2.8.

Brar added a commit to Brar/npgsql that referenced this issue Jul 4, 2018

Brar added a commit to Brar/npgsql that referenced this issue Jul 4, 2018

Add test for #1591
Test if NpgsqlCommandBuilder.GetUpdateCommand() creates a command that
has its Parameters set to the correct NpgsqlDbType so that updating the
backend via NpgsqlDataAdapter works.

Brar added a commit to Brar/npgsql that referenced this issue Jul 4, 2018

Implement NpgsqlCommandBuilder.ApplyParameterInfo()
In order to safely update the database with a command that was generated
via NpgsqlCommandBuilder.GetUpdateCommand() we need to have the correct
NpgsqlDbType set for every parameter.
This commit is mostly a backport of the 4.x code that is necessary to
do this in NpgsqlCommandBuilder.ApplyParameterInfo().

Fixes #1591

Brar added a commit to Brar/npgsql that referenced this issue Jul 5, 2018

Implement NpgsqlCommandBuilder.ApplyParameterInfo()
In order to safely update the database with a command that was generated
via NpgsqlCommandBuilder.GetUpdateCommand() we need to have the correct
NpgsqlDbType set for every parameter.
This commit is mostly a backport of the 4.x code that is necessary to
do this in NpgsqlCommandBuilder.ApplyParameterInfo().

Fixes #1591
@Brar

This comment has been minimized.

Show comment
Hide comment
@Brar

Brar Jul 5, 2018

Member

Closed via 7950b21

Member

Brar commented Jul 5, 2018

Closed via 7950b21

@Brar Brar closed this Jul 5, 2018

@Brar

This comment has been minimized.

Show comment
Hide comment
@Brar

Brar Jul 5, 2018

Member

though we might want to cherry-pick the commit containing the test for 4.0 as it was/is obviously missing

@roji do you want me to forward-port the test (cfbf9ff) to dev to avoid similar regressions in the future?

Member

Brar commented Jul 5, 2018

though we might want to cherry-pick the commit containing the test for 4.0 as it was/is obviously missing

@roji do you want me to forward-port the test (cfbf9ff) to dev to avoid similar regressions in the future?

@roji

This comment has been minimized.

Show comment
Hide comment
@roji

roji Jul 5, 2018

Member

Sure, sounds good. You can go ahead and push a commit directly with the test, without a PR+review.

Member

roji commented Jul 5, 2018

Sure, sounds good. You can go ahead and push a commit directly with the test, without a PR+review.

Brar added a commit that referenced this issue Jul 5, 2018

Add test for #1591
Test if NpgsqlCommandBuilder.GetUpdateCommand() creates a command that
has its Parameters set to the correct NpgsqlDbType so that updating the
backend via NpgsqlDataAdapter works.

(cherry-picked cfbf9ff)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment