fornwall commented on PR #4504: URL: https://github.com/apache/arrow-adbc/pull/4504#issuecomment-4955679711
> How come we sometimes have .c_str() and sometimes not? (Maybe we should just bring in std::string_literals?) The reason for `c_str()` here is that we're using the ADBC C API, as in [AdbcStatementSetSqlQuery](https://github.com/apache/arrow-adbc/blob/main/c/include/arrow-adbc/adbc.h#L2068), which takes the query as `const char*`: ```c AdbcStatusCode AdbcStatementSetSqlQuery(struct AdbcStatement* statement, const char* query, struct AdbcError* error); ``` This contrasts to the validation suite internal functions, which is C++ and takes strings or string views. Given that, I don't think `std::string_literals` would help here - that helps with reducing syntactic overhead literals be strings (`std::string("mystring")` -> `"mystring"s`), but not the other way around. One thing we could do here to perhaps reduce noice is extracting the query into a variable, like: ```c++ ASSERT_THAT(AdbcStatementSetSqlQuery( &statement, ("SELECT * FROM " + quirks()->QuoteIdentifier("bulk_ingest") + " ORDER BY " + quirks()->QuoteIdentifier("col") + " ASC NULLS FIRST") .c_str(), &error), IsOkStatus(&error)); ``` -> ```c++ std::string query = ("SELECT * FROM " + quirks()->QuoteIdentifier("bulk_ingest") + " ORDER BY " + quirks()->QuoteIdentifier("col") + " ASC NULLS FIRST"); ASSERT_THAT(AdbcStatementSetSqlQuery(&statement, query.c_str(), &error), IsOkStatus(&error)); ``` Should I do that here? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
