gongxun0928 commented on PR #1064:
URL: https://github.com/apache/cloudberry/pull/1064#issuecomment-2918803883

   > **⚠️ Design conflict: PLT optimization vs. shared backend usability**
   > 
   > This change introduces a configuration behavior shift that creates tension 
between performance optimization and user configurability.
   > 
   > **❌ Problem 1: Users can no longer link `postgres` against 
`libpostgres.so`**
   > 
   > With the current Makefile, `postgres` is always linked using object files, 
regardless of whether `--enable-shared-postgres-backend` is passed.
   > 
   > Previously, passing:
   > 
   > ```shell
   > ./configure --enable-shared-postgres-backend
   > ```
   > 
   > would result in `postgres` being linked against `libpostgres.so`. That’s 
no longer possible _without modifying the Makefile_.
   > 
   > This is a regression in usability: the user’s intent to use the shared 
library is now ignored, and the only way to restore the prior behavior is to 
manually patch the build system.
   > 
   > **✅ Clarification: Extensions like `pax` still work — `libpostgres.so` is 
built when requested**
   > 
   > To be clear: with `--enable-shared-postgres-backend`, `libpostgres.so` is 
still built, so extensions like `pax` that depend on it will function.
   > 
   > No Makefile changes are needed for that use case — only if the user wants 
`postgres` itself to link against `libpostgres.so` (which is no longer 
possible).
   > 
   > **✅ Recommended resolution**
   > 
   > 1. **Respect the existing flag behavior:**
   >    
   >    * When `--enable-shared-postgres-backend` is set, build 
`libpostgres.so` _and_ link `postgres` against it (as before)
   >    * When `--disable-shared-postgres-backend` is set, link `postgres` 
using object files
   > 2. **Optional: introduce a new flag for more explicit control**
   >    If we want to support all combinations cleanly, introduce something 
like:
   >    ```shell
   >    --link-postgres-with-shared=yes|no
   >    ```
   >    
   >    
   >        
   >          
   >        
   >    
   >          
   >        
   >    
   >        
   >      
   >    This would decouple building `libpostgres.so` from whether `postgres` 
uses it, allowing:
   >    
   >    * Extensions to build with `libpostgres.so`
   >    * `postgres` to still link using object files
   > 
   > **🎯 Summary**
   > 
   > This change removes a previously supported build path where `postgres` was 
linked with `libpostgres.so`, even when the user explicitly requests it. That 
breaks expectations and removes configurability.
   > 
   > We should either:
   > 
   > * Restore the previous behavior when `--enable-shared-postgres-backend` is 
passed
   > * Or clearly split the flags to distinguish building the shared library 
vs. using it
   
   
   
   > **⚠️ Design conflict: PLT optimization vs. shared backend usability**
   > 
   > This change introduces a configuration behavior shift that creates tension 
between performance optimization and user configurability.
   > 
   > **❌ Problem 1: Users can no longer link `postgres` against 
`libpostgres.so`**
   > 
   > With the current Makefile, `postgres` is always linked using object files, 
regardless of whether `--enable-shared-postgres-backend` is passed.
   > 
   > Previously, passing:
   > 
   > ```shell
   > ./configure --enable-shared-postgres-backend
   > ```
   > 
   > would result in `postgres` being linked against `libpostgres.so`. That’s 
no longer possible _without modifying the Makefile_.
   > 
   > This is a regression in usability: the user’s intent to use the shared 
library is now ignored, and the only way to restore the prior behavior is to 
manually patch the build system.
   > 
   > **✅ Clarification: Extensions like `pax` still work — `libpostgres.so` is 
built when requested**
   > 
   > To be clear: with `--enable-shared-postgres-backend`, `libpostgres.so` is 
still built, so extensions like `pax` that depend on it will function.
   > 
   > No Makefile changes are needed for that use case — only if the user wants 
`postgres` itself to link against `libpostgres.so` (which is no longer 
possible).
   > 
   > **✅ Recommended resolution**
   > 
   > 1. **Respect the existing flag behavior:**
   >    
   >    * When `--enable-shared-postgres-backend` is set, build 
`libpostgres.so` _and_ link `postgres` against it (as before)
   >    * When `--disable-shared-postgres-backend` is set, link `postgres` 
using object files
   > 2. **Optional: introduce a new flag for more explicit control**
   >    If we want to support all combinations cleanly, introduce something 
like:
   >    ```shell
   >    --link-postgres-with-shared=yes|no
   >    ```
   >    
   >    
   >        
   >          
   >        
   >    
   >          
   >        
   >    
   >        
   >      
   >    This would decouple building `libpostgres.so` from whether `postgres` 
uses it, allowing:
   >    
   >    * Extensions to build with `libpostgres.so`
   >    * `postgres` to still link using object files
   > 
   > **🎯 Summary**
   > 
   > This change removes a previously supported build path where `postgres` was 
linked with `libpostgres.so`, even when the user explicitly requests it. That 
breaks expectations and removes configurability.
   > 
   > We should either:
   > 
   > * Restore the previous behavior when `--enable-shared-postgres-backend` is 
passed
   > * Or clearly split the flags to distinguish building the shared library 
vs. using it
   
   thanks for the detailed feedback! 
   
   I understand the concerns and will revisit the build options. There are two 
key considerations here:
   1. restore user choice: It’s important to allow users to decide whether 
postgres should be linked against libpostgres.so or built with static object 
files.
   2. disk space constraint: When using a statically linked postgres, the 
ic-cbdb-parallel test may exceed the disk space limit of the runner container. 
   
   To address the first issue, i prefer Recommendation 2 which better meets the 
requirements but introduces complexity.  Introduce a new flag 
--link-postgres-with-shared=yes|no to explicitly control how postgres is 
linked. 
   Not Recommendation 1 (restoring the original behavior) because there are 
cases where users may need to compile the pax extension while also mitigating 
the performance impact of PLT function calls. This new flag ensures both 
scenarios are supported without conflict.
   
   Following Recommendation 2, we will support four possible combinations:
   1. `./configure --link-postgres-with-shared=yes && 
--enable-shared-postgres-backend`:
   This behavior will be identical to the previous behavior of 
--enable-shared-postgres-backend.
   
   2. `./configure --link-postgres-with-shared=no && 
--enable-shared-postgres-backend`:
   Both postgres and libpostgres.so will be built, but postgres will be 
statically linked with all object files.
   
   3. `./configure --link-postgres-with-shared=yes && 
--disable-shared-postgres-backend`:
   This combination will not be allowed, and an error message will be 
displayed. If --link-postgres-with-shared=yes is specified, libpostgres.so must 
also be built.
   
   4. `./configure  --link-postgres-with-shared=no && 
--disable-shared-postgres-backend`:
   Only postgres will be built, and it will be statically linked with all 
object files.
   
   
   
   
   
   
   


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