Add GPU support for try_variant_get [databricks] - #15370
Conversation
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
5b078ef to
a1c47cb
Compare
|
build |
Greptile SummaryThis PR adds GPU acceleration for Spark 4.x
Confidence Score: 5/5Safe to merge; all findings are style/rule-conformance issues that do not affect correctness under normal conditions. The core extraction logic is well-structured, resource management is correct in all practically reachable paths, and the integration tests cover the full range of supported/unsupported cases. The bare .close() calls in the accumulator loops are a rule-conformance issue rather than a real leak risk since cuDF close() does not throw, and the missing shim entries in the test annotation are a coverage gap with no functional impact. Files Needing Attention: GpuVariantGet.scala (allRowsCovered and normalizeIntegers accumulator loops); GpuColumnVectorVariantSuite.scala (shim annotation missing 404 and 413) Important Files Changed
Sequence DiagramsequenceDiagram
participant PR as GpuProjectExec
participant VG as GpuVariantGet.doColumnar
participant CU as VariantUtils (cuDF)
participant BR as GpuCpuBridgeExpression
PR->>VG: "input GpuColumnVector (VariantType STRUCT)"
VG->>VG: "build cuDF view [metadata, value]"
VG->>CU: "getVariantFieldValue(cudfVariant, path)"
CU-->>VG: "rawValue ColumnView"
alt "all rows GPU-decodable"
VG->>CU: "castVariantValue(rawValue, targetDType)"
CU-->>VG: "decoded ColumnVector"
VG->>VG: "normalizeIntegers / narrowInteger"
VG-->>PR: "result ColumnVector"
else "coercion required"
VG->>BR: "columnarEval(batch)"
BR-->>VG: "CPU result ColumnVector"
VG-->>PR: "result ColumnVector"
end
Reviews (9): Last reviewed commit: "Merge branch 'main' into variant-extract..." | Re-trigger Greptile |
| def isVariantCudfAvailable: Boolean = { | ||
| try { | ||
| val variantUtils = Class.forName("ai.rapids.cudf.VariantUtils", true, | ||
| Thread.currentThread().getContextClassLoader) | ||
| variantUtils.getMethod("getVariantFieldValue", classOf[ColumnView], classOf[String]) | ||
| variantUtils.getMethod("castVariantValue", classOf[ColumnView], classOf[DType]) | ||
| variantUtils.getMethod("extractVariantField", classOf[ColumnView], classOf[String], | ||
| classOf[DType]) | ||
| true | ||
| } catch { | ||
| case _: ClassNotFoundException | _: NoSuchMethodException | _: LinkageError => false | ||
| } | ||
| } |
There was a problem hiding this comment.
Availability check validates unused methods
isVariantCudfAvailable verifies the existence of getVariantFieldValue and castVariantValue, but neither method is called anywhere in this file — only extractVariantField is actually invoked at runtime. If a future cuDF release renames or removes those two helper methods while keeping extractVariantField, the entire feature silently falls back to CPU even though the required API is present. The guard should only check the method the code actually calls.
| private def isVariantPhysicalType(fileType: Type): Boolean = { | ||
| if (fileType.isPrimitive || fileType.asGroupType().getFieldCount != 2) { | ||
| false | ||
| } else { | ||
| val groupType = fileType.asGroupType() | ||
| Seq("value", "metadata").zipWithIndex.forall { case (name, index) => | ||
| val field = groupType.getType(index) | ||
| field.getName == name && | ||
| field.isRepetition(Type.Repetition.REQUIRED) && | ||
| field.isPrimitive && | ||
| field.asPrimitiveType().getPrimitiveTypeName == PrimitiveTypeName.BINARY | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Strict
REQUIRED check may reject valid Variant files
isVariantPhysicalType rejects any Parquet file where value or metadata is encoded with OPTIONAL repetition. Non-Spark Variant writers (e.g., Delta Lake connectors, Arrow-based tools) may write these fields as OPTIONAL BINARY — still spec-compliant Variant — which would fail this check and cause an incompatibility error or unexpected CPU fallback. Consider accepting OPTIONAL repetition as well, or at minimum document why REQUIRED is the only valid encoding here.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
f172901 to
a1c47cb
Compare
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
…pport Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
|
NOTE: release/26.08 has been created from main. Please retarget your PR to release/26.08 if it should be included in the release. |
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
…pport Signed-off-by: Niranjan Artal <nartal@nvidia.com>
Signed-off-by: Niranjan Artal <nartal@nvidia.com>
|
build |
|
@greptile-apps review the PR again. |
|
build |
Contributes to #15220 and #15221.
Description
This PR adds GPU support for Spark 4.x Variant extraction from Parquet reads.
It accelerates non-strict
try_variant_getwhen:$.fieldor$.nested.field.tinyint,smallint,int,bigint, orstring.The implementation uses the cuDF Java
VariantUtils.getVariantFieldValueandVariantUtils.castVariantValueAPIs added in rapidsai/cudf#23069. Exact string and signed-integer Variant values are decoded on GPU. Batches containing values that require Spark coercion semantics, such as string-to-integer conversion, are evaluated through the CPU bridge to preserve Spark-compatible results.Strict
variant_get, non-literal paths, array paths, quoted-key paths, and unsupported target types continue to fall back to CPU. Variant writes remain unsupported on GPU.Changes
try_variant_getacross Spark 4.0+ shims, including Databricks 17.3.tinyint,smallint,int,bigint, andstringtargets.LIST<UINT8>andSTRINGVariant children during host conversion.Configuration
Variant extraction is currently registered as an incompatible operation and requires:
The CPU bridge used for Spark-compatible runtime coercions is enabled by default.
Testing
variant_test.py: 31 passed.400db173) build and focused tests: passed.Performance
Tested on a Tesla T4 with OSS Spark 4.0.0 using 10 million rows, 128 Parquet partitions,
local[4], and 6 GB driver memory. Each result is the average of three measured runs after one warm-up run.$.idasBIGINT$.nameasSTRINGProjection queries used Spark's
noopsink.Benchmark queries
A nested integer filter followed by
COUNT(*)achieved a 1.51x speedup. DirectSUM(try_variant_get(...))did not improve performance because its partial aggregate remains on CPU when the input schema contains the rawVariantType; the Variant extraction and final aggregate still run on GPU.Checklists
Documentation
Testing
Performance