[GitHub] [arrow-adbc] lidavidm commented on a diff in pull request #583: chore(r/adbcpostgresql): Add windows build support

["lidavidm (via GitHub)" <gi...@apache.org>]

lidavidm commented on code in PR #583:
URL: https://github.com/apache/arrow-adbc/pull/583#discussion_r1166432034


##########
c/driver/postgresql/util.h:
##########
@@ -40,11 +40,22 @@ namespace adbcpq {
 #define CONCAT(x, y) x##y
 #define MAKE_NAME(x, y) CONCAT(x, y)
 
-#if defined(_WIN32)
+#if defined(_WIN32) && defined(_MSC_VER)
 static inline uint32_t SwapNetworkToHost(uint32_t x) { return ntohl(x); }
 static inline uint32_t SwapHostToNetwork(uint32_t x) { return htonl(x); }
 static inline uint64_t SwapNetworkToHost(uint64_t x) { return ntohll(x); }
 static inline uint64_t SwapHostToNetwork(uint64_t x) { return htonll(x); }
+#elif defined(_WIN32)
+// e.g., msys2, where ntohll is not necessarily defined
+static inline uint32_t SwapNetworkToHost(uint32_t x) { return ntohl(x); }
+static inline uint32_t SwapHostToNetwork(uint32_t x) { return htonl(x); }
+static inline uint64_t SwapNetworkToHost(uint64_t x) {

Review Comment:
   Cool, it looks like s2 is Apache-licensed which simplifies things



##########
c/driver/postgresql/type.h:
##########
@@ -49,7 +49,7 @@ struct TypeMapping {
   std::unordered_map<uint32_t, PgType> type_mapping;
   // Maps standardized type names to the PostgreSQL type OID to use
   // Example: kInt8 == 20
-  std::unordered_map<PgType, uint32_t> canonical_types;
+  std::unordered_map<int32_t, uint32_t> canonical_types;

Review Comment:
   nit: can you just add a note here for why we aren't using the enum type?



-- 
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