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

Insertion of array type column does not work on npgsql. #24055

Closed
rkdrnfds opened this issue Mar 20, 2018 · 4 comments · Fixed by #24227
Closed

Insertion of array type column does not work on npgsql. #24055

rkdrnfds opened this issue Mar 20, 2018 · 4 comments · Fixed by #24227
Assignees

Comments

@rkdrnfds
Copy link

When do array type column insertion via Npgsql, it does not work properly.

There's ongoing issue opened at npgsql github with full description of the problem, so I'm linking the issus so you may look at it.

npgsql/efcore.pg#309

@jordanlewis
Copy link
Member

Hi @rkdrnfds, thanks for the report. We're not experts at npgsql so we might need a little bit of help getting to the bottom of this. Could you please provide a fully working reproduction example?

@justinj
Copy link
Contributor

justinj commented Mar 20, 2018

Could you also try on the newest beta? I suspect this might have been fixed by #23467.

@rkdrnfds
Copy link
Author

rkdrnfds commented Mar 23, 2018

I created repro github for this issue.
repro github

you can test it by manually creating database with table as below
which connects from program by connection string specified in MyDbContext.cs

table DDL:

CREATE TABLE "Chunks" (
	"Id" INT NOT NULL DEFAULT unique_rowid(),
	CONSTRAINT "PK_Chunks" PRIMARY KEY ("Id" ASC),
	FAMILY "primary" ("Id")
)

I tested it in both
v2.0-alpha.20180129 and
v2.0-beta.20180319

0129 alpha reproduces exact error.

but 0319 beta produces other errors during migration though EF core and inserting int type
which I mentioned in other issues. so it just crashes before checking array type.

I'm linking those additional beta version issues here so you can check it.

While creating database by EF Core Cli Command:
#24173
After created database manually:
#24172

@jordanlewis
Copy link
Member

Thanks for the report. I can reproduce the issue you're seeing. I think the problem is that the .NET driver sends arrays of int4, which we don't properly handle today. The following diff fixes it:

diff --git a/pkg/sql/sqlbase/structured.go b/pkg/sql/sqlbase/structured.go
index c4683cf0b..8fcdd2b70 100644
--- a/pkg/sql/sqlbase/structured.go
+++ b/pkg/sql/sqlbase/structured.go
@@ -2277,7 +2277,10 @@ func DatumTypeToColumnSemanticType(ptyp types.T) (ColumnType_SemanticType, error
                if ptyp.FamilyEqual(types.FamCollatedString) {
                        return ColumnType_COLLATEDSTRING, nil
                }
-               return -1, pgerror.NewErrorf(pgerror.CodeFeatureNotSupportedError, "unsupported result type: %s", ptyp)
+               if wrapper, ok := ptyp.(types.TOidWrapper); ok {
+                       return DatumTypeToColumnSemanticType(wrapper.T)
+               }
+               return -1, pgerror.NewErrorf(pgerror.CodeFeatureNotSupportedError, "unsupported result type: %s, %T, %+v", ptyp, ptyp, ptyp)
        }
 }

diff --git a/pkg/sql/sqlbase/table.go b/pkg/sql/sqlbase/table.go
index 2bf7536b5..12a53d53a 100644
--- a/pkg/sql/sqlbase/table.go
+++ b/pkg/sql/sqlbase/table.go
@@ -1922,7 +1922,8 @@ func encodeArray(d *tree.DArray, scratch []byte) ([]byte, error) {
                return scratch, err
        }
        scratch = scratch[0:0]
-       elementType, err := parserTypeToEncodingType(d.ParamTyp)
+       unwrapped := types.UnwrapType(d.ParamTyp)
+       elementType, err := parserTypeToEncodingType(unwrapped)
        if err != nil {
                return nil, err
        }

but it needs some more testing.

justinj pushed a commit to justinj/cockroach that referenced this issue Mar 26, 2018
Fixes cockroachdb#24055.

This was breaking arrays with Npgsql.

🔬🐶 on the C# stuff.

Release note (bug fix): Fixed a bug involving Npgsql and array values.
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

Successfully merging a pull request may close this issue.

3 participants