leborchuk commented on code in PR #1951:
URL: https://github.com/apache/cloudberry/pull/1951#discussion_r3949599688


##########
.github/workflows/build-cloudberry.yml:
##########
@@ -1463,6 +1463,34 @@ jobs:
             exit 0
           fi
 
+          # datalake_fdw needs the Arrow and Parquet C++ libraries, which the
+          # build image does not carry -- nothing in the RPM uses them, so they
+          # would be weight every other job paid for.
+          if [[ "${PGXS_EXTENSION}" == "contrib/datalake_fdw" ]]; then
+            . /etc/os-release
+            if [[ "${VERSION_ID%%.*}" == "8" ]]; then
+              # EPEL 8 has them, but its libarrow-devel needs a utf8proc-devel
+              # that modular filtering keeps out of PowerTools, so it cannot be
+              # installed.  The Arrow project's own repository can.  Pinned to
+              # the version EPEL 10 carries, both because that is one version
+              # fewer to have working and because the newest wants C++20, which
+              # Rocky 8's gcc 8 does not have.
+              # EPEL as well, and not only for Arrow itself: arrow-devel needs
+              # re2-devel and parquet-devel needs thrift-devel, and on EL8 both
+              # of those live in EPEL.
+              dnf install -y \
+                
https://apache.jfrog.io/artifactory/arrow/almalinux/8/apache-arrow-release-latest.rpm

Review Comment:
   After merge we need add it to building docker container, like it was with a 
PAX



##########
contrib/datalake_fdw/src/common/dl_resource.c:
##########
@@ -0,0 +1,139 @@
+/*-------------------------------------------------------------------------
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ *
+ * dl_resource.c
+ *       Cleanups that happen even when nothing calls them.

Review Comment:
   Sorry, but here we implemented linked list. Why now reuse lib/ilist.h 
instead of our own implementation?
   
   The server already ships intrusive linked lists (src/include/lib/ilist.h), 
and this exact pattern already uses them: 
contrib/pax_storage/src/cpp/comm/pax_resource.cc:35 stores a dlist_node in the 
entry struct and uses dlist_push_tail / dlist_delete. The PR's dl_resource.c 
instead hand-rolls a singly-linked list with **link splice traversal in all 
three functions.
   
   The overall code will be like
   ```
   typedef struct DlResourceEntry
   {
       dlist_node  node;
       ResourceOwner owner;
       DlResourceRelease release;
       void       *arg;
   } DlResourceEntry;
   
   static dlist_head dl_resources = DLIST_STATIC_INIT(dl_resources);
   
   /* in the callback */
   dlist_foreach_modify(iter, &dl_resources)
   {
       DlResourceEntry *entry = dlist_container(DlResourceEntry, node, 
iter.cur);
       if (entry->owner != CurrentResourceOwner)
           continue;
       if (isCommit)
           elog(WARNING, "datalake_fdw leaked a resource: %p", entry->arg);
       dlist_delete(&entry->node);
       entry->release(entry->arg);
       free(entry);
   }
   ```



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to