Re: [PR] [GLUTEN-9949][VL] Enable enhanced feature compile [incubator-gluten]

2025-06-12 Thread via GitHub


zhztheplayer commented on code in PR #9957:
URL: https://github.com/apache/incubator-gluten/pull/9957#discussion_r2142055504


##
gluten-core/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##
@@ -117,6 +117,9 @@ class GlutenConfig(conf: SQLConf) extends Logging {
   def enableExtendedColumnPruning: Boolean =
 getConf(ENABLE_EXTENDED_COLUMN_PRUNING)
 
+  def enableEnhancedFeature(): Boolean =
+System.getenv().getOrDefault("GLUTEN_ENABLE_ENHANCED_FEATURE", 
"false").toBoolean
+

Review Comment:
   Should this be defined in `GlutenConfig.scala` or in `VeloxConfig.scala`?
   
   And it feels worth it to add an individual JNI method to avoid relying on 
the env?
   
   E.g.,
   
   ```java
   native boolean isEnhancedFeaturesEnabled();
   ```
   
   More generally in the future, we may have
   
   ```java
   native String[] getEnhancedFeatures();
   ```



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



Re: [PR] [GLUTEN-9949][VL] Enable enhanced feature compile [incubator-gluten]

2025-06-12 Thread via GitHub


zhouyuan commented on code in PR #9957:
URL: https://github.com/apache/incubator-gluten/pull/9957#discussion_r2142190563


##
.github/workflows/velox_backend_cache.yml:
##
@@ -106,6 +106,34 @@ jobs:
   path: '${{ env.CCACHE_DIR }}'
   key: ccache-centos8-release-shared-${{runner.arch}}-${{github.sha}}
 
+  cache-enhanced-shared-lib-centos-8:
+runs-on: ${{ matrix.os }}
+strategy:
+  matrix:
+os: [ ubuntu-22.04 ]
+container: apache/gluten:centos-8-jdk8
+steps:
+  - uses: actions/checkout@v2
+  - name: Get Ccache
+uses: actions/cache/restore@v3
+with:
+  path: '${{ env.CCACHE_DIR }}'
+  key: 
ccache-enhanced-centos8-release-shared-${{runner.arch}}-${{github.sha}}

Review Comment:
   could we reuse the centos8-shared cache? by default the cache size for each 
project is relatively small(10G)



##
ep/build-velox/src/get_velox.sh:
##
@@ -20,17 +20,22 @@ VELOX_REPO=https://github.com/oap-project/velox.git
 VELOX_BRANCH=2025_06_11
 VELOX_HOME=""
 RUN_SETUP_SCRIPT=ON
+VELOX_ENHANCED_REPO=https://github.com/oap-project/velox.git
+VELOX_ENHANCED_BRANCH=2025_06_10

Review Comment:
   since new CI is added for this feature and sometimes the gluten:main code is 
tightly coupled with velox rebase branch. so on each rebase we may need to take 
care of these two branches 



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



Re: [PR] [GLUTEN-9949][VL] Enable enhanced feature compile [incubator-gluten]

2025-06-12 Thread via GitHub


PHILO-HE commented on code in PR #9957:
URL: https://github.com/apache/incubator-gluten/pull/9957#discussion_r2142017389


##
dev/builddeps-veloxbe.sh:
##
@@ -121,6 +122,10 @@ do
 ENABLE_GPU=("${arg#*=}")
 shift # Remove argument name from processing
 ;;
+--enable_enhanced_feature=*)

Review Comment:
   Name suggestion: `xxx_features`, to imply multiple features are being 
covered or will be covered. Ditto for other places.



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



Re: [PR] [GLUTEN-9949][VL] Enable enhanced feature compile [incubator-gluten]

2025-06-12 Thread via GitHub


PHILO-HE commented on code in PR #9957:
URL: https://github.com/apache/incubator-gluten/pull/9957#discussion_r2142017389


##
dev/builddeps-veloxbe.sh:
##
@@ -121,6 +122,10 @@ do
 ENABLE_GPU=("${arg#*=}")
 shift # Remove argument name from processing
 ;;
+--enable_enhanced_feature=*)

Review Comment:
   Name suggestion: xxx_features, to imply multiple features are being covered 
or will be covered. Ditto for other places.



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



Re: [PR] [GLUTEN-9949][VL] Enable enhanced feature compile [incubator-gluten]

2025-06-12 Thread via GitHub


jinchengchenghh commented on code in PR #9957:
URL: https://github.com/apache/incubator-gluten/pull/9957#discussion_r2142132867


##
gluten-core/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##
@@ -117,6 +117,9 @@ class GlutenConfig(conf: SQLConf) extends Logging {
   def enableExtendedColumnPruning: Boolean =
 getConf(ENABLE_EXTENDED_COLUMN_PRUNING)
 
+  def enableEnhancedFeature(): Boolean =
+System.getenv().getOrDefault("GLUTEN_ENABLE_ENHANCED_FEATURE", 
"false").toBoolean
+

Review Comment:
   The VeloxConfig.scala is in backends-velox module, but gluten-iceberg only 
relies on gluten-substrait, gluten-iceberg should can access this config, so I 
place it in GlutenConfig.scala
   JNI call is also ok, I will change to it.



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



Re: [PR] [GLUTEN-9949][VL] Enable enhanced feature compile [incubator-gluten]

2025-06-12 Thread via GitHub


github-actions[bot] commented on PR #9957:
URL: 
https://github.com/apache/incubator-gluten/pull/9957#issuecomment-296568

   Run Gluten Clickhouse CI on x86


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



Re: [PR] [GLUTEN-9949][VL] Enable enhanced feature compile [incubator-gluten]

2025-06-12 Thread via GitHub


zhztheplayer commented on code in PR #9957:
URL: https://github.com/apache/incubator-gluten/pull/9957#discussion_r2142055504


##
gluten-core/src/main/scala/org/apache/gluten/config/GlutenConfig.scala:
##
@@ -117,6 +117,9 @@ class GlutenConfig(conf: SQLConf) extends Logging {
   def enableExtendedColumnPruning: Boolean =
 getConf(ENABLE_EXTENDED_COLUMN_PRUNING)
 
+  def enableEnhancedFeature(): Boolean =
+System.getenv().getOrDefault("GLUTEN_ENABLE_ENHANCED_FEATURE", 
"false").toBoolean
+

Review Comment:
   Should this be defined in `GlutenConfig.scala` or in `VeloxConfig.scala`?
   
   And it feels worth it to add an individual JNI method to avoid relying on 
the env?
   
   E.g.,
   
   ```java
   native boolean isEnhancedFeaturesEnabled();
   ```
   
   More generally in the future, we may have
   
   ```java
   native String[] getEnabledEnhancedFeatures();
   ```



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



Re: [PR] [GLUTEN-9949][VL] Enable enhanced feature compile [incubator-gluten]

2025-06-12 Thread via GitHub


github-actions[bot] commented on PR #9957:
URL: 
https://github.com/apache/incubator-gluten/pull/9957#issuecomment-2965853148

   Run Gluten Clickhouse CI on x86


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