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

c/driver/postgresql: Get option for strings returns null terminator #1256

Closed
paleolimbot opened this issue Nov 6, 2023 · 1 comment · Fixed by #1258
Closed

c/driver/postgresql: Get option for strings returns null terminator #1256

paleolimbot opened this issue Nov 6, 2023 · 1 comment · Fixed by #1258

Comments

@paleolimbot
Copy link
Member

First noted here: #1129 (comment)

library(adbcdrivermanager)

adbc_database_init(
  adbcpostgresql::adbcpostgresql(),
  uri = Sys.getenv("ADBC_POSTGRESQL_TEST_URI")
) |> 
  adbc_connection_init() |>
  adbc_connection_get_option("adbc.connection.autocommit")
#> Error in adbc_connection_get_option(adbc_connection_init(adbc_database_init(adbcpostgresql::adbcpostgresql(),
#> :embedded nul in string: 'true\0'

It seems like this is intentional: https://github.com/apache/arrow-adbc/blob/main/c/driver/postgresql/connection.cc#L772-L776

...but I didn't expect it given the symmetry with getting bytes options and commitment to communicating the final size. Was it the intention that the null-terminator is included in the output size?

@lidavidm
Copy link
Member

lidavidm commented Nov 6, 2023

Intentional:

arrow-adbc/adbc.h

Lines 1587 to 1588 in 313245b

/// to by value. If there is sufficient space, the driver will copy
/// the option value (including the null terminator) to buffer and set

Because C strings expect null terminators so it's generally easier to just make sure it's there (like how std::string technically wouldn't need one because it tracks the length, but for interoperability it still tracks a null terminator anyways)

paleolimbot added a commit that referenced this issue Nov 6, 2023
@lidavidm lidavidm added this to the ADBC Libraries 0.9.0 milestone Nov 6, 2023
paleolimbot added a commit to paleolimbot/arrow-adbc that referenced this issue Nov 9, 2023
vleslief-ms pushed a commit to vleslief-ms/arrow-adbc that referenced this issue Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants