Skip to content

Address code review comments for Arrow#40939#109

Merged
alinaliBQ merged 7 commits intoapache-odbcfrom
address-minor-comments
Sep 29, 2025
Merged

Address code review comments for Arrow#40939#109
alinaliBQ merged 7 commits intoapache-odbcfrom
address-minor-comments

Conversation

@alinaliBQ
Copy link

Addresses the following code review comments:
apache#40939 (comment)

What does this comment mean? It seems there is a null check inside the loop.

apache#40939 (comment)

nits, but (1) std::memcpy, (2) include cstring?

apache#40939 (comment)

nit, but is the non-const reference meant to be an in-out parameter? I think that is OK but this function confusingly uses a mix of pointers and non-const references

@alinaliBQ alinaliBQ closed this Sep 25, 2025
@alinaliBQ alinaliBQ reopened this Sep 25, 2025
@alinaliBQ alinaliBQ changed the title Address code review comments Address code review comments for Arrow#40939 Sep 25, 2025
@alinaliBQ alinaliBQ marked this pull request as ready for review September 26, 2025 16:49
@alinaliBQ
Copy link
Author

alinaliBQ commented Sep 26, 2025

The segfault error seems pretty random to me, looking into it

@alinaliBQ alinaliBQ merged commit 9ac6e57 into apache-odbc Sep 29, 2025
14 of 18 checks passed
@alinaliBQ alinaliBQ deleted the address-minor-comments branch September 29, 2025 17:14
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