Skip to content

Conversation

@amoeba
Copy link
Member

@amoeba amoeba commented Jul 24, 2025

No description provided.

@lidavidm lidavidm changed the title feat(postgresql): add test for composite type behavior feat(c/driver/postgresql): add test for composite type behavior Jul 24, 2025
@amoeba
Copy link
Member Author

amoeba commented Jul 24, 2025

Still working on this, currently this fails with,

/Users/bryce/src/apache/arrow-adbc/c/driver/postgresql/postgresql_test.cc:1143: Failure
Expected equality of these values:
  reader.schema->children[0]->format
    Which is: "z"
  "i"

@lidavidm
Copy link
Member

Hmm, maybe it isn't actually supported then...Or maybe you have to reconnect after creating the type to reload the type mapping

@amoeba amoeba force-pushed the feat/c-postgresql-composite-test branch from 763605b to 3e2eb6e Compare July 24, 2025 21:16
@amoeba
Copy link
Member Author

amoeba commented Jul 24, 2025

I added what I think was a mid-test reconnect in 3e2eb6e. The new test still fails,

adbc ❯ driver/postgresql/adbc-driver-postgresql-test --gtest_filter="*Composite*"                                                                   (adbc)
adbc-driver-postgresql-test(30459,0x204bd5f00) malloc: nano zone abandoned due to inability to reserve vm space.
Running main() from /Users/runner/miniforge3/conda-bld/gtest-split_1748319995326/work/googletest/src/gtest_main.cc
Note: Google Test filter = *Composite*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from PostgresStatementTest
[ RUN      ] PostgresStatementTest.PostgresCompositeTest
/Users/bryce/src/apache/arrow-adbc/c/driver/postgresql/postgresql_test.cc:1893: Failure
Expected equality of these values:
  reader.schema->children[0]->format
    Which is: "z"
  "i"

[  FAILED  ] PostgresStatementTest.PostgresCompositeTest (103 ms)
[----------] 1 test from PostgresStatementTest (103 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (103 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] PostgresStatementTest.PostgresCompositeTest

 1 FAILED TEST

@amoeba
Copy link
Member Author

amoeba commented Jul 29, 2025

Hi @lidavidm could you take a look at my test to see if that looks like I'm testing it correctly? If so, I'm not sure how hard adding support would be but I could at least update #3195 to document the current status.

@lidavidm
Copy link
Member

Sorry this is on my backlog to poke

@amoeba
Copy link
Member Author

amoeba commented Jul 30, 2025

No problem at all. Thanks.

Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I gave it a test in Python, creating the type beforehand in psql, and it worked. I think the AdbcDatabase needs to be recreated as that's what caches the type info. I think elsewhere someone requested a way to drop and recreate the type cache and maybe we should consider that

@lidavidm lidavidm changed the title feat(c/driver/postgresql): add test for composite type behavior test(c/driver/postgresql): add test for composite type behavior Aug 10, 2025
@lidavidm
Copy link
Member

In [7]: cur.execute("SELECT * FROM asdf")

In [8]: cur.fetchallarrow()
Out[8]: 
pyarrow.Table
a: struct<x: int32, y: string>
  child 0, x: int32
  child 1, y: string
----
a: [
  -- is_valid: all not null
  -- child 0 type: int32
[1]
  -- child 1 type: string
["foo"]]

@amoeba
Copy link
Member Author

amoeba commented Aug 11, 2025

Very interesting, thanks for figuring it out.

I pushed a fixed test which uses a test helper you may or may not like. I thought about making a new fixture but just adding a ResetTest method on StatementTest was near at hand. Happy to rework this.

@amoeba amoeba force-pushed the feat/c-postgresql-composite-test branch from 237a0b0 to ab421cb Compare August 12, 2025 00:25
@amoeba amoeba marked this pull request as ready for review August 12, 2025 00:28
@github-actions github-actions bot added this to the ADBC Libraries 20 milestone Aug 12, 2025
@amoeba
Copy link
Member Author

amoeba commented Aug 12, 2025

Rebased.

@lidavidm
Copy link
Member

Hmm, ASAN is unhappy about something.

@amoeba
Copy link
Member Author

amoeba commented Aug 12, 2025

I'll take a look later this week.

@lidavidm
Copy link
Member

@amoeba do you think you want to do this for the next release?

@amoeba
Copy link
Member Author

amoeba commented Oct 31, 2025

Hey @lidavidm, I won't be able to get this in for the release. I'll try for the next one.

@lidavidm lidavidm removed this from the ADBC Libraries 21 milestone Nov 3, 2025
@lidavidm lidavidm added this to the ADBC Libraries 22 milestone Nov 3, 2025
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 this pull request may close these issues.

2 participants