Support multiple where calls in DeltaTable.optimize
#1353
Conversation
|
#1344 continued |
| @@ -125,14 +125,16 @@ case class OptimizeTableCommand( | |||
| val partitionColumns = txn.snapshot.metadata.partitionColumns | |||
| // Parse the predicate expression into Catalyst expression and verify only simple filters | |||
| // on partition columns are present | |||
| val partitionPredicates = partitionPredicate.map(predicate => { | |||
|
|
|||
| val partitionPredicates = userPartitionPredicates.flatMap(partitionPredicate => | |||
There was a problem hiding this comment.
nit: maybe re-format the code a bit like this:
val partitionPredicates = userPartitionPredicates.flatMap { partitionPredicate =>
partitionPredicate.map { predicate =>
val predicates = parsePredicates(sparkSession, predicate)
verifyPartitionPredicates(
sparkSession,
partitionColumns,
predicates)
predicates
}.getOrElse(Seq.empty)
}
The current format took me a while to realize getOrElse is inside the function.
| @@ -39,7 +39,7 @@ class DeltaOptimizeBuilder private( | |||
| sparkSession: SparkSession, | |||
| tableIdentifier: String, | |||
| options: Map[String, String]) extends AnalysisHelper { | |||
| @volatile private var partitionFilter: Option[String] = None | |||
| @volatile private var partitionFilter: Seq[Option[String]] = Seq.empty | |||
There was a problem hiding this comment.
Just found we may be able to write a cleaner code if we change the type to Seq[String] (including changing OptimizeTableCommand.userPartitionPredicates to Seq[String]) ? Sorry. I should have found this earlier.
Could you also remove @volatile since you are touching the code? It's not needed.
There was a problem hiding this comment.
removed volatile . if you don't mind me asking what is the use Annotation volatile here ?
There was a problem hiding this comment.
@volatile is the same as Java volatile. As DeltaOptimizeBuilder is not supposed to be used in multiple thread, we don't need it.
Description
Resolves #1338
How was this patch tested?
test suite & by check the metrics of optimize operation in history table
one test added which replicates the previous (test above it ) with multiple where statement
Does this PR introduce any user-facing changes?