You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "WillAyd (via GitHub)" <gi...@apache.org> on 2023/06/13 20:00:59 UTC
[GitHub] [arrow-adbc] WillAyd commented on a diff in pull request #783: clean(c/driver/postgresql): Changed import to common/util
WillAyd commented on code in PR #783:
URL: https://github.com/apache/arrow-adbc/pull/783#discussion_r1228635573
##########
c/driver/common/CMakeLists.txt:
##########
@@ -17,8 +17,8 @@
add_library(adbc_driver_common STATIC utils.c)
set_target_properties(adbc_driver_common PROPERTIES POSITION_INDEPENDENT_CODE ON)
-include_directories(SYSTEM ${REPOSITORY_ROOT})
-include_directories(SYSTEM ${REPOSITORY_ROOT}/c/vendor)
+target_include_directories(adbc_driver_common SYSTEM
Review Comment:
Is there any particular reason for us to specify SYSTEM here? Just repeating what I see on other includes but not sure the purpose
##########
c/vendor/nanoarrow/nanoarrow.h:
##########
@@ -191,6 +191,9 @@ static inline void ArrowArrayStreamMove(struct ArrowArrayStream* src,
#define _NANOARROW_CHECK_RANGE(x_, min_, max_) \
NANOARROW_RETURN_NOT_OK((x_ >= min_ && x_ <= max_) ? NANOARROW_OK : EINVAL)
+#define _NANOARROW_CHECK_UPPER_LIMIT(x_, max_) \
Review Comment:
Without this was getting errors like:
```c
/home/willayd/clones/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:192:31: error: comparison of unsigned expression in ‘>= 0’ is always true [-Werror=type-limits]
192 | NANOARROW_RETURN_NOT_OK((x_ >= min_ && x_ <= max_) ? NANOARROW_OK : EINVAL)
| ^
/home/willayd/clones/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:187:23: note: in definition of macro ‘_NANOARROW_RETURN_NOT_OK_IMPL’
187 | const int NAME = (EXPR); \
| ^~~~
/home/willayd/clones/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:192:3: note: in expansion of macro ‘NANOARROW_RETURN_NOT_OK’
192 | NANOARROW_RETURN_NOT_OK((x_ >= min_ && x_ <= max_) ? NANOARROW_OK : EINVAL)
| ^~~~~~~~~~~~~~~~~~~~~~~
/home/willayd/clones/arrow-adbc/c/vendor/nanoarrow/nanoarrow.h:2606:7: note: in expansion of macro ‘_NANOARROW_CHECK_RANGE’
2606 | _NANOARROW_CHECK_RANGE(value, 0, UINT8_MAX);
```
which make sense. Not sure why they are appearing now. Need to upstream this in nanoarrow
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org