[DOC] Polish wording and fix inaccuracies in docs - #12675
Open
philo-he wants to merge 7 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR polishes wording and updates technical notes across the Velox documentation, aiming to improve clarity and correct a few support-status details. It also includes a small change to the release build script.
Changes:
- Refines phrasing/formatting in multiple Velox docs (getting-started, CI, limitations, memory profiling).
- Updates Velox scalar function support tables to mark additional functions as supported.
- Extends the release build script to include Spark 4.1.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adjusts top-of-page presentation and wording in the project README. |
| docs/velox-backend-scalar-function-support.md | Marks additional scalar functions as supported in Velox. |
| docs/velox-backend-limitations.md | Rewords the map-hash-key limitation section and removes older limitation text. |
| docs/get-started/VeloxLocalCache.md | Clarifies and expands configuration guidance for Velox local cache. |
| docs/get-started/Velox.md | Polishes build/setup wording and a few explanatory sections. |
| docs/developers/velox-backend-CI.md | Improves clarity around CI Docker images and caching behavior. |
| docs/developers/ProfileMemoryOfGlutenWithVelox.md | Improves jemalloc profiling example and adds explanatory note. |
| dev/release/build-release.sh | Adds Spark 4.1 to the release build matrix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1
to
4
| <img src="docs/image/gluten-logo.svg" alt="Gluten" width="260"> | ||
|
|
||
| # Apache Gluten | ||
|
|
||
| **A Middle Layer for Offloading JVM-based SQL Engines' Execution to Native Engines** |
Member
Author
There was a problem hiding this comment.
It's redundant with the logo. I also noticed that the Velox project uses its logo without a title as well, so perhaps we can keep this removal.
| The header option should be true. And now we only support DatasourceV1, i.e., user should set `spark.sql.sources.useV1SourceList=csv`. User defined read option is not supported, which will make CSV read fall back to vanilla Spark in most case. | ||
| CSV read will also fall back to vanilla Spark and log warning when user specifies schema is different with file schema. | ||
|
|
||
| ### Utilizing Map Type as Hash Keys in ColumnarShuffleExchange |
Member
Author
There was a problem hiding this comment.
Arrow based CSV reader has been removed. So we can remove it.
| spark.gluten.sql.columnar.backend.velox.ssdCacheSize // SSD cache size. Memory-only caching is used when set to 0. Default: 128MB. | ||
| spark.gluten.sql.columnar.backend.velox.ssdCacheShards // Number of SSD cache shards. Default: 1. | ||
| spark.gluten.sql.columnar.backend.velox.ssdCacheIOThreads // Number of IO threads for SSD cache read/write. Enables read-ahead when > 1. Default: 4. | ||
| spark.gluten.sql.columnar.backend.velox.loadQuantum // Load quantum size. Must be set to 8MB when Velox cache is enabled, otherwise Velox fails. Default: 256MB. |
Comment on lines
65
to
68
| for spark_version in 4.0 4.1 | ||
| do | ||
| ${GLUTEN_HOME}/build/mvn clean install -Pjava-17 -Pscala-2.13 -Pbackends-velox -Pspark-${spark_version} -Piceberg,paimon -DskipTests | ||
| done |
philo-he
changed the title
Member
Author
|
@zhouyuan, could you help review this PR? |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
README.md:4
- The README no longer has a top-level H1 (the
# Apache Glutenline was removed). This typically hurts GitHub rendering/SEO and makes the document structure less accessible (screen readers and TOCs rely on headings). Consider restoring a single H1 header near the top (e.g., directly below the logo) and keeping the tagline as supporting text.
<img src="docs/image/gluten-logo.svg" alt="Gluten" width="260">
**A Middle Layer for Offloading JVM-based SQL Engines' Execution to Native Engines**
docs/developers/velox-backend-CI.md:21
- The text says Dockerfiles are under
dev/docker/, but the table mixes bare filenames (e.g.,Dockerfile.centos8-dynamic-build) with a relative subdir (cudf/Dockerfile). This is inconsistent and can be confusing for readers trying to locate the files. Consider listing full repo-relative paths consistently (e.g.,dev/docker/Dockerfile.centos8-dynamic-buildanddev/docker/cudf/Dockerfile) and formatting file paths as inline code.
A weekly job defined in `docker_image.yml` builds the Docker images used for CI verification. The Dockerfiles (under `dev/docker/`) and their corresponding images are listed below:
file | images | comments
-- | -- | --
Dockerfile.centos7-gcc13-static-build | apache/gluten:vcpkg-centos-7-gcc13 | centos 7, static link, jdk8
Dockerfile.centos8-gcc13-static-build | apache/gluten:vcpkg-centos-8-gcc13 | centos 8, static link, jdk8
Dockerfile.centos8-dynamic-build | apache/gluten:centos-8-jdk8 | centos 8, dynamic link, jdk8
Dockerfile.centos8-dynamic-build | apache/gluten:centos-8-jdk11 | centos 8, dynamic link, jdk11
Dockerfile.centos8-dynamic-build | apache/gluten:centos-8-jdk17 | centos 8, dynamic link, jdk17
cudf/Dockerfile | apache/gluten:centos-9-jdk8-cudf | centos 9, dynamic link, jdk8
docs/get-started/VeloxLocalCache.md:26
- Hyphenate the compound modifier: change 'UUID based suffix' to 'UUID-based suffix'.
to "spark.gluten.sql.columnar.backend.velox.ssdCachePath", with UUID based suffix,
e.g. "/tmp/cache.13e8ab65-3af4-46ac-8d28-ff99b2a9ec9b0". Gluten cannot reuse older caches for now, and the old cache files
docs/get-started/VeloxLocalCache.md:20
- These comments make strong version-dependent claims ('Velox fails', 'currently causes SSD cache writes to fail') but don’t provide a direct reference (the Velox issue isn’t linked) or scope (which Velox/Gluten versions are affected). To keep the docs accurate over time, please link to the referenced issue and, if possible, qualify the statements with the affected versions/conditions (or point to a configuration troubleshooting section) so readers can assess applicability.
spark.gluten.sql.columnar.backend.velox.loadQuantum // Load quantum size. Must be at most 8MB when Velox cache is enabled, otherwise Velox fails. Default: 256MB.
spark.gluten.sql.columnar.backend.velox.ssdODirect // Enable O_DIRECT on cache write. Currently causes SSD cache writes to fail if enabled; see Velox issue #10597. Default: false.
Comment on lines
145
to
+149
| Only reading with INT96 and dictionary encoding is supported. When reading INT64 represented millisecond/microsecond timestamps, or INT96 represented timestamps of other encodings, exceptions can occur. | ||
|
|
||
| - Complex types | ||
| - Parquet scan of nested array with struct or array as element type is not supported in Velox (fallback behavior). | ||
| - Parquet scan of nested map with struct as key type, or array type as value type is not supported in Velox (fallback behavior). | ||
|
|
||
| ### CSV Read | ||
| The header option should be true. And now we only support DatasourceV1, i.e., user should set `spark.sql.sources.useV1SourceList=csv`. User defined read option is not supported, which will make CSV read fall back to vanilla Spark in most case. | ||
| CSV read will also fall back to vanilla Spark and log warning when user specifies schema is different with file schema. | ||
|
|
||
| ### Utilizing Map Type as Hash Keys in ColumnarShuffleExchange | ||
| Spark uses the `spark.sql.legacy.allowHashOnMapType` configuration to support hash map key functions. | ||
| Gluten enables this configuration during the creation of ColumnarShuffleExchange, as shown in the code [link](https://github.com/apache/gluten/blob/0dacac84d3bf3d2759a5dd7e0735147852d2845d/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala#L355-L363). | ||
| This method bypasses Spark's unresolved checks and creates projects with the hash(mapType) operator before ColumnarShuffleExchange. | ||
| However, if `spark.sql.legacy.allowHashOnMapType` is disabled in a test environment, projects using the hash(mapType) expression may throw an `Invalid call to dataType on unresolved object` exception during validation, causing them to fallback to vanilla Spark, as referenced in the code [link](https://github.com/apache/spark/blob/de5fa426e23b84fc3c2bddeabcd2e1eda515abd5/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/hash.scala#L291-L296). | ||
| Enabling this configuration allows the project to be offloaded to Velox. | ||
| Spark's `spark.sql.legacy.allowHashOnMapType` configuration controls whether hashing is allowed on map-type keys. | ||
| Gluten enables this configuration when creating `ColumnarShuffleExchange`, as shown [here](https://github.com/apache/gluten/blob/0dacac84d3bf3d2759a5dd7e0735147852d2845d/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala#L355-L363). |
Member
Author
There was a problem hiding this comment.
Just updated the PR description, this "CSV Read" section is not valid now because the CSV reader has been removed from the code.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (5)
docs/velox-backend-limitations.md:147
- This change removes the prior "Complex types" Parquet-scan limitation bullets entirely, but the PR description only calls out removing the CSV reader section. If the complex-type limitations still apply, they should be kept here (or moved elsewhere with a link) so users don't lose that guidance.
### Utilizing Map Type as Hash Keys in ColumnarShuffleExchange
dev/release/build-release.sh:67
- This PR is titled "[DOC] ..." but this hunk changes the release build matrix by adding Spark 4.1 builds. Since it changes release behavior (not just documentation), consider either (a) splitting this script change into a separate PR, or (b) updating the PR title/scope to reflect the non-doc change.
for spark_version in 4.0 4.1
do
${GLUTEN_HOME}/build/mvn clean install -Pjava-17 -Pscala-2.13 -Pbackends-velox -Pspark-${spark_version} -Piceberg,paimon -DskipTests
docs/velox-backend-scalar-function-support.md:214
map_from_arraysis listed as fully supported ("S"), but the Velox plan validator explicitly blacklistsmap_from_arraysas unsupported (seecpp/velox/substrait/SubstraitToVeloxPlanValidator.ccwhere it returns false for names inkBlackList). This table entry should not be marked as supported for the Velox backend.
| map_from_arrays | MapFromArrays | S | |
docs/developers/ProfileMemoryOfGlutenWithVelox.md:116
- The doc first suggests distributing
libjemalloc.sovia--files/spark.files, which places it in the executor working directory, but this example uses an absolute/path/to/...forLD_PRELOAD. That can be misleading for users following the--filesapproach; use a relative./libjemalloc.so(or clarify that jemalloc must be preinstalled on every node).
spark.executorEnv.LD_PRELOAD /path/to/jemalloc/libjemalloc.so
docs/developers/velox-backend-CI.md:21
- The Dockerfile column is now a mix of bare filenames (e.g.
Dockerfile.centos8-dynamic-build) and a subdirectory path (cudf/Dockerfile). Since these are meant to be referenced from the repo root, it's clearer and more consistent to use full relative paths (as the previous version did).
Dockerfile.centos8-gcc13-static-build | apache/gluten:vcpkg-centos-8-gcc13 | centos 8, static link, jdk8
Dockerfile.centos8-dynamic-build | apache/gluten:centos-8-jdk8 | centos 8, dynamic link, jdk8
Dockerfile.centos8-dynamic-build | apache/gluten:centos-8-jdk11 | centos 8, dynamic link, jdk11
Dockerfile.centos8-dynamic-build | apache/gluten:centos-8-jdk17 | centos 8, dynamic link, jdk17
cudf/Dockerfile | apache/gluten:centos-9-jdk8-cudf | centos 9, dynamic link, jdk8
philo-he
force-pushed
the
update-doc-1.7-release
branch
from
August 4, 2026 05:39
2f7cf76 to
5e3c934
Compare
Member
Author
|
Also cc @zhztheplayer, @jackylee-ch for a review if you are available. Thanks! |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (5)
dev/release/build-release.sh:67
- This PR is titled as documentation-only ([DOC] ...), but this hunk changes release build behavior by adding Spark 4.1 to the release build matrix. To avoid misleading scope/signaling, either split this script change into a separate PR or update the PR title to reflect the non-doc release/build change.
for spark_version in 4.0 4.1
do
${GLUTEN_HOME}/build/mvn clean install -Pjava-17 -Pscala-2.13 -Pbackends-velox -Pspark-${spark_version} -Piceberg,paimon -DskipTests
docs/velox-backend-scalar-function-support.md:214
map_from_arraysis marked as fully supported (S), but the Velox plan validator explicitly rejects it:cpp/velox/substrait/SubstraitToVeloxPlanValidator.ccincludesmap_from_arraysinkBlackListand fails validation when encountered. This doc entry should not indicate full support until the validator (and backend) actually support offloading it.
| map_from_arrays | MapFromArrays | S | |
docs/get-started/VeloxLocalCache.md:19
- The
loadQuantumnote says "otherwise Velox fails", but the actual failure mode is Gluten throwing anIllegalArgumentExceptionduring listener initialization whencacheEnabled=trueandloadQuantum > 8MB(seebackends-velox/.../VeloxListenerApi.scala). Consider wording this as a clear configuration constraint and fail-fast behavior to avoid implying a Velox runtime crash.
spark.gluten.sql.columnar.backend.velox.loadQuantum // Load quantum size. Must be at most 8MB when Velox cache is enabled, otherwise Velox fails. Default: 256MB.
docs/get-started/VeloxLocalCache.md:26
- The config key is currently wrapped in quotes, which renders as plain text and is easy to miss in Markdown. Prefer backticks for configuration keys; also consider hyphenating "UUID-based".
It's recommended to mount SSDs to the cache path to get the best performance of local caching. Cache files will be written
to "spark.gluten.sql.columnar.backend.velox.ssdCachePath", with UUID based suffix,
e.g. "/tmp/cache.13e8ab65-3af4-46ac-8d28-ff99b2a9ec9b0". Gluten cannot reuse older caches for now, and the old cache files
docs/get-started/Velox.md:21
- This paragraph uses "Ubuntu 20.04/22.04/CentOS 7/8", but the "Supported Version" table above still uses "Ubuntu20.04/22.04, Centos7/8". Align the table row with the updated naming/capitalization to keep the doc consistent.
Currently, the statically built Gluten+Velox backend supports all Linux OSes but is only tested on **Ubuntu 20.04/22.04/CentOS 7/8**. The dynamically built backend supports **Ubuntu 20.04/22.04/CentOS 7/8** and their variants.
jackylee-ch
reviewed
Aug 4, 2026
Contributor
There was a problem hiding this comment.
The supported functions count should also been changed.
Member
Author
There was a problem hiding this comment.
Yes, just updated. Thanks.
| for spark_version in 4.0 | ||
| for spark_version in 4.0 4.1 | ||
| do | ||
| ${GLUTEN_HOME}/build/mvn clean install -Pjava-17 -Pscala-2.13 -Pbackends-velox -Pspark-${spark_version} -Piceberg,paimon -DskipTests |
Contributor
There was a problem hiding this comment.
Do we support Paimon in Spark 4.1? If not, we may need remove it for Spark 4.1
Member
Author
There was a problem hiding this comment.
Yes, paimon doesn't support 4.1 (see the following link). The build will fail if both profile spark-4.1 and paimon are used. I just excluded this change from this PR and will create a separate one. Then, this PR will only focus on document update. Thanks.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (3)
docs/velox-backend-limitations.md:149
- This section removal drops the prior “Complex types” Parquet-scan limitations along with the CSV note. The PR description only mentions removing the CSV section, so this looks like an unintentional documentation loss unless there is evidence the complex-type limitations are no longer applicable. Please either restore the complex-type limitation bullets or add a short note/link explaining where this limitation is documented now (and update PR description if the removal is intentional).
Only reading with INT96 and dictionary encoding is supported. When reading INT64 represented millisecond/microsecond timestamps, or INT96 represented timestamps of other encodings, exceptions can occur.
### Utilizing Map Type as Hash Keys in ColumnarShuffleExchange
Spark's `spark.sql.legacy.allowHashOnMapType` configuration controls whether hashing is allowed on map-type keys.
Gluten enables this configuration when creating `ColumnarShuffleExchange`, as shown [here](https://github.com/apache/gluten/blob/0dacac84d3bf3d2759a5dd7e0735147852d2845d/backends-velox/src/main/scala/org/apache/gluten/backendsapi/velox/VeloxSparkPlanExecApi.scala#L355-L363).
docs/get-started/VeloxLocalCache.md:16
- The documented defaults for Velox local cache sizes don’t match the actual config defaults in code.
memCacheSizeandssdCacheSizedefault to 1GB inVeloxConfig.scala, not 128MB, so readers will be misled when sizing caches.
spark.gluten.sql.columnar.backend.velox.memCacheSize // In-memory cache size. Default: 128MB.
spark.gluten.sql.columnar.backend.velox.ssdCachePath // Folder to store cache files, preferably on SSD. Default: "/tmp".
spark.gluten.sql.columnar.backend.velox.ssdCacheSize // SSD cache size. Memory-only caching is used when set to 0. Default: 128MB.
docs/developers/ProfileMemoryOfGlutenWithVelox.md:116
- This example recommends distributing
libjemalloc.sovia--files/spark.files, butLD_PRELOADis set to an absolute path. With--files, Spark localizes the file into the working directory, so the more reliable setting here is to preload the localized filename (e.g.,./libjemalloc.so) unless you require jemalloc to be preinstalled at the same path on every executor host.
spark.executorEnv.LD_PRELOAD /path/to/jemalloc/libjemalloc.so
jackylee-ch
reviewed
Aug 5, 2026
| | map_contains_key | MapContainsKey | S | | | ||
| | map_entries | MapEntries | S | | | ||
| | map_from_arrays | MapFromArrays | | | | ||
| | map_from_arrays | MapFromArrays | S | | |
Contributor
There was a problem hiding this comment.
the map_from_arrays is still in the blacklists, so we need a extra pr, like #10058, to support it?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes are proposed in this pull request?
This PR is targeted to main branch. I will create a backport PR to 1.7 branch.
Removed
CSV Readsection fromvelox-backend-limitations.md, which is not required since Arrow-based CSV reader has been removed.How was this patch tested?
N/A
Was this patch authored or co-authored using generative AI tooling?
Co-authored by Claude Sonnet 5, used to polish the phrasing in these documents.