-
Notifications
You must be signed in to change notification settings - Fork 303
Adding support for DML writes to DV enabled tables #15869
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1300,10 +1300,24 @@ case class DeltaParquetTableReader( | |
| override protected lazy val resources: Seq[AutoCloseable] = | ||
| Seq(reader) ++ buffers ++ dvInfos.map(_.serializedBitmap) | ||
|
|
||
| private val rowIndexColumn = readDataSchema.fieldNames.indexOf(ROW_INDEX_COLUMN_NAME) | ||
|
|
||
| override protected def postProcessChunk(chunk: Table): Table = { | ||
| // The cuDF reader prepends an extra index column in the output table. | ||
| // We need to drop it before returning as we don't use it. | ||
| RapidsDeletionVectors.dropFirstColumn(chunk) | ||
| // Keep the prepended cuDF physical index through schema evolution when Delta requests it. | ||
| if (rowIndexColumn >= 0) chunk else RapidsDeletionVectors.dropFirstColumn(chunk) | ||
| } | ||
|
|
||
| override protected def evolveSchemaAndClose(table: Table): Table = { | ||
| if (rowIndexColumn < 0) { | ||
| super.evolveSchemaAndClose(table) | ||
| } else { | ||
| withResource(table.getColumn(0).castTo(DType.INT64)) { physicalRowIndex => | ||
| val dataTable = RapidsDeletionVectors.dropFirstColumn(table) | ||
| val evolvedTable = super.evolveSchemaAndClose(dataTable) | ||
| RapidsDeletionVectors.replaceColumnAndClose( | ||
| evolvedTable, rowIndexColumn, physicalRowIndex) | ||
| } | ||
|
Comment on lines
+1314
to
+1319
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block looks more complicated than it should. IIUC, |
||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1365,24 +1379,34 @@ object MakeParquetTableWithDVProducer extends Logging { | |
| } | ||
| } | ||
| } | ||
| // The cuDF reader prepends an extra index column in the output table. | ||
| // We need to drop it before returning as we don't use it. | ||
| val tableWithoutIndex = RapidsDeletionVectors.dropFirstColumn(table) | ||
| closeOnExcept(tableWithoutIndex) { _ => | ||
| GpuParquetScan.throwIfRebaseNeededInExceptionMode(tableWithoutIndex, dateRebaseMode, | ||
| timestampRebaseMode) | ||
| if (readDataSchema.length < tableWithoutIndex.getNumberOfColumns) { | ||
| throw new QueryExecutionException(s"Expected ${readDataSchema.length} columns " + | ||
| s"but read ${tableWithoutIndex.getNumberOfColumns} from ${splits.mkString("; ")}") | ||
| // Preserve cuDF physical row indexes only for Delta internal row-index scans. | ||
| val rowIndexColumn = readDataSchema.fieldNames.indexOf(ROW_INDEX_COLUMN_NAME) | ||
| val physicalRowIndex = if (rowIndexColumn >= 0) { | ||
| Some(table.getColumn(0).castTo(DType.INT64)) | ||
| } else { | ||
| None | ||
| } | ||
| withResource(physicalRowIndex) { _ => | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this close the |
||
| val tableWithoutIndex = RapidsDeletionVectors.dropFirstColumn(table) | ||
| closeOnExcept(tableWithoutIndex) { _ => | ||
| GpuParquetScan.throwIfRebaseNeededInExceptionMode(tableWithoutIndex, dateRebaseMode, | ||
| timestampRebaseMode) | ||
| if (readDataSchema.length < tableWithoutIndex.getNumberOfColumns) { | ||
| throw new QueryExecutionException(s"Expected ${readDataSchema.length} columns " + | ||
| s"but read ${tableWithoutIndex.getNumberOfColumns} from ${splits.mkString("; ")}") | ||
| } | ||
| } | ||
| metrics(NUM_OUTPUT_BATCHES) += 1 | ||
| val evolvedSchemaTable = ParquetSchemaUtils.evolveSchemaIfNeededAndClose(tableWithoutIndex, | ||
| clippedParquetSchema, readDataSchema, isSchemaCaseSensitive, useFieldId) | ||
| val tableWithRowIndex = physicalRowIndex.map { index => | ||
| RapidsDeletionVectors.replaceColumnAndClose(evolvedSchemaTable, rowIndexColumn, index) | ||
| }.getOrElse(evolvedSchemaTable) | ||
| val outputTable = GpuParquetScan.rebaseDateTime(tableWithRowIndex, dateRebaseMode, | ||
| timestampRebaseMode) | ||
| GpuMetric.recordOutputBatchBytes(outputTable, metrics.get(GPU_OUTPUT_BATCH_BYTES)) | ||
| new SingleGpuDataProducer(outputTable) | ||
| } | ||
| metrics(NUM_OUTPUT_BATCHES) += 1 | ||
| val evolvedSchemaTable = ParquetSchemaUtils.evolveSchemaIfNeededAndClose(tableWithoutIndex, | ||
| clippedParquetSchema, readDataSchema, isSchemaCaseSensitive, useFieldId) | ||
| val outputTable = GpuParquetScan.rebaseDateTime(evolvedSchemaTable, dateRebaseMode, | ||
| timestampRebaseMode) | ||
| GpuMetric.recordOutputBatchBytes(outputTable, metrics.get(GPU_OUTPUT_BATCH_BYTES)) | ||
| new SingleGpuDataProducer(outputTable) | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -237,6 +237,23 @@ object RapidsDeletionVectors extends Logging { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Replaces one column in a table and consumes the input table. The returned table owns | ||
| * references to the replacement and all unchanged columns. | ||
| */ | ||
| def replaceColumnAndClose( | ||
| table: Table, | ||
| outputColumn: Int, | ||
| replacement: ColumnVector): Table = { | ||
| require(outputColumn >= 0 && outputColumn < table.getNumberOfColumns, | ||
| "Invalid replacement column position") | ||
| withResource(table) { input => | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this close every column in the table as well, which shouldn't be? |
||
| val outputColumns = (0 until input.getNumberOfColumns).map(input.getColumn).toArray | ||
| outputColumns(outputColumn) = replacement | ||
| new Table(outputColumns: _*) | ||
| } | ||
| } | ||
|
|
||
| def isIfNotContainedRowIndexFilter(filterTypeOpt: Option[RowIndexFilterType]): Boolean = { | ||
| filterTypeOpt.contains(RowIndexFilterType.IF_NOT_CONTAINED) | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,8 @@ package org.apache.spark.sql.delta.rapids | |
| import org.apache.spark.sql.{Column, DataFrame, SparkSession} | ||
| import org.apache.spark.sql.catalyst.expressions.Expression | ||
| import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan | ||
| import org.apache.spark.sql.delta.actions.FileAction | ||
| import org.apache.spark.sql.delta.commands.TouchedFileWithDV | ||
|
|
||
| /** | ||
| * Trait to abstract version-specific Spark API differences between Delta 3.3.x and Spark 4.x | ||
|
|
@@ -82,6 +84,14 @@ trait DeltaCommandShims { | |
| */ | ||
| def exprToColumn(expr: Expression): Column | ||
|
|
||
| /** | ||
| * Apply version-specific Delta statistics handling to new deletion-vector actions. | ||
| */ | ||
|
Comment on lines
+87
to
+89
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please mention that this interface and its implementations are ported from Delta. |
||
| def processUnmodifiedData( | ||
| spark: OperationSparkSession, | ||
| touchedFiles: Seq[TouchedFileWithDV], | ||
| txn: GpuOptimisticTransactionBase): (Seq[FileAction], Map[String, Long]) | ||
|
|
||
| /** | ||
| * Recache by plan with the correct SparkSession type. | ||
| */ | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this logic belongs here.
evolveSchemaAndClose()should handle the schema evolution. But this line seems just replacing the row index column with a valid one.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we just do
RapidsDeletionVectors.replaceColumnAndCloseinpostProcessChunkinstead?postProcessChunkis the callback to do any Delta-specific post processing after the read.