Re: [PR] Feature: Apache Iceberg lake tables as an extension (datalake_fdw skeleton) [cloudberry]

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

MisterRaindrop commented on code in PR #1842:
URL: https://github.com/apache/cloudberry/pull/1842#discussion_r3900092804


##########
.github/workflows/build-cloudberry.yml:
##########
@@ -326,6 +326,10 @@ jobs:
                                "contrib/pg_buffercache:installcheck",
                                "contrib/sslinfo:installcheck"]
               },
+              {"test":"ic-datalake-fdw",
+               "make_configs":["contrib/datalake_fdw:installcheck"],
+               "shared_preload_libraries":"datalake_fdw"

Review Comment:
   Done in a58aec79f14 — datalake_fdw is now built and tested the way anyone outside the project would consume it, rather than compiled into the server we ship.
   
   - `configure-cloudberry.sh` no longer passes `--enable-datalake-fdw`, so the RPM every test job installs is unchanged by this module. `configure-cloudberry.sh` has therefore dropped out of this PR's diff entirely.
   - The test job gained a step that, for a matrix entry naming a `pgxs_extension`, builds and installs it with PGXS against the server that was just installed. It has to run before the demo cluster is created: a module named in `shared_preload_libraries` must exist by the time the cluster starts.
   - `--enable-datalake-fdw` itself stays, for people building from source who do want it in-tree.
   
   I validated it on a real run before bringing it here (fork, `workflow_dispatch`, `ic-datalake-fdw` only):
   
   - the build job's log shows `checking whether to build with datalake_fdw support ... no`, and zero compilation of the module — the RPM genuinely does not contain it;
   - the new step compiled and installed it in 2.5s (`datalake_fdw.so`, `.control`, `--1.0.sql`);
   - the cluster started with it preloaded and the regression passed 3/3.
   
   One thing to weigh, since it is a real cost rather than a detail: **the in-tree path now has no CI coverage at all.** The `contrib/Makefile` conditional and the configure option feeding it are no longer built by anything automated — building from source by hand is the only thing that exercises them. That was previously verified precisely because `ic-datalake-fdw` was green on the RPM build.
   
   @tuhaihe you had said the previous shape looked good and matched PAX, which it did — every optional module here (`pax`, `gpcloud`, `orafce`, `mapreduce`, `ic-proxy`) is `no` by default in `configure.ac` and switched on in `configure-cloudberry.sh`. This change deliberately steps away from that pattern in favour of exercising the extension path instead. Happy to go back to the PAX shape if you'd rather keep the in-tree wiring covered — the previous commit is still in this branch's history.



-- 
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: commits-unsubscribe@cloudberry.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org